On Mon, Oct 20, 2014 at 03:38:11PM +0200, Maxime Villard wrote: > I think the sanity check should be: > > Index: ffs_vfsops.c > =================================================================== > RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vfsops.c,v > retrieving revision 1.299 > diff -u -r1.299 ffs_vfsops.c > --- ffs_vfsops.c 24 May 2014 16:34:04 -0000 1.299 > +++ ffs_vfsops.c 20 Oct 2014 13:01:46 -0000 > @@ -974,7 +974,7 @@ > continue; > > /* Validate size of superblock */ > - if (sbsize > MAXBSIZE || sbsize < sizeof(struct fs)) > + if (sbsize > SBLOCKSIZE || sbsize < sizeof(struct fs)) > continue; > > /* Check that we can handle the file system blocksize */ > > Tested on NetBSD-current: no longer crashes. > > Ok/Comments?
I think the check should be left alone, but afterwards the value should be clamped to the amount of data that can actually be transferred. Otherwise I think it may break, e.g. on volumes with odd block sizes. I'm not sure what gets put in sbsize in such cases; if it's always SBLOCKSIZE, then we should probably reject anything that isn't exactly SBLOCKSIZE and forget about slop or hypothetical compatibility with larger superblocks. If it's the block size of the block the superblock is in, though, then there should be a test like this. Another semi-related thing to check, btw: does it invalidate the buffers for candidate superblocks that reads and then rejects? Failure to do this when the volume blocksize isn't SBLOCKSIZE (since it seems to always read buffers of size SBLOCKSIZE here) might cause interesting overlapping buffer lossage later. -- David A. Holland dholl...@netbsd.org