Message-ID: <20250216210101.l4d6mm42pyopgrjj@pali>
Date: Sun, 16 Feb 2025 22:01:01 +0100
From: Pali Rohár <pali@...nel.org>
To: Amir Goldstein <amir73il@...il.com>
Cc: "Darrick J. Wong" <djwong@...nel.org>,
	ronnie sahlberg <ronniesahlberg@...il.com>,
	Chuck Lever <chuck.lever@...cle.com>,
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
	Steve French <sfrench@...ba.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-cifs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/4] fs: Extend FS_IOC_FS[GS]ETXATTR API for Windows
 attributes

On Sunday 16 February 2025 21:34:42 Amir Goldstein wrote:
> On Sun, Feb 16, 2025 at 9:01 PM Pali Rohár <pali@...nel.org> wrote:
> >
> > On Sunday 16 February 2025 20:55:09 Amir Goldstein wrote:
> > > On Sun, Feb 16, 2025 at 5:42 PM Pali Rohár <pali@...nel.org> wrote:
> > > >
> > > > struct fsxattr has 8 reserved padding bytes. Use these bytes for defining
> > > > new fields fsx_xflags2, fsx_xflags2_mask and fsx_xflags_mask in backward
> > > > compatible manner. If the new FS_XFLAG_HASEXTFIELDS flag in fsx_xflags is
> > > > not set then these new fields are treated as not present, like before this
> > > > change.
> > > >
> > > > New field fsx_xflags_mask for SET operation specifies which flags in
> > > > fsx_xflags are going to be changed. This would allow userspace application
> > > > to change just subset of all flags. For GET operation this field specifies
> > > > which FS_XFLAG_* flags are supported by the file.
> > > >
> > > > New field fsx_xflags2 specify new flags FS_XFLAG2_* which defines some of
> > > > Windows FILE_ATTRIBUTE_* attributes, which are mostly not going to be
> > > > interpreted or used by the kernel, and are mostly going to be used by
> > > > userspace. Field fsx_xflags2_mask then specify mask for them.
> > > >
> > > > This change defines just API without filesystem support for them. These
> > > > attributes can be implemented later for Windows filesystems like FAT, NTFS,
> > > > exFAT, UDF, SMB, NFS4 which all native storage for those attributes (or at
> > > > least some subset of them).
> > > >
> > > > Signed-off-by: Pali Rohár <pali@...nel.org>
> > > > ---
> > > >  include/uapi/linux/fs.h | 36 +++++++++++++++++++++++++++++++-----
> > > >  1 file changed, 31 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > > > index 367bc5289c47..93e947d6e604 100644
> > > > --- a/include/uapi/linux/fs.h
> > > > +++ b/include/uapi/linux/fs.h
> > > > @@ -145,15 +145,26 @@ struct fsxattr {
> > > >         __u32           fsx_nextents;   /* nextents field value (get)   */
> > > >         __u32           fsx_projid;     /* project identifier (get/set) */
> > > >         __u32           fsx_cowextsize; /* CoW extsize field value (get/set)*/
> > > > -       unsigned char   fsx_pad[8];
> > > > +       __u16           fsx_xflags2;    /* xflags2 field value (get/set)*/
> > > > +       __u16           fsx_xflags2_mask;/*mask for xflags2 (get/set)*/
> > > > +       __u32           fsx_xflags_mask;/* mask for xflags (get/set)*/
> > > > +       /*
> > > > +        * For FS_IOC_FSSETXATTR ioctl, fsx_xflags_mask and fsx_xflags2_mask
> > > > +        * fields specify which FS_XFLAG_* and FS_XFLAG2_* flags from fsx_xflags
> > > > +        * and fsx_xflags2 fields are going to be changed.
> > > > +        *
> > > > +        * For FS_IOC_FSGETXATTR ioctl, fsx_xflags_mask and fsx_xflags2_mask
> > > > +        * fields specify which FS_XFLAG_* and FS_XFLAG2_* flags are supported.
> > > > +        */
> > > >  };
> > > >
> > > >  /*
> > > > - * Flags for the fsx_xflags field
> > > > + * Flags for the fsx_xflags and fsx_xflags_mask fields
> > > >   */
> > > >  #define FS_XFLAG_REALTIME      0x00000001      /* data in realtime volume */
> > > >  #define FS_XFLAG_PREALLOC      0x00000002      /* preallocated file extents */
> > > > -#define FS_XFLAG_IMMUTABLE     0x00000008      /* file cannot be modified */
> > > > +#define FS_XFLAG_IMMUTABLEUSER 0x00000004      /* file cannot be modified, changing this bit does not require CAP_LINUX_IMMUTABLE, equivalent of Windows FILE_ATTRIBUTE_READONLY */
> > >
> > > So why not call it FS_XFLAG2_READONLY? IDGI
> >
> > Just because to show that these two flags are similar, just one is for
> > root (or CAP_LINUX_IMMUTABLE) and another is for normal user.
> >
> > For example FreeBSD has also both flags (one for root and one for user)
> > and uses names SF_IMMUTABLE and UF_IMMUTABLE.
> >
> > For me having FS_XFLAG_IMMUTABLE and FS_XFLAG2_READONLY sounds less
> > clear, and does not explain how these two flags differs.
> >
> 
> Yes, I understand, but I do not agree.
> 
> What is your goal here?
> 
> Do you want to implement FreeBSD UF_IMMUTABLE?
> maybe UF_APPEND as well?
> Did anyone ask for this functionality?
> Not that I know of.

None of those.

> The requirement is to implement an API to the functionality known
> as READONLY in SMB and NTFS. Right?

Yes. But Linux is already calling this functionality as "immutable", not
as "readonly". That is why I thought that using existing name is better
than inventing a new name for some existing functionality.

> TBH, I did not study the semantics of READONLY, but I had a
> strong feeling that if we looked closely, we will find that other things are
> possible to do with READONLY files that are not possible with IMMUTABLE
> files. So I asked ChatGPT and it told me that all these can be changed:
> 1. File Attributes (Hidden, System, Archive, or Indexed).
> 2. Permissions (ACL - Access Control List)
> 3. Timestamps
> 4. Alternate Data Streams (ADS)
> 
> I do not know if ChatGPT is correct

Do not trust ChatGPT.

Modification of main data stream (= file content) or alternate data
streams is not possible.

Changing of file or extended attributes is possible.
Timestamp is a file attribute. xattrs are extended attributes.

About ACL I was not sure. But now I tried it and changing ACL is
possible even with readonly attribute set.

So 1. 2. and 3. is possible to change. 4. not.

> but it also told me that a READONLY
> file can be deleted (without removing the flag first).

That is wrong. File cannot be deleted if the READONLY attribute is set.
You first need to clear the READONLY flag. This is one of the main point
of READONLY attribute.

For example cifs.ko for unlink syscall issue SMB removal and if it is
failing due to readonly attribute, it clears it and try removal again.

> This is very very far from what IS_IMMUTABLE is.
> IS_IMMUTABLE is immutable to any metadata change.

I was always looking at this readonly attribute as IMMUTABLE as it
prevents modification of file content and prevents unlinking the file.

For me it was always very similar to immutable. But if you think that
changing ACL and file/extended attributes is the reason why it should
not be called immutable, then I'm fine. Maybe it needs better
documentation and explanation what each of those two flags can and
cannot do.

> Thanks,
> Amir.

Powered by blists - more mailing lists