FFS: memory corruption

[Maxime Villard <max%M00nBSD.net@localhost>]

There is a kmem inconsistency in ffs_reload().

676	newfs = kmem_alloc(fs->fs_sbsize, KM_SLEEP);
677	memcpy(newfs, bp->b_data, fs->fs_sbsize);

and later:

705	memcpy(fs, newfs, (u_int)fs->fs_sbsize);
706	brelse(bp, 0);
707	kmem_free(newfs, fs->fs_sbsize);

The memcpy here overwrites fs->fs_sbsize with the new superblock
size read on the disk; so the value may change. kmem_free may
then pick up the wrong cache - and panic.

This looks like a proper bug, not "intentional". I would suggest
the following:

Index: ffs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.300
diff -u -r1.300 ffs_vfsops.c
--- ffs_vfsops.c	24 Oct 2014 13:18:51 -0000	1.300
+++ ffs_vfsops.c	27 Oct 2014 18:34:17 -0000
@@ -648,6 +648,7 @@
 	struct ufsmount *ump;
 	daddr_t sblockloc;
 	struct vnode_iterator *marker;
+	u_int32_t sbsize;
 
 	if ((mp->mnt_flag & MNT_RDONLY) == 0)
 		return (EINVAL);
@@ -666,15 +667,16 @@
 	 * Step 2: re-read superblock from disk.
 	 */
 	fs = ump->um_fs;
+	sbsize = fs->fs_sbsize;
 
 	/* XXX we don't handle possibility that superblock moved. */
-	error = bread(devvp, fs->fs_sblockloc / DEV_BSIZE, fs->fs_sbsize,
+	error = bread(devvp, fs->fs_sblockloc / DEV_BSIZE, sbsize,
 		      NOCRED, 0, &bp);
 	if (error) {
 		return (error);
 	}
-	newfs = kmem_alloc(fs->fs_sbsize, KM_SLEEP);
-	memcpy(newfs, bp->b_data, fs->fs_sbsize);
+	newfs = kmem_alloc(sbsize, KM_SLEEP);
+	memcpy(newfs, bp->b_data, sbsize);
 #ifdef FFS_EI
 	if (ump->um_flags & UFS_NEEDSWAP) {
 		ffs_sb_swap((struct fs*)bp->b_data, newfs);
@@ -687,7 +689,7 @@
 	     newfs->fs_bsize > MAXBSIZE ||
 	     newfs->fs_bsize < sizeof(struct fs)) {
 		brelse(bp, 0);
-		kmem_free(newfs, fs->fs_sbsize);
+		kmem_free(newfs, sbsize);
 		return (EIO);		/* XXX needs translation */
 	}
 	/* Store off old fs_sblockloc for fs_oldfscompat_read. */
@@ -702,9 +704,9 @@
 	newfs->fs_contigdirs = fs->fs_contigdirs;
 	newfs->fs_ronly = fs->fs_ronly;
 	newfs->fs_active = fs->fs_active;
-	memcpy(fs, newfs, (u_int)fs->fs_sbsize);
+	memcpy(fs, newfs, sbsize);
 	brelse(bp, 0);
-	kmem_free(newfs, fs->fs_sbsize);
+	kmem_free(newfs, sbsize);
 
 	/* Recheck for apple UFS filesystem */
 	ump->um_flags &= ~UFS_ISAPPLEUFS;


Ok/Comments?