Date: Sat, 19 Oct 2002 17:49:49 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: BOUWSMA Beery <freebsd-misuser@ipv6.netscum.dyndns.dk> Cc: freebsd-fs@FreeBSD.ORG Subject: Re: UFS2 panic (and other issues) at mount with unusual `newfs' values Message-ID: <20021019165432.W2104-100000@gamplex.bde.org> In-Reply-To: <200210121522.g9CFMMt00455@MAIL.NetScum.DynDNS.dK>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021019165432.W2104-100000>