From owner-freebsd-fs Sat Oct 19 0:39:16 2002 Delivered-To: freebsd-fs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1E08137B401 for ; Sat, 19 Oct 2002 00:39:11 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 083C343E9C for ; Sat, 19 Oct 2002 00:39:10 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id RAA32031; Sat, 19 Oct 2002 17:39:02 +1000 Date: Sat, 19 Oct 2002 17:49:49 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: BOUWSMA Beery Cc: freebsd-fs@FreeBSD.ORG Subject: Re: UFS2 panic (and other issues) at mount with unusual `newfs' values In-Reply-To: <200210121522.g9CFMMt00455@MAIL.NetScum.DynDNS.dK> Message-ID: <20021019165432.W2104-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org On Sat, 12 Oct 2002, BOUWSMA Beery wrote: > [IPv6-only address above; strip the obvious for IPv4-only mail] > > About three months ago, I bothered both this list and -current with > a message, and I've finally gotten around to taking a closer look at > it, keeping it in -fs this time. I wrote: > > > It seems that any -f fsize value larger than 8192 given to `newfs' > > creates a UFS2 filesystem that causes a panic when mounted, even > > > #15 0xc0297772 in ffs_mount (mp=0xc1918400, path=0xc1922180 "/mnt", data=---Can' > > t read userspace from dump, or kernel process--- > > Adding some debuggery makes it appear that the real crash occurs > in sys/ufs/ffs/ffs_vfsops.c in ffs_mountfs() right here: > > /* XXX DEBUG */ printf("ffs_mountfs: checkpoint 9\n"); > bcopy(bp->b_data, ump->um_fs, (u_int)fs->fs_sbsize); > /* XXX DEBUG */ printf("ffs_mountfs: checkpoint 10\n"); > > This should be line 690, plus or minus a few, in 14.Sep 1.190 > revision of this (last one I tried to build). The panic is just because we have allocated a buffer of size SBLOCKSIZE using bread(), but here we bcopy an amount potentially larger than SBLOCKSIZE into the buffer. Your patch that was recently committed to newfs/mkfs.c stops mkfs creating filesystems with such a larger fs_sbsize. I use the following patches which would have turned the panic into a mount failure. The original version of these was to try to get MAXBSIZE = DEV_BSIZE working so that the block size could be the same as the fragment size even when the fragment size is DEV_BSIZE. This doesn't quite work due to problems with fs_sbsize being small instead of large. IIRC, sizeof(struct fs) is slightly less than 1536 and things fail unless fs_bsize is larger than this (it must be at least 2048 after rounding). %%% Index: ffs_vfsops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_vfsops.c,v retrieving revision 1.194 diff -u -2 -r1.194 ffs_vfsops.c --- ffs_vfsops.c 15 Oct 2002 20:00:06 -0000 1.194 +++ ffs_vfsops.c 16 Oct 2002 12:47:01 -0000 @@ -429,8 +425,17 @@ newfs->fs_magic != FS_UFS2_MAGIC) || newfs->fs_bsize > MAXBSIZE || - newfs->fs_bsize < sizeof(struct fs)) { - brelse(bp); - return (EIO); /* XXX needs translation */ + newfs->fs_sbsize > SBLOCKSIZE) { + bp->b_flags |= B_INVAL | B_NOCACHE; + brelse(bp); + return (EIO); /* XXX needs translation */ } + if (newfs->fs_bsize < sizeof(struct fs)) + printf("newfs->fs_bsize < sizeof(struct fs) (%lu <= %lu)\n", + (u_long)newfs->fs_bsize, (u_long)sizeof(struct fs)); + if (newfs->fs_sbsize != fragroundup(newfs, sizeof(struct fs))) + printf( + "newfs->fs_sbsize != fragroundup(newfs, sizeof(struct fs)) (%lu != %lu)\n", + (u_long)newfs->fs_sbsize, + (u_long)fragroundup(newfs, sizeof(struct fs))); /* * Copy pointer fields back into superblock before copying in XXX @@ -625,6 +623,7 @@ fs->fs_sblockloc == sblockloc)) && fs->fs_bsize <= MAXBSIZE && - fs->fs_bsize >= sizeof(struct fs)) + fs->fs_sbsize <= SBLOCKSIZE) break; + bp->b_flags |= B_INVAL | B_NOCACHE; brelse(bp); bp = NULL; @@ -685,5 +690,5 @@ ump->um_vfree = ffs_vfree; bcopy(bp->b_data, ump->um_fs, (u_int)fs->fs_sbsize); - if (fs->fs_sbsize < SBLOCKSIZE) + if (fs->fs_sbsize != fs->fs_bsize) bp->b_flags |= B_INVAL | B_NOCACHE; brelse(bp); %%% Notes on the above patch: %%% % Index: ffs_vfsops.c % =================================================================== % RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_vfsops.c,v % retrieving revision 1.194 % diff -u -2 -r1.194 ffs_vfsops.c % --- ffs_vfsops.c 15 Oct 2002 20:00:06 -0000 1.194 % +++ ffs_vfsops.c 16 Oct 2002 12:47:01 -0000 % @@ -429,8 +425,17 @@ % newfs->fs_magic != FS_UFS2_MAGIC) || % newfs->fs_bsize > MAXBSIZE || % - newfs->fs_bsize < sizeof(struct fs)) { % - brelse(bp); % - return (EIO); /* XXX needs translation */ % + newfs->fs_sbsize > SBLOCKSIZE) { Don't reject filesystems with a small blocksize, since these can work. Reject filesystems with a too-large fs_sbsize instead. % + bp->b_flags |= B_INVAL | B_NOCACHE; Discard the buffer immediately since we are certain we don't want it, and keeping buffers of unusual sizes around can be dangerous. The usual size is fs_bsize for a valid filesystems, and here we don't even have a valid filesystem. % + brelse(bp); % + return (EIO); /* XXX needs translation */ Fix indentation. % } % + if (newfs->fs_bsize < sizeof(struct fs)) % + printf("newfs->fs_bsize < sizeof(struct fs) (%lu <= %lu)\n", % + (u_long)newfs->fs_bsize, (u_long)sizeof(struct fs)); % + if (newfs->fs_sbsize != fragroundup(newfs, sizeof(struct fs))) % + printf( % + "newfs->fs_sbsize != fragroundup(newfs, sizeof(struct fs)) (%lu != %lu)\n", % + (u_long)newfs->fs_sbsize, % + (u_long)fragroundup(newfs, sizeof(struct fs))); % /* % * Copy pointer fields back into superblock before copying in XXX Debugging code. newfs->fs_bsize can be smaller than sizeof(struct fs) when MAXBSIZE < 4096. Some of these cases worked for me and I think all should work provided the blocking is done carefully. % @@ -625,6 +623,7 @@ % fs->fs_sblockloc == sblockloc)) && % fs->fs_bsize <= MAXBSIZE && % - fs->fs_bsize >= sizeof(struct fs)) % + fs->fs_sbsize <= SBLOCKSIZE) % break; % + bp->b_flags |= B_INVAL | B_NOCACHE; % brelse(bp); % bp = NULL; As above (above is for the reload case; this is the main case). Invalidating the buffer is more useful here, since we bread() with size SBLOCKSIZE here but only want to keep a buffer of size fs_sbsize. % @@ -685,5 +690,5 @@ % ump->um_vfree = ffs_vfree; % bcopy(bp->b_data, ump->um_fs, (u_int)fs->fs_sbsize); % - if (fs->fs_sbsize < SBLOCKSIZE) % + if (fs->fs_sbsize != fs->fs_bsize) % bp->b_flags |= B_INVAL | B_NOCACHE; % brelse(bp); More superstition about keeping odd-size buffers. I think both of the conditions are wrong. Only buffers of size fs_sbsize are useful. %%% > First question: with UFS2, what should the values for sbsize > be, so I know if the above line is at fault, or if the numbers > I give below indicate a problem elsewhere, like in newfs or > something? I believe fs_sbsize is now just sizeof(struct fs) rounded up a little now that there is no rotational info at the end of it. It must be rounded to a multiple of DEV_BSIZE. Further rounding to a multiple of fs_bsize or SBLOCKSIZE isn't essential although it might simplify blocking. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message