From: Ricardo Branco <rbranco@suse.de>
Subject: ext2fs: Drop EXT2FS_SYSTEM_FLAGS and make its behaviour the default
To: tech@openbsd.org
Date: Mon, 29 Jan 2024 19:50:40 +0100

On Linux, only root can set the immutable & append file flags. OpenBSD 
_should_ stick with those semantics.

The default behaviour allows the user to set the immutable/append flags, 
which is problematic for at least 2 reasons:

1. The user setting the flag when only root should be allowed.
1. When the user sets those flags, he cannot unset them later when 
running on Linux, FreeBSD (and perhaps other systems).  Only root.

FreeBSD solved this issue already in 2009:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=122047

Problem also reported to NetBSD, who recently committed the changes:
https://mail-index.netbsd.org/netbsd-bugs/2024/01/29/msg081371.html


Best,
R
diff --git share/man/man4/options.4 share/man/man4/options.4
index 3fdf8bd9453..43d01fb4df0 100644
--- share/man/man4/options.4
+++ share/man/man4/options.4
@@ -243,16 +243,6 @@ See
 .Bl -ohang
 .It Cd option BUFCACHEPERCENT= Ns Ar integer
 The maximum percentage of DMA-reachable physical memory the buffer cache may use.
-.It Cd option EXT2FS_SYSTEM_FLAGS
-This option changes the behavior of the APPEND and IMMUTABLE flags
-for a file on an
-.Em EXT2FS
-filesystem.
-Without this option, the superuser or owner of the file can set and clear them.
-With this option, only the superuser can set them, and they can't be cleared
-if the securelevel is greater than 0.
-See also
-.Xr chflags 1 .
 .It Cd option FFS_SOFTUPDATES
 Enables a scheme that uses partial ordering of buffer cache operations
 to allow metadata updates in FFS to happen asynchronously, increasing write
diff --git sys/ufs/ext2fs/ext2fs_vnops.c sys/ufs/ext2fs/ext2fs_vnops.c
index 235590d7c74..ad6f74ac004 100644
--- sys/ufs/ext2fs/ext2fs_vnops.c
+++ sys/ufs/ext2fs/ext2fs_vnops.c
@@ -183,13 +183,8 @@ ext2fs_getattr(void *v)
 	vap->va_mtime.tv_nsec = 0;
 	vap->va_ctime.tv_sec = ip->i_e2fs_ctime;
 	vap->va_ctime.tv_nsec = 0;
-#ifdef EXT2FS_SYSTEM_FLAGS
 	vap->va_flags = (ip->i_e2fs_flags & EXT2_APPEND) ? SF_APPEND : 0;
 	vap->va_flags |= (ip->i_e2fs_flags & EXT2_IMMUTABLE) ? SF_IMMUTABLE : 0;
-#else
-	vap->va_flags = (ip->i_e2fs_flags & EXT2_APPEND) ? UF_APPEND : 0;
-	vap->va_flags |= (ip->i_e2fs_flags & EXT2_IMMUTABLE) ? UF_IMMUTABLE : 0;
-#endif
 	vap->va_gen = ip->i_e2fs_gen;
 	/* this doesn't belong here */
 	if (vp->v_type == VBLK)
@@ -232,7 +227,6 @@ ext2fs_setattr(void *v)
 		if (cred->cr_uid != ip->i_e2fs_uid &&
 			(error = suser_ucred(cred)))
 			return (error);
-#ifdef EXT2FS_SYSTEM_FLAGS
 		if (cred->cr_uid == 0) {
 			if ((ip->i_e2fs_flags &
 			    (EXT2_APPEND | EXT2_IMMUTABLE)) && securelevel > 0)
@@ -244,12 +238,6 @@ ext2fs_setattr(void *v)
 		} else {
 			return (EPERM);
 		}
-#else
-		ip->i_e2fs_flags &= ~(EXT2_APPEND | EXT2_IMMUTABLE);
-		ip->i_e2fs_flags |=
-		    (vap->va_flags & UF_APPEND) ? EXT2_APPEND : 0 |
-		    (vap->va_flags & UF_IMMUTABLE) ? EXT2_IMMUTABLE: 0;
-#endif
 		ip->i_flag |= IN_CHANGE;
 		if (vap->va_flags & (IMMUTABLE | APPEND))
 			return (0);