From: Ricardo Branco <rbranco@suse.de>
Subject: Re: ext2fs: Drop EXT2FS_SYSTEM_FLAGS and make its behaviour the default
To: tech@openbsd.org
Date: Fri, 18 Apr 2025 18:05:26 +0200

Sending updated patch as attachment.


Best,


On 1/29/24 7:50 PM, Ricardo Branco wrote:
> 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 a6ed33cc497..43bdb669b5c 100644
--- share/man/man4/options.4
+++ share/man/man4/options.4
@@ -171,26 +171,6 @@ standard file system used on many CD-ROMs.
 It also supports Joliet extensions.
 See
 .Xr mount_cd9660 8 .
-.It Cd option EXT2FS
-Includes code implementing the Second Extended File System
-.Em ( EXT2FS ) ,
-commonly used on the Linux operating system.
-This option is provided here for compatibility.
-Some specific features of
-.Em EXT2FS
-like the "behavior on errors" are not implemented.
-This file system
-can't be used with
-.Vt uid_t
-or
-.Vt gid_t
-values greater than 65535.
-Also, the filesystem will not function correctly on architectures with
-differing byte-orders.
-That is, a big-endian machine will not be able to read an
-ext2fs filesystem created on an i386 or other little-endian machine.
-See
-.Xr mount_ext2fs 8 .
 .It Cd option FFS
 Includes code implementing the Berkeley Fast File System
 .Em ( FFS ) .
diff --git sys/ufs/ext2fs/ext2fs_vnops.c sys/ufs/ext2fs/ext2fs_vnops.c
index c3f049e9993..46bc77f2fe4 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);