ping

On 10/23/25 09:49, Martijn van Duren wrote:
> 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 - 0377483fd63d89703211981e944e9e01c48a851d
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 - 0377483fd63d89703211981e944e9e01c48a851d
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))