Re: CVS commit: src/sys/fs/msdosfs

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

jnemeth@ wrote:

> On Nov 20,  5:37am, "Izumi Tsutsui" wrote:
> } 
> } Module Name:        src
> } Committed By:       tsutsui
> } Date:               Sat Jun 30 11:01:42 UTC 2012
> } 
> } Modified Files:
> }     src/sys/fs/msdosfs: msdosfs_vfsops.c
> } 
> } Log Message:
> } 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.

> } See also:
> } http://mail-index.NetBSD.org/source-changes/2012/06/30/msg035298.html
> } 
> } To generate a diff of this commit:
> } cvs rdiff -u -r1.94 -r1.95 src/sys/fs/msdosfs/msdosfs_vfsops.c
> } 
> } Please note that diffs are not public domain; they are subject to the
> } copyright notices on the relevant files.
> } 
> } Modified files:
> } 
> } --- src/sys/fs/msdosfs/msdosfs_vfsops.c:1.94        Tue Mar 13 18:40:37 2012
> } +++ src/sys/fs/msdosfs/msdosfs_vfsops.c     Sat Jun 30 11:01:41 2012
> } @@ -493,7 +493,7 @@ msdosfs_mountfs(struct vnode *devvp, str
> }             goto error_exit;
> }  
> }     error = getdisksize(devvp, &psize, &secsize);
> } -   if (error) {
> } +   if (error || secsize == 0) {
> 
>      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.

---
Izumi Tsutsui