dholland@ wrote: > On Tue, Jul 03, 2012 at 08:01:21PM +0900, Izumi Tsutsui wrote: > > > } Add a sanity check if secsize returned from getdisksize() isn't bogus. > > > } This prevent possible panic "panic: buf mem pool index 23" later in > > > } vfs_bio.c:buf_mempoolidx(). > > > } (I'm not sure if it's okay for getdisksize() to assume that > > > } partinfo taken from DIOCGPART is properly initialized > > > } on all disk(9) devices or not) > > > > > > I think a better question is, why is getdisksize() returning > > > garbage instead of returning an error? > > > > I don't know because there is no getdisksize(9) man page. > > Probably we have to ask the author of it. > > getdisksize() is not complicated; it tries DIOCGPART and then > DIOCGWEDGEINFO and that's about it.
Yes, but it is not implementation issue but design issue. > I think it's wrong for either of those to be returning a size of 0. Yes, but which one? getdisksize() or DIOCGPART in each driver? > > > If this check is really necessary, I would be more inclined to use > > > KASSERT(). > > > > I can change it to KASSERT if we can assume that partinfo taken > > from DIOCGPART must be initialized properly on all disk(9) devices, > > i.e. if we can say disklab->d_secsize==0 means driver's bug, > > but I could not find any evidence. > > > > All vfs mountfs functions are called in case of "root on generic" > > via mountroot in setroot() so I thought returning error was better > > than panic in that case, even after md(4) was fixed. > > But if you don't agree it, please revert it. > > If we know it can happen, crashing isn't very productive. > > However, can you move the test to inside getdisksize()? Make it print > a message and return EINVAL (or something). That way things other than > msdosfs are also protected. It depends how the design issue is resolved. If DIOCGPART must not return secsize=0, KASSERT in getdisksize() seems reasonable. If uninitialized secsize=0 from DIOCGPART is acceptable, getdisksize() or each caller should have sanity checks. --- Izumi Tsutsui
