git: d464a7698de8 - main - ffs: restore backward compatibility of newfs and makefs with older binaries

The branch main has been updated by chs:

URL: https://cgit.FreeBSD.org/src/commit/?id=d464a7698de8fe18530ea65fac90dce56b860a59

commit d464a7698de8fe18530ea65fac90dce56b860a59
Author:     Chuck Silvers <chs@FreeBSD.org>
AuthorDate: 2023-05-30 02:26:28 +0000
Commit:     Chuck Silvers <chs@FreeBSD.org>
CommitDate: 2023-05-30 02:26:28 +0000

    ffs: restore backward compatibility of newfs and makefs with older binaries
    
    The previous change to CGSIZE had the unintended side-effect of allowing
    newfs and makefs to create file systems that would fail validation when
    examined by older commands and kernels, by allowing newfs/makefs to pack
    slightly more blocks into a CG than those older binaries think is valid.
    Fix this by having newfs/makefs artificially restrict the number of blocks
    in a CG to the slightly smaller value that those older binaries will accept.
    The validation code will continue to accept the slightly larger value
    that the current newfs/makefs (before this change) could create.
    
    Fixes:          0a6e34e950cd5889122a199c34519b67569be9cc
    Reviewed by:    mckusick
    MFC after:      3 days
    Sponsored by:   Netflix
---
 sbin/newfs/mkfs.c          | 26 +++++++++++++++++++++++---
 usr.sbin/makefs/ffs/mkfs.c | 26 +++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/sbin/newfs/mkfs.c b/sbin/newfs/mkfs.c
index adc64f47cac6..28b02d250e17 100644
--- a/sbin/newfs/mkfs.c
+++ b/sbin/newfs/mkfs.c
@@ -76,6 +76,23 @@ __FBSDID("$FreeBSD$");
 #define UMASK		0755
 #define POWEROF2(num)	(((num) & ((num) - 1)) == 0)
 
+/*
+ * The definition of "struct cg" used to contain an extra field at the end
+ * to represent the variable-length data that followed the fixed structure.
+ * This had the effect of artificially limiting the number of blocks that
+ * newfs would put in a CG, since newfs thought that the fixed-size header
+ * was bigger than it really was.  When we started validating that the CG
+ * header data actually fit into one fs block, the placeholder field caused
+ * a problem because it caused struct cg to be a different size depending on
+ * platform.  The placeholder field was later removed, but this caused a
+ * backward compatibility problem with older binaries that still thought
+ * struct cg was larger, and a new file system could fail validation if
+ * viewed by the older binaries.  To avoid this compatibility problem, we
+ * now artificially reduce the amount of space that the variable-length data
+ * can use such that new file systems will pass validation by older binaries.
+ */
+#define CGSIZEFUDGE 8
+
 static struct	csum *fscs;
 #define	sblock	disk.d_fs
 #define	acg	disk.d_cg
@@ -369,7 +386,8 @@ retry:
 			sblock.fs_fpg = minfpg;
 		sblock.fs_ipg = roundup(howmany(sblock.fs_fpg, fragsperinode),
 		    INOPB(&sblock));
-		if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize)
+		if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize -
+		    CGSIZEFUDGE)
 			break;
 		density -= sblock.fs_fsize;
 	}
@@ -388,9 +406,11 @@ retry:
 		if (Oflag > 1 || (Oflag == 1 && sblock.fs_ipg <= 0x7fff)) {
 			if (sblock.fs_size / sblock.fs_fpg < MINCYLGRPS)
 				break;
-			if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize)
+			if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize -
+			    CGSIZEFUDGE)
 				continue;
-			if (CGSIZE(&sblock) == (unsigned long)sblock.fs_bsize)
+			if (CGSIZE(&sblock) == (unsigned long)sblock.fs_bsize -
+			    CGSIZEFUDGE)
 				break;
 		}
 		sblock.fs_fpg -= sblock.fs_frag;
diff --git a/usr.sbin/makefs/ffs/mkfs.c b/usr.sbin/makefs/ffs/mkfs.c
index d48dc65aac68..4e88dae7aae8 100644
--- a/usr.sbin/makefs/ffs/mkfs.c
+++ b/usr.sbin/makefs/ffs/mkfs.c
@@ -80,6 +80,23 @@ static int count_digits(int);
 #define	UMASK		0755
 #define	POWEROF2(num)	(((num) & ((num) - 1)) == 0)
 
+/*
+ * The definition of "struct cg" used to contain an extra field at the end
+ * to represent the variable-length data that followed the fixed structure.
+ * This had the effect of artificially limiting the number of blocks that
+ * newfs would put in a CG, since newfs thought that the fixed-size header
+ * was bigger than it really was.  When we started validating that the CG
+ * header data actually fit into one fs block, the placeholder field caused
+ * a problem because it caused struct cg to be a different size depending on
+ * platform.  The placeholder field was later removed, but this caused a
+ * backward compatibility problem with older binaries that still thought
+ * struct cg was larger, and a new file system could fail validation if
+ * viewed by the older binaries.  To avoid this compatibility problem, we
+ * now artificially reduce the amount of space that the variable-length data
+ * can use such that new file systems will pass validation by older binaries.
+ */
+#define CGSIZEFUDGE 8
+
 static union {
 	struct fs fs;
 	char pad[SBLOCKSIZE];
@@ -347,7 +364,8 @@ ffs_mkfs(const char *fsys, const fsinfo_t *fsopts, time_t tstamp)
 			sblock.fs_fpg = minfpg;
 		sblock.fs_ipg = roundup(howmany(sblock.fs_fpg, fragsperinode),
 		    INOPB(&sblock));
-		if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize)
+		if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize -
+		    CGSIZEFUDGE)
 			break;
 		density -= sblock.fs_fsize;
 	}
@@ -366,9 +384,11 @@ ffs_mkfs(const char *fsys, const fsinfo_t *fsopts, time_t tstamp)
 		    INOPB(&sblock));
 		if (sblock.fs_size / sblock.fs_fpg < 1)
 			break;
-		if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize)
+		if (CGSIZE(&sblock) < (unsigned long)sblock.fs_bsize -
+		    CGSIZEFUDGE)
 			continue;
-		if (CGSIZE(&sblock) == (unsigned long)sblock.fs_bsize)
+		if (CGSIZE(&sblock) == (unsigned long)sblock.fs_bsize -
+		    CGSIZEFUDGE)
 			break;
 		sblock.fs_fpg -= sblock.fs_frag;
 		sblock.fs_ipg = roundup(howmany(sblock.fs_fpg, fragsperinode),