[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys/fs/msdosfs
> 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.
Main Index |
Thread Index |