Subject: checking for unknown bits in ufs's fs_flags
To: None <tech-kern@netbsd.org>
From: Darrin B.Jewell <dbj@netbsd.org>
List: tech-kern
Date: 06/02/2005 22:57:21
--=-=-=


The first of the two patches below causes fsck check for unrecognized
bits in the ufs filesystem fs_flags field and causes fsck -p and mount
to return an error rather than potentially harm the disk.  This
provides some protection in current sources against unknown future ffs
features.

Unfortunately, not all potential future features that set a flag would
be harmed by our current fsck, and not all potential future features
are guaranteed to set the flag.  Furthermore, some flags, are just
persistent store for a mount option and don't really provide strong
information that a filesystem's layout is different.  However, I think
it will enhance the fs_flags semantics and provide some future
protection.  Does anyone have an objection to this check?

As I mentioned, not all features will likely set a flag.  Consider the
case of ufs2 extended attributes, which have been in use in freebsd
since the original release of ufs2.  Since they were there from the
start, no such flag was needed or useful.  Unfortunately, it was an
error for us to take the ufs2 changes without at least including
support in fsck for scanning the blocks for extended attributes.
Currently, our fsck will corrupt freebsd filesystems that have
extended attributes set.  To fix this, I plan to commit the second
patch below, although of course there's no way to go back and upgrade
the fsck for already released sources.  Getting this fix in sooner
rather than later will ease the upgrade path if/when we do add the
rest of the extended attribute support.

Darrin


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=fsflags.diff
Content-Description: patch to check fs_flags for unknown bits

Index: src/sys/ufs/ffs/fs.h
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/fs.h,v
retrieving revision 1.45
diff -u -u -p -r1.45 fs.h
--- src/sys/ufs/ffs/fs.h	26 Feb 2005 22:32:20 -0000	1.45
+++ src/sys/ufs/ffs/fs.h	1 Jun 2005 19:46:25 -0000
@@ -414,6 +414,9 @@ struct fs {
 #define FS_MULTILABEL	0x20	/* file system is MAC multi-label */
 #define FS_FLAGS_UPDATED 0x80	/* flags have been moved to new location */
 
+/* File system flags that are ok for NetBSD if set in fs_flags */
+#define FS_KNOWN_FLAGS (FS_DOSOFTDEP)
+
 /*
  * File system internal flags, also in fs_flags.
  * (Pick highest number to avoid conflicts with others)
Index: src/sys/ufs/ffs/ffs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.163
diff -u -u -p -r1.163 ffs_vfsops.c
--- src/sys/ufs/ffs/ffs_vfsops.c	29 Mar 2005 02:41:06 -0000	1.163
+++ src/sys/ufs/ffs/ffs_vfsops.c	1 Jun 2005 19:46:26 -0000
@@ -598,6 +598,16 @@ ffs_reload(mp, cred, p)
 	}
 	ffs_oldfscompat_read(fs, ump, sblockloc);
 	ump->um_maxfilesize = fs->fs_maxfilesize;
+
+	if (fs->fs_flags & ~(FS_KNOWN_FLAGS|FS_INTERNAL)) {
+		uprintf("%s: unknown ufs flags: 0x%08"PRIx32"%s\n",
+				mp->mnt_stat.f_mntonname, fs->fs_flags,
+				(mp->mnt_flag & MNT_FORCE) ? "" : ", not mounting");
+		if ((mp->mnt_flag & MNT_FORCE) == 0) {
+			return (EINVAL);
+		}
+	}
+
 	if (fs->fs_pendingblocks != 0 || fs->fs_pendinginodes != 0) {
 		fs->fs_pendingblocks = 0;
 		fs->fs_pendinginodes = 0;
@@ -829,6 +839,16 @@ ffs_mountfs(devvp, mp, p)
 	ffs_oldfscompat_read(fs, ump, sblockloc);
 	ump->um_maxfilesize = fs->fs_maxfilesize;
 
+	if (fs->fs_flags & ~(FS_KNOWN_FLAGS|FS_INTERNAL)) {
+		uprintf("%s: unknown ufs flags: 0x%08"PRIx32"%s\n",
+				mp->mnt_stat.f_mntonname, fs->fs_flags,
+				(mp->mnt_flag & MNT_FORCE) ? "" : ", not mounting");
+		if ((mp->mnt_flag & MNT_FORCE) == 0) {
+			error = EINVAL;
+			goto out;
+		}
+	}
+
 	if (fs->fs_pendingblocks != 0 || fs->fs_pendinginodes != 0) {
 		fs->fs_pendingblocks = 0;
 		fs->fs_pendinginodes = 0;
Index: src/sbin/fsck_ffs/setup.c
===================================================================
RCS file: /cvsroot/src/sbin/fsck_ffs/setup.c,v
retrieving revision 1.75
diff -u -u -p -r1.75 setup.c
--- src/sbin/fsck_ffs/setup.c	19 Jan 2005 17:33:59 -0000	1.75
+++ src/sbin/fsck_ffs/setup.c	1 Jun 2005 19:46:27 -0000
@@ -218,6 +218,13 @@ setup(const char *dev)
 	/*
 	 * Check and potentially fix certain fields in the super block.
 	 */
+	if (sblock->fs_flags & ~(FS_KNOWN_FLAGS)) {
+		pfatal("UNKNOWN FLAGS=0x%08x IN SUPERBLOCK", sblock->fs_flags);
+		if (reply("CLEAR") == 1) {
+			sblock->fs_flags &= FS_KNOWN_FLAGS;
+			sbdirty();
+		}
+	}
 	if (sblock->fs_optim != FS_OPTTIME && sblock->fs_optim != FS_OPTSPACE) {
 		pfatal("UNDEFINED OPTIMIZATION IN SUPERBLOCK");
 		if (reply("SET TO DEFAULT") == 1) {

--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=fsck_extattr.diff
Content-Description: patch to correctly account for extattr ufs2 blocks

Index: pass1.c
===================================================================
RCS file: /u3/n/rsync/cvsroot/src/sbin/fsck_ffs/pass1.c,v
retrieving revision 1.37
diff -u -u -b -r1.37 pass1.c
--- pass1.c	30 Apr 2005 20:24:32 -0000	1.37
+++ pass1.c	3 Jun 2005 01:30:09 -0000
@@ -395,6 +395,25 @@
 	else
 		idesc->id_type = ADDR;
 	(void)ckinode(dp, idesc);
+	if (is_ufs2 && iswap32(dp->dp2.di_extsize) > 0) {
+		int ret, offset;
+		idesc->id_type = ADDR;
+		ndb = howmany(iswap32(dp->dp2.di_extsize), sblock->fs_bsize);
+		for (j = 0; j < NXADDR; j++) {
+			if (--ndb == 0 &&
+			    (offset = blkoff(sblock, iswap32(dp->dp2.di_extsize))) != 0)
+				idesc->id_numfrags = numfrags(sblock,
+				    fragroundup(sblock, offset));
+			else
+				idesc->id_numfrags = sblock->fs_frag;
+			if (dp->dp2.di_extb[j] == 0)
+				continue;
+			idesc->id_blkno = iswap64(dp->dp2.di_extb[j]);
+			ret = (*idesc->id_func)(idesc);
+			if (ret & STOP)
+				break;
+		}
+	}
 	idesc->id_entryno *= btodb(sblock->fs_fsize);
 	if (is_ufs2)
 		blocks = iswap64(dp->dp2.di_blocks);

--=-=-=--