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
