On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote: > From: Amir Goldstein <amir73il@xxxxxxxxx> > > We intend to add support for more xflags to selective filesystems and > We cannot rely on copy_struct_from_user() to detect this extension. > > In preparation of extending the API, do not allow setting xflags unknown > by this kernel version. > > Also do not pass the read-only flags and read-only field fsx_nextents to > filesystem. > > These changes should not affect existing chattr programs that use the > ioctl to get fsxattr before setting the new values. > > Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@xxxxxxxxxx/ > Cc: Pali Rohár <pali@xxxxxxxxxx> > Cc: Andrey Albershteyn <aalbersh@xxxxxxxxxx> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx> > --- > fs/file_attr.c | 8 +++++++- > include/linux/fileattr.h | 20 ++++++++++++++++++++ > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/fs/file_attr.c b/fs/file_attr.c > index 4e85fa00c092..62f08872d4ad 100644 > --- a/fs/file_attr.c > +++ b/fs/file_attr.c > @@ -99,9 +99,10 @@ EXPORT_SYMBOL(vfs_fileattr_get); > int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa) > { > struct fsxattr xfa; > + __u32 mask = FS_XFLAGS_MASK; > > memset(&xfa, 0, sizeof(xfa)); > - xfa.fsx_xflags = fa->fsx_xflags; > + xfa.fsx_xflags = fa->fsx_xflags & mask; I wonder, should it be an error if a filesystem sets an fsx_xflags bit outside of FS_XFLAGS_MASK? I guess that's one way to prevent filesystems from overriding the VFS bits. ;) Though couldn't that be: xfa.fsx_xflags = fa->fsx_xflags & FS_XFLAGS_MASK; instead? And same below? > xfa.fsx_extsize = fa->fsx_extsize; > xfa.fsx_nextents = fa->fsx_nextents; > xfa.fsx_projid = fa->fsx_projid; > @@ -118,11 +119,16 @@ static int copy_fsxattr_from_user(struct fileattr *fa, > struct fsxattr __user *ufa) > { > struct fsxattr xfa; > + __u32 mask = FS_XFLAGS_MASK; > > if (copy_from_user(&xfa, ufa, sizeof(xfa))) > return -EFAULT; > > + if (xfa.fsx_xflags & ~mask) > + return -EINVAL; I wonder if you want EOPNOTSUPP here? We don't know how to support unknown xflags. OTOH if you all have beaten this to death while I was out then don't start another round just for me. :P --D > + > fileattr_fill_xflags(fa, xfa.fsx_xflags); > + fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK; > fa->fsx_extsize = xfa.fsx_extsize; > fa->fsx_nextents = xfa.fsx_nextents; > fa->fsx_projid = xfa.fsx_projid; > diff --git a/include/linux/fileattr.h b/include/linux/fileattr.h > index 6030d0bf7ad3..e2a2f4ae242d 100644 > --- a/include/linux/fileattr.h > +++ b/include/linux/fileattr.h > @@ -14,6 +14,26 @@ > FS_XFLAG_NODUMP | FS_XFLAG_NOATIME | FS_XFLAG_DAX | \ > FS_XFLAG_PROJINHERIT) > > +/* Read-only inode flags */ > +#define FS_XFLAG_RDONLY_MASK \ > + (FS_XFLAG_PREALLOC | FS_XFLAG_HASATTR) > + > +/* Flags to indicate valid value of fsx_ fields */ > +#define FS_XFLAG_VALUES_MASK \ > + (FS_XFLAG_EXTSIZE | FS_XFLAG_COWEXTSIZE) > + > +/* Flags for directories */ > +#define FS_XFLAG_DIRONLY_MASK \ > + (FS_XFLAG_RTINHERIT | FS_XFLAG_NOSYMLINKS | FS_XFLAG_EXTSZINHERIT) > + > +/* Misc settable flags */ > +#define FS_XFLAG_MISC_MASK \ > + (FS_XFLAG_REALTIME | FS_XFLAG_NODEFRAG | FS_XFLAG_FILESTREAM) > + > +#define FS_XFLAGS_MASK \ > + (FS_XFLAG_COMMON | FS_XFLAG_RDONLY_MASK | FS_XFLAG_VALUES_MASK | \ > + FS_XFLAG_DIRONLY_MASK | FS_XFLAG_MISC_MASK) > + > /* > * Merged interface for miscellaneous file attributes. 'flags' originates from > * ext* and 'fsx_flags' from xfs. There's some overlap between the two, which > > -- > 2.47.2 > >
