On Tuesday 04 February 2025 00:02:44 Amir Goldstein wrote:
> On Mon, Feb 3, 2025 at 11:20 PM Pali Rohár <pali@xxxxxxxxxx> wrote:
> >
> > On Monday 03 February 2025 22:59:46 Amir Goldstein wrote:
> > > On Sun, Feb 2, 2025 at 4:23 PM Pali Rohár <pali@xxxxxxxxxx> wrote:
> > > > And there is still unresolved issue with FILE_ATTRIBUTE_READONLY.
> > > > Its meaning is similar to existing Linux FS_IMMUTABLE_FL, just
> > > > FILE_ATTRIBUTE_READONLY does not require root / CAP_LINUX_IMMUTABLE.
> > > >
> > > > I think that for proper support, to enforce FILE_ATTRIBUTE_READONLY
> > > > functionality, it is needed to introduce new flag e.g.
> > > > FS_IMMUTABLE_FL_USER to allow setting / clearing it also for normal
> > > > users without CAP_LINUX_IMMUTABLE. Otherwise it would be unsuitable for
> > > > any SMB client, SMB server or any application which would like to use
> > > > it, for example wine.
> > > >
> > > > Just to note that FreeBSD has two immutable flags SF_IMMUTABLE and
> > > > UF_IMMUTABLE, one settable only by superuser and second for owner.
> > > >
> > > > Any opinion?
> > >
> > > For filesystems that already support FILE_ATTRIBUTE_READONLY,
> > > can't you just set S_IMMUTABLE on the inode and vfs will do the correct
> > > enforcement?
> > >
> > > The vfs does not control if and how S_IMMUTABLE is set by filesystems,
> > > so if you want to remove this vfs flag without CAP_LINUX_IMMUTABLE
> > > in smb client, there is nothing stopping you (I think).
> >
> > Function fileattr_set_prepare() checks for CAP_LINUX_IMMUTABLE when
> > trying to change FS_IMMUTABLE_FL bit. This function is called from
> > ioctl(FS_IOC_SETFLAGS) and also from ioctl(FS_IOC_FSSETXATTR).
> > And when function fileattr_set_prepare() fails then .fileattr_set
> > callback is not called at all. So I think that it is not possible to
> > remove the IMMUTABLE flag from userspace without capability for smb
> > client.
> >
>
> You did not understand what I meant.
>
> You cannot relax the CAP_LINUX_IMMUTABLE for setting FS_IMMUTABLE_FL
> and there is no reason that you will need to relax it.
>
> The vfs does NOT enforce permissions according to FS_IMMUTABLE_FL
> The vfs enforces permissions according to the S_IMMUTABLE in-memory
> inode flag.
>
> There is no generic vfs code that sets S_IMMUTABLE inode flags, its
> the filesystems that translate the on-disk FS_IMMUTABLE_FL to
> in-memory S_IMMUTABLE inode flag.
>
> So if a filesystem already has an internal DOSATTRIB flags set, this
> filesystem can set the in-memory S_IMMUTABLE inode flag according
> to its knowledge of the DOSATTRIB_READONLY flag and the
> CAP_LINUX_IMMUTABLE rules do not apply to the DOSATTRIB_READONLY
> flag, which is NOT the same as the FS_IMMUTABLE_FL flag.
>
> > And it would not solve this problem for local filesystems (ntfs or ext4)
> > when Samba server or wine would want to set this bit.
> >
>
> The Samba server would use the FS_IOC_FS[GS]ETXATTR ioctl
> API to get/set dosattrib, something like this:
>
> struct fsxattr fsxattr;
> ret = ioctl_get_fsxattr(fd, &fsxattr);
> if (!ret && fsxattr.fsx_xflags & FS_XFLAG_HASDOSATTR) {
> fsxattr.fsx_dosattr |= fs_dosattrib_readonly;
> ret = ioctl_set_fsxattr(fd, &fsxattr);
> }
Thanks for more explanation. First time I really did not understood it.
But now I think I understood it. So basically there would be two flags
which would result in setting S_IMMUTABLE on inode. One is the existing
FS_IMMUTABLE_FL which requires the capability and some new flag (e.g.
FS_XFLAG_HASDOSATTR) which would not require it and can be implemented
for cifs, vfat, ntfs, ... Right?
> For ntfs/ext4, you will need to implement on-disk support for
> set/get the dosattrib flags.
ntfs has already on-disk support for FILE_ATTRIBUTE_READONLY.
On-disk support for ext4 and other linux filesystems can be discussed
later. I think that this could be more controversial.
> I can certainly not change the meaning of existing on-disk
> flag of FS_IMMUTABLE_FL to a flag that can be removed
> without CAP_LINUX_IMMUTABLE. that changes the meaning
> of the flag.
>
> If ext4 maintainers agrees, you may be able to reuse some
> old unused on-disk flags (e.g. EXT4_UNRM_FL) as storage
> place for FS_DOSATTRIB_READONLY, but that would be
> quite hackish.
>
> > > How about tackling this one small step at a time, not in that order
> > > necessarily:
> > >
> > > 1. Implement the standard API with FS_IOC_FS[GS]ETXATTR ioctl
> > > and with statx to get/set some non-controversial dosattrib flags on
> > > ntfs/smb/vfat
> > > 2. Wire some interesting dosattrib flags (e.g. compr/enrypt) to local
> > > filesystems that already support storing those bits
> > > 3. Wire network servers (e.g. Samba) to use the generic API if supported
> > > 4. Add on-disk support for storing the dosattrib flags to more local fs
> > > 5. Update S_IMMUTABLE inode flag if either FS_XFLAG_IMMUTABLE
> > > or FS_DOSATTRIB_READONLY are set on the file
> > >
> > > Thoughts?
> > >
>
> Anything wrong with the plan above?
> It seems that you are looking for shortcuts and I don't think that
> it is a good way to make progress.
>
> Thanks,
> Amir.
If other developers agree that the FS_IOC_FS[GS]ETXATTR ioctl is the
right direction then for me it looks good.
