From: Martijn van Duren <openbsd+tech@list.imperialat.at>
Subject: Re: Message when moving file to ext2
To: Walter Alejandro Iglesias <wai@roquesor.com>, tech@openbsd.org
Date: Thu, 23 Oct 2025 09:49:29 +0200

On Thu, 2025-10-23 at 02:14 +0200, Martijn van Duren wrote:
> On Wed, 2025-10-22 at 21:35 +0200, Walter Alejandro Iglesias wrote:
> > Hello list,
> > 
> > Is the message below expected when moving a file to an ext2 file system?
> > 
> >   $ doas mount -t ext2fs /dev/sd1i /mnt
> >   $ mv file /mnt
> >   mv: /mnt/file: set flags: Operation not permitted
> > 
> Moving to tech@
> 
> I ran into this issue some time ago with install(1). Back then I fixed
> it by making sure that install(1) doesn't call chflags(2) when the flags
> are identical.
> 
> The culprit for this warning is the following diff[0], which now always
> returns EPERM when a user tries to change flags (which we clearly aren't
> trying to do). Since I can't tell which other tools might bite us I
> think it might be wise to implement the logic I put into install(1) into
> the ext2fs driver.
> 
> The diff below works on my machine(tm). Could you give it a spin?
> 
> martijn@
> 
> ps. ext2 has a few more flags than our tools support[1]. But we could
> add nodump to the mapping, if only for a bit more complete insight.
> pps. Maybe it's a good idea to return EINVAL for UF_IMMUTABLE,
> UF_APPEND, and SF_ARCHIVED. That way it will be clear for the
> admin that certain flags aren't supported and won't be dropped
> silently. 
> 
> [0] https://marc.info/?l=openbsd-tech&m=174499208717194&w=2
> [1] https://wiki.osdev.org/Ext2#Inode_Flags
> 
After thinking about it a little more, with some early conversion of
the chflag(2) flags to their ext2fs equivalent it's quite easy to
follow the ufs_setattr() logic. This becomes more apparent when
UF_NODUMP is added.
This diff keeps the logic that unsupported flags in both the ext2fs
and chflags(2) sets are ignored.

martijn@

diff /usr/src
path + /usr/src
commit - 243c12c31b603d599722a28305b1c1bab6ea9d19
blob - 4357be5b9efc7b6a077251c58c1d91539b3d9dd5
file + sys/ufs/ext2fs/ext2fs_dinode.h
--- sys/ufs/ext2fs/ext2fs_dinode.h
+++ sys/ufs/ext2fs/ext2fs_dinode.h
@@ -135,6 +135,9 @@ struct ext2fs_dinode {
 #define EXT4_EXTENTS		0x00080000	/* inode uses extents */
 #define EXT4_EOFBLOCKS		0x00400000	/* blocks allocated beyond EOF */
 
+#define EXT2_SF_SETTABLE	(EXT2_IMMUTABLE | EXT2_APPEND)
+#define EXT2_UF_SETTABLE	(EXT2_NODUMP)
+
 /* Size of on-disk inode. */
 #define EXT2_REV0_DINODE_SIZE	128
 #define EXT2_DINODE_SIZE(fs)	((fs)->e2fs.e2fs_rev > E2FS_REV0 ?  \
commit - 243c12c31b603d599722a28305b1c1bab6ea9d19
blob - c0ffdeb9166c32d67d308129cd4c7c5b535cc620
file + sys/ufs/ext2fs/ext2fs_vnops.c
--- sys/ufs/ext2fs/ext2fs_vnops.c
+++ sys/ufs/ext2fs/ext2fs_vnops.c
@@ -185,6 +185,7 @@ ext2fs_getattr(void *v)
 	vap->va_ctime.tv_nsec = 0;
 	vap->va_flags = (ip->i_e2fs_flags & EXT2_APPEND) ? SF_APPEND : 0;
 	vap->va_flags |= (ip->i_e2fs_flags & EXT2_IMMUTABLE) ? SF_IMMUTABLE : 0;
+	vap->va_flags |= (ip->i_e2fs_flags & EXT2_NODUMP) ? UF_NODUMP : 0;
 	vap->va_gen = ip->i_e2fs_gen;
 	/* this doesn't belong here */
 	if (vp->v_type == VBLK)
@@ -210,6 +211,7 @@ ext2fs_setattr(void *v)
 	struct vnode *vp = ap->a_vp;
 	struct inode *ip = VTOI(vp);
 	struct ucred *cred = ap->a_cred;
+	u_long e2flags;
 	int error;
 
 	/*
@@ -227,16 +229,24 @@ ext2fs_setattr(void *v)
 		if (cred->cr_uid != ip->i_e2fs_uid &&
 			(error = suser_ucred(cred)))
 			return (error);
+		/* Ignore unsupported flags */
+		vap->va_flags &= (SF_APPEND | SF_IMMUTABLE | UF_NODUMP);
+		e2flags = (vap->va_flags & SF_APPEND) ? EXT2_APPEND : 0 |
+		    (vap->va_flags & SF_IMMUTABLE) ? EXT2_IMMUTABLE : 0 |
+		    (vap->va_flags & UF_NODUMP) ? EXT2_NODUMP : 0;
 		if (cred->cr_uid == 0) {
 			if ((ip->i_e2fs_flags &
 			    (EXT2_APPEND | EXT2_IMMUTABLE)) && securelevel > 0)
 				return (EPERM);
-			ip->i_e2fs_flags &= ~(EXT2_APPEND | EXT2_IMMUTABLE);
-			ip->i_e2fs_flags |=
-			    (vap->va_flags & SF_APPEND) ? EXT2_APPEND : 0 |
-			    (vap->va_flags & SF_IMMUTABLE) ? EXT2_IMMUTABLE: 0;
+			ip->i_e2fs_flags &=
+			    ~(EXT2_SF_SETTABLE | EXT2_UF_SETTABLE);
+			ip->i_e2fs_flags |= e2flags;
 		} else {
-			return (EPERM);
+			if (ip->i_e2fs_flags & (EXT2_IMMUTABLE | EXT2_APPEND) ||
+			    (vap->va_flags & UF_SETTABLE) != vap->va_flags)
+				return (EPERM);
+			ip->i_e2fs_flags &= EXT2_SF_SETTABLE;
+			ip->i_e2fs_flags |= e2flags & EXT2_UF_SETTABLE;
 		}
 		ip->i_flag |= IN_CHANGE;
 		if (vap->va_flags & (IMMUTABLE | APPEND))