Index | Thread | Search

From:
Philip Guenther <guenther@gmail.com>
Subject:
delete broken support for antique FFS images
To:
Tech-OpenBSD <tech@openbsd.org>
Date:
Mon, 1 Jan 2024 23:27:00 -0800

Download raw body.

Thread
The original 4.2(?)BSD FFS always put the 'value' of a symlink in a data 
block.  It was realized that 99+% of symlinks could be fit inside the 
symlink's inode if you reusing the area where the direct and indirect 
block addresses would normally be stored, so Kirk taught ffs to do that.  
We had partial support for filesystems that used that old format, but I 
unintentionally broke it in the 64bit time_t work in early 2014, such that 
since OpenBSD 5.5, we haven't handled fs with that old layout, at least on 
little-endian systems.

So, since no one actually putting such filesystems to use noticed, it's 
clearly time to completely disable support for them.


Delete support for FFS filesystems before the in-inode symlink 
optimization.  As observed by ali_farzanrad(at)riseup.net, support for 
these was broken in the 5.5 release in early 2014 by the time_t changes.  
No one noticed before now, so clearly this isn't something we need to 
continue to support; rejecting in ffs_validate() is an improvement.

Maybe I should keep the braces on the 'if' in ufs_readlink()?


Philip Guenther


Index: sbin/fsck_ffs/dir.c
===================================================================
RCS file: /data/src/openbsd/src/sbin/fsck_ffs/dir.c,v
diff -u -p -r1.33 dir.c
--- sbin/fsck_ffs/dir.c	8 Feb 2023 08:25:44 -0000	1.33
+++ sbin/fsck_ffs/dir.c	2 Jan 2024 04:46:10 -0000
@@ -52,10 +52,6 @@ struct	dirtemplate dirhead = {
 	0, 12, DT_DIR, 1, ".",
 	0, DIRBLKSIZ - 12, DT_DIR, 2, ".."
 };
-struct	odirtemplate odirhead = {
-	0, 12, 1, ".",
-	0, DIRBLKSIZ - 12, 2, ".."
-};
 
 static int expanddir(union dinode *, char *);
 static void freedir(ino_t, ino_t);
Index: sys/ufs/ffs/ffs_inode.c
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ffs/ffs_inode.c,v
diff -u -p -r1.81 ffs_inode.c
--- sys/ufs/ffs/ffs_inode.c	12 Dec 2021 09:14:59 -0000	1.81
+++ sys/ufs/ffs/ffs_inode.c	2 Jan 2024 03:26:49 -0000
@@ -152,9 +152,7 @@ ffs_truncate(struct inode *oip, off_t le
 		return (0);
 
 	if (ovp->v_type == VLNK &&
-	    (DIP(oip, size) < oip->i_ump->um_maxsymlinklen ||
-	     (oip->i_ump->um_maxsymlinklen == 0 &&
-	      oip->i_din1->di_blocks == 0))) {
+	    DIP(oip, size) < oip->i_ump->um_maxsymlinklen) {
 #ifdef DIAGNOSTIC
 		if (length != 0)
 			panic("ffs_truncate: partial truncate of symlink");
Index: sys/ufs/ffs/ffs_vfsops.c
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ffs/ffs_vfsops.c,v
diff -u -p -r1.195 ffs_vfsops.c
--- sys/ufs/ffs/ffs_vfsops.c	5 Jul 2023 15:13:28 -0000	1.195
+++ sys/ufs/ffs/ffs_vfsops.c	2 Jan 2024 03:25:04 -0000
@@ -675,8 +675,8 @@ ffs_validate(struct fs *fsp)
 		return (0); /* Invalid number of fragments */
 
 	if (fsp->fs_inodefmt == FS_42INODEFMT)
-		fsp->fs_maxsymlinklen = 0;
-	else if (fsp->fs_maxsymlinklen < 0)
+		return (0); /* Obsolete format, support broken in 2014 */
+	if (fsp->fs_maxsymlinklen <= 0)
 		return (0); /* Invalid max size of short symlink */
 
 	return (1); /* Super block is okay */
Index: sys/ufs/ffs/ffs_vnops.c
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ffs/ffs_vnops.c,v
diff -u -p -r1.100 ffs_vnops.c
--- sys/ufs/ffs/ffs_vnops.c	26 Jun 2022 05:20:43 -0000	1.100
+++ sys/ufs/ffs/ffs_vnops.c	2 Jan 2024 03:28:01 -0000
@@ -202,8 +202,7 @@ ffs_read(void *v)
 		panic("ffs_read: mode");
 
 	if (vp->v_type == VLNK) {
-		if (DIP(ip, size) < ip->i_ump->um_maxsymlinklen ||
-		    (ip->i_ump->um_maxsymlinklen == 0 && DIP(ip, blocks) == 0))
+		if (DIP(ip, size) < ip->i_ump->um_maxsymlinklen)
 			panic("ffs_read: short symlink");
 	} else if (vp->v_type != VREG && vp->v_type != VDIR)
 		panic("ffs_read: type %d", vp->v_type);
Index: sys/ufs/ufs/dir.h
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ufs/dir.h,v
diff -u -p -r1.12 dir.h
--- sys/ufs/ufs/dir.h	4 May 2019 15:38:12 -0000	1.12
+++ sys/ufs/ufs/dir.h	2 Jan 2024 04:46:35 -0000
@@ -112,15 +112,8 @@ struct	direct {
 #define DIRECTSIZ(namlen)						\
 	((offsetof(struct direct, d_name) +				\
 	  ((namlen)+1)*sizeof(((struct direct *)0)->d_name[0]) + 3) & ~3)
-#if (BYTE_ORDER == LITTLE_ENDIAN)
-#define DIRSIZ(oldfmt, dp) \
-    ((oldfmt) ? \
-    ((sizeof(struct direct) - (MAXNAMLEN+1)) + (((dp)->d_type+1 + 3) &~ 3)) : \
-    ((sizeof(struct direct) - (MAXNAMLEN+1)) + (((dp)->d_namlen+1 + 3) &~ 3)))
-#else
 #define DIRSIZ(oldfmt, dp) \
     ((sizeof(struct direct) - (MAXNAMLEN+1)) + (((dp)->d_namlen+1 + 3) &~ 3))
-#endif
 #define OLDDIRFMT	1
 #define NEWDIRFMT	0
 
@@ -138,20 +131,6 @@ struct dirtemplate {
 	int16_t		dotdot_reclen;
 	u_int8_t	dotdot_type;
 	u_int8_t	dotdot_namlen;
-	char		dotdot_name[4];	/* ditto */
-};
-
-/*
- * This is the old format of directories, sanz type element.
- */
-struct odirtemplate {
-	u_int32_t	dot_ino;
-	int16_t		dot_reclen;
-	u_int16_t	dot_namlen;
-	char		dot_name[4];	/* must be multiple of 4 */
-	u_int32_t	dotdot_ino;
-	int16_t		dotdot_reclen;
-	u_int16_t	dotdot_namlen;
 	char		dotdot_name[4];	/* ditto */
 };
 #endif /* !_DIR_H_ */
Index: sys/ufs/ufs/ufs_dirhash.c
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ufs/ufs_dirhash.c,v
diff -u -p -r1.42 ufs_dirhash.c
--- sys/ufs/ufs/ufs_dirhash.c	15 Mar 2019 05:42:38 -0000	1.42
+++ sys/ufs/ufs/ufs_dirhash.c	2 Jan 2024 04:40:48 -0000
@@ -50,7 +50,6 @@
 
 #define WRAPINCR(val, limit)	(((val) + 1 == (limit)) ? 0 : ((val) + 1))
 #define WRAPDECR(val, limit)	(((val) == 0) ? ((limit) - 1) : ((val) - 1))
-#define OFSFMT(ip)		((ip)->i_ump->um_maxsymlinklen == 0)
 #define BLKFREE2IDX(n)		((n) > DH_NFSTATS ? DH_NFSTATS : (n))
 
 int ufs_mindirhashsize;
@@ -112,7 +111,7 @@ ufsdirhash_build(struct inode *ip)
 
 	/* Check if we can/should use dirhash. */
 	if (ip->i_dirhash == NULL) {
-		if (DIP(ip, size) < ufs_mindirhashsize || OFSFMT(ip))
+		if (DIP(ip, size) < ufs_mindirhashsize)
 			return (-1);
 	} else {
 		/* Hash exists, but sysctls could have changed. */
Index: sys/ufs/ufs/ufs_lookup.c
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ufs/ufs_lookup.c,v
diff -u -p -r1.59 ufs_lookup.c
--- sys/ufs/ufs/ufs_lookup.c	11 Jan 2022 03:13:59 -0000	1.59
+++ sys/ufs/ufs/ufs_lookup.c	2 Jan 2024 04:41:32 -0000
@@ -64,8 +64,6 @@ int	dirchk = 1;
 int	dirchk = 0;
 #endif
 
-#define OFSFMT(ip)	((ip)->i_ump->um_maxsymlinklen == 0)
-
 /*
  * Convert a component of a pathname into a pointer to a locked inode.
  * This is a very central and rather complicated routine.
@@ -299,7 +297,7 @@ searchloop:
 			int size = ep->d_reclen;
 
 			if (ep->d_ino != 0)
-				size -= DIRSIZ(OFSFMT(dp), ep);
+				size -= DIRSIZ(0, ep);
 			if (size > 0) {
 				if (size >= slotneeded) {
 					slotstatus = FOUND;
@@ -322,14 +320,7 @@ searchloop:
 		 * Check for a name match.
 		 */
 		if (ep->d_ino) {
-#			if (BYTE_ORDER == LITTLE_ENDIAN)
-				if (OFSFMT(dp))
-					namlen = ep->d_type;
-				else
-					namlen = ep->d_namlen;
-#			else
-				namlen = ep->d_namlen;
-#			endif
+			namlen = ep->d_namlen;
 			if (namlen == cnp->cn_namelen &&
 			    !memcmp(cnp->cn_nameptr, ep->d_name, namlen)) {
 #ifdef UFS_DIRHASH
@@ -440,9 +431,9 @@ found:
 	 * Check that directory length properly reflects presence
 	 * of this entry.
 	 */
-	if (dp->i_offset + DIRSIZ(OFSFMT(dp), ep) > DIP(dp, size)) {
+	if (dp->i_offset + DIRSIZ(0, ep) > DIP(dp, size)) {
 		ufs_dirbad(dp, dp->i_offset, "i_ffs_size too small");
-		DIP_ASSIGN(dp, size, dp->i_offset + DIRSIZ(OFSFMT(dp), ep));
+		DIP_ASSIGN(dp, size, dp->i_offset + DIRSIZ(0, ep));
 		dp->i_flag |= IN_CHANGE | IN_UPDATE;
 	}
 	brelse(bp);
@@ -626,17 +617,10 @@ ufs_dirbadentry(struct vnode *vdp, struc
 
 	dp = VTOI(vdp);
 
-#	if (BYTE_ORDER == LITTLE_ENDIAN)
-		if (OFSFMT(dp))
-			namlen = ep->d_type;
-		else
-			namlen = ep->d_namlen;
-#	else
-		namlen = ep->d_namlen;
-#	endif
+	namlen = ep->d_namlen;
 	if ((ep->d_reclen & 0x3) != 0 ||
 	    ep->d_reclen > DIRBLKSIZ - (entryoffsetinblock & (DIRBLKSIZ - 1)) ||
-	    ep->d_reclen < DIRSIZ(OFSFMT(dp), ep) || namlen > MAXNAMLEN) {
+	    ep->d_reclen < DIRSIZ(0, ep) || namlen > MAXNAMLEN) {
 		/*return (1); */
 		printf("First bad\n");
 		goto bad;
@@ -674,15 +658,7 @@ ufs_makedirentry(struct inode *ip, struc
 	memset(newdirp->d_name + (cnp->cn_namelen & ~(DIR_ROUNDUP-1)),
 	    0, DIR_ROUNDUP);
 	memcpy(newdirp->d_name, cnp->cn_nameptr, cnp->cn_namelen);
-	if (OFSFMT(ip)) {
-		newdirp->d_type = 0;
-#		if (BYTE_ORDER == LITTLE_ENDIAN)
-			{ u_char tmp = newdirp->d_namlen;
-			newdirp->d_namlen = newdirp->d_type;
-			newdirp->d_type = tmp; }
-#		endif
-	} else
-		newdirp->d_type = IFTODT(DIP(ip, mode));
+	newdirp->d_type = IFTODT(DIP(ip, mode));
 }
   
 /*
@@ -712,7 +688,7 @@ ufs_direnter(struct vnode *dvp, struct v
 	cr = cnp->cn_cred;
 	p = cnp->cn_proc;
 	dp = VTOI(dvp);
-	newentrysize = DIRSIZ(OFSFMT(dp), dirp);
+	newentrysize = DIRSIZ(0, dirp);
 
 	if (dp->i_count == 0) {
 		/*
@@ -827,7 +803,7 @@ ufs_direnter(struct vnode *dvp, struct v
 	 * dp->i_offset + dp->i_count would yield the space.
 	 */
 	ep = (struct direct *)dirbuf;
-	dsize = ep->d_ino ? DIRSIZ(OFSFMT(dp), ep) : 0;
+	dsize = ep->d_ino ? DIRSIZ(0, ep) : 0;
 	spacefree = ep->d_reclen - dsize;
 	for (loc = ep->d_reclen; loc < dp->i_count; ) {
 		nep = (struct direct *)(dirbuf + loc);
@@ -852,7 +828,7 @@ ufs_direnter(struct vnode *dvp, struct v
 			dsize = 0;
 			continue;
 		}
-		dsize = DIRSIZ(OFSFMT(dp), nep);
+		dsize = DIRSIZ(0, nep);
 		spacefree += nep->d_reclen - dsize;
 #ifdef UFS_DIRHASH
 		if (dp->i_dirhash != NULL)
@@ -1030,8 +1006,7 @@ ufs_dirrewrite(struct inode *dp, struct 
 	if (error)
 		return (error);
 	ep->d_ino = newinum;
-	if (!OFSFMT(dp))
-		ep->d_type = newtype;
+	ep->d_type = newtype;
 	oip->i_effnlink--;
 	if (DOINGSOFTDEP(vdp)) {
 		softdep_change_linkcnt(oip, 0);
@@ -1087,14 +1062,7 @@ ufs_dirempty(struct inode *ip, ufsino_t 
 		if (dp->d_ino == 0)
 			continue;
 		/* accept only "." and ".." */
-#		if (BYTE_ORDER == LITTLE_ENDIAN)
-			if (OFSFMT(ip))
-				namlen = dp->d_type;
-			else
-				namlen = dp->d_namlen;
-#		else
-			namlen = dp->d_namlen;
-#		endif
+		namlen = dp->d_namlen;
 		if (namlen > 2)
 			return (0);
 		if (dp->d_name[0] != '.')
@@ -1145,14 +1113,7 @@ ufs_checkpath(struct inode *source, stru
 			IO_NODELOCKED, cred, NULL, curproc);
 		if (error != 0)
 			break;
-#		if (BYTE_ORDER == LITTLE_ENDIAN)
-			if (OFSFMT(VTOI(vp)))
-				namlen = dirbuf.dotdot_type;
-			else
-				namlen = dirbuf.dotdot_namlen;
-#		else
-			namlen = dirbuf.dotdot_namlen;
-#		endif
+		namlen = dirbuf.dotdot_namlen;
 		if (namlen != 2 ||
 		    dirbuf.dotdot_name[0] != '.' ||
 		    dirbuf.dotdot_name[1] != '.') {
Index: sys/ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /data/src/openbsd/src/sys/ufs/ufs/ufs_vnops.c,v
diff -u -p -r1.158 ufs_vnops.c
--- sys/ufs/ufs/ufs_vnops.c	8 Sep 2023 20:00:28 -0000	1.158
+++ sys/ufs/ufs/ufs_vnops.c	2 Jan 2024 03:32:02 -0000
@@ -81,14 +81,10 @@ void filt_ufsdetach(struct knote *);
 /*
  * A virgin directory (no blushing please).
  */
-static struct dirtemplate mastertemplate = {
+static const struct dirtemplate mastertemplate = {
 	0, 12, DT_DIR, 1, ".",
 	0, DIRBLKSIZ - 12, DT_DIR, 2, ".."
 };
-static struct odirtemplate omastertemplate = {
-	0, 12, 1, ".",
-	0, DIRBLKSIZ - 12, 2, ".."
-};
 
 /*
  * Update the times in the inode
@@ -1127,7 +1123,7 @@ ufs_mkdir(void *v)
 	struct vnode *tvp;
 	struct buf *bp;
 	struct direct newdir;
-	struct dirtemplate dirtemplate, *dtp;
+	struct dirtemplate dirtemplate;
 	int error, dmode, blkoff;
 
 #ifdef DIAGNOSTIC
@@ -1187,11 +1183,7 @@ ufs_mkdir(void *v)
 	/* 
 	 * Initialize directory with "." and ".." from static template.
 	 */
-	if (dp->i_ump->um_maxsymlinklen > 0)
-		dtp = &mastertemplate;
-	else
-		dtp = (struct dirtemplate *)&omastertemplate;
-	dirtemplate = *dtp;
+	dirtemplate = mastertemplate;
 	dirtemplate.dot_ino = ip->i_number;
 	dirtemplate.dotdot_ino = dp->i_number;
 
@@ -1411,9 +1403,6 @@ ufs_readdir(void *v)
 	caddr_t diskbuf;
 	size_t count, entries;
 	int bufsize, readcnt, error;
-#if (BYTE_ORDER == LITTLE_ENDIAN)
-	int ofmt = VTOI(ap->a_vp)->i_ump->um_maxsymlinklen == 0;
-#endif
 
 	if (uio->uio_rw != UIO_READ)
 		return (EINVAL);
@@ -1468,16 +1457,8 @@ ufs_readdir(void *v)
 		off += dp->d_reclen;
 		u.dn.d_off = off;
 		u.dn.d_fileno = dp->d_ino;
-#if (BYTE_ORDER == LITTLE_ENDIAN)
-		if (ofmt) {
-			u.dn.d_type = dp->d_namlen;
-			u.dn.d_namlen = dp->d_type;
-		} else
-#endif
-		{
-			u.dn.d_type = dp->d_type;
-			u.dn.d_namlen = dp->d_namlen;
-		}
+		u.dn.d_type = dp->d_type;
+		u.dn.d_namlen = dp->d_namlen;
 		memcpy(u.dn.d_name, dp->d_name, u.dn.d_namlen);
 		memset(u.dn.d_name + u.dn.d_namlen, 0, u.dn.d_reclen
 		    - u.dn.d_namlen - offsetof(struct dirent, d_name));
@@ -1513,10 +1494,8 @@ ufs_readlink(void *v)
 	u_int64_t isize;
 
 	isize = DIP(ip, size);
-	if (isize < ip->i_ump->um_maxsymlinklen ||
-	    (ip->i_ump->um_maxsymlinklen == 0 && DIP(ip, blocks) == 0)) {
+	if (isize < ip->i_ump->um_maxsymlinklen)
 		return (uiomove((char *)SHORTLINK(ip), isize, ap->a_uio));
-	}
 	return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred));
 }