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);
}
For ntfs/ext4, you will need to implement on-disk support for
set/get the dosattrib flags.
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.
