This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
An xfstests test generic/766 with basic coverage is at:
https://github.com/alberand/xfstests/commits/b4/file-attr/
NAME
file_getattr/file_setattr - get/set filesystem inode attributes
SYNOPSIS
#include <sys/syscall.h> /* Definition of SYS_* constants */
#include <unistd.h>
long syscall(SYS_file_getattr, int dirfd, const char *pathname,
struct fsx_fileattr *fsx, size_t size,
unsigned int at_flags);
long syscall(SYS_file_setattr, int dirfd, const char *pathname,
struct fsx_fileattr *fsx, size_t size,
unsigned int at_flags);
Note: glibc doesn't provide for file_getattr()/file_setattr(),
use syscall(2) instead.
DESCRIPTION
The file_getattr()/file_setattr() are used to set extended file
attributes. These syscalls take dirfd in conjunction with the
pathname argument. The syscall then operates on inode opened
according to openat(2) semantics.
This is an alternative to FS_IOC_FSGETXATTR/FS_IOC_FSSETXATTR
ioctl with a difference that file don't need to be open as file
can be referenced with a path instead of fd. By having this one
can manipulated filesystem inode attributes not only on regular
files but also on special ones. This is not possible with
FS_IOC_FSSETXATTR ioctl as ioctl() can not be called on special
files directly for the filesystem inode.
at_flags can be set to AT_SYMLINK_NOFOLLOW or AT_EMPTY_PATH.
RETURN VALUE
On success, 0 is returned. On error, -1 is returned, and errno
is set to indicate the error.
ERRORS
EINVAL Invalid at_flag specified (only
AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH is
supported).
EINVAL Size was smaller than any known version of
struct fsx_fileattr.
EINVAL Invalid combination of parameters provided in
fsx_fileattr for this type of file.
E2BIG Size of input argument struct fsx_fileattr
is too big.
EBADF Invalid file descriptor was provided.
EPERM No permission to change this file.
EOPNOTSUPP Filesystem does not support setting attributes
on this type of inode
HISTORY
Added in Linux 6.16.
EXAMPLE
Create directory and file "mkdir ./dir && touch ./dir/foo" and then
execute the following program:
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <linux/fs.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>
#if !defined(SYS_file_getattr) && defined(__x86_64__)
#define SYS_file_getattr 468
#define SYS_file_setattr 469
struct fsx_fileattr {
__u32 fsx_xflags;
__u32 fsx_extsize;
__u32 fsx_nextents;
__u32 fsx_projid;
__u32 fsx_cowextsize;
};
#endif
int
main(int argc, char **argv) {
int dfd;
int error;
struct fsx_fileattr fsx;
dfd = open("./dir", O_RDONLY);
if (dfd == -1) {
printf("can not open ./dir");
return dfd;
}
error = syscall(SYS_file_getattr, dfd, "./foo", &fsx,
sizeof(struct fsx_fileattr), 0);
if (error) {
printf("can not call SYS_file_getattr: %s",
strerror(errno));
return error;
}
printf("./dir/foo flags: %d\n", fsx.fsx_xflags);
fsx.fsx_xflags |= FS_XFLAG_NODUMP;
error = syscall(SYS_file_setattr, dfd, "./foo", &fsx,
sizeof(struct fsx_fileattr), 0);
if (error) {
printf("can not call SYS_file_setattr: %s",
strerror(errno));
return error;
}
printf("./dir/foo flags: %d\n", fsx.fsx_xflags);
return error;
}
SEE ALSO
ioctl(2), ioctl_iflags(2), ioctl_xfs_fsgetxattr(2), openat(2)
---
Changes in v6:
- Update cover letter example and docs
- Applied __free() attribute for syscall stack objects
- Introduced struct fsx_fileattr
- Replace 'struct fsxattr' with 'struct fsx_fileattr'
- Add helper to fill in fsx_fileattr from fileattr
- Dropped copy_fsx_to_user() header declaration
- Link to v5: https://lore.kernel.org/r/20250513-xattrat-syscall-v5-0-22bb9c6c767f@kernel.org
Changes in v5:
- Remove setting of LOOKUP_EMPTY flags which does not have any effect
- Return -ENOSUPP from vfs_fileattr_set()
- Add fsxattr masking (by Amir)
- Fix UAF issue dentry
- Fix getname_maybe_null() issue with NULL path
- Implement file_getattr/file_setattr hooks
- Return LSM return code from file_setattr
- Rename from getfsxattrat/setfsxattrat to file_getattr/file_setattr
- Link to v4: https://lore.kernel.org/r/20250321-xattrat-syscall-v4-0-3e82e6fb3264@kernel.org
Changes in v4:
- Use getname_maybe_null() for correct handling of dfd + path semantic
- Remove restriction for special files on which flags are allowed
- Utilize copy_struct_from_user() for better future compatibility
- Add draft man page to cover letter
- Convert -ENOIOCTLCMD to -EOPNOSUPP as more appropriate for syscall
- Add missing __user to header declaration of syscalls
- Link to v3: https://lore.kernel.org/r/20250211-xattrat-syscall-v3-1-a07d15f898b2@kernel.org
Changes in v3:
- Remove unnecessary "dfd is dir" check as it checked in user_path_at()
- Remove unnecessary "same filesystem" check
- Use CLASS() instead of directly calling fdget/fdput
- Link to v2: https://lore.kernel.org/r/20250122-xattrat-syscall-v2-1-5b360d4fbcb2@kernel.org
v1:
https://lore.kernel.org/linuxppc-dev/20250109174540.893098-1-aalbersh@kernel.org/
Previous discussion:
https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/
---
Amir Goldstein (1):
fs: prepare for extending file_get/setattr()
Andrey Albershteyn (5):
fs: split fileattr related helpers into separate file
lsm: introduce new hooks for setting/getting inode fsxattr
selinux: implement inode_file_[g|s]etattr hooks
fs: make vfs_fileattr_[get|set] return -EOPNOSUPP
fs: introduce file_getattr and file_setattr syscalls
arch/alpha/kernel/syscalls/syscall.tbl | 2 +
arch/arm/tools/syscall.tbl | 2 +
arch/arm64/tools/syscall_32.tbl | 2 +
arch/m68k/kernel/syscalls/syscall.tbl | 2 +
arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
arch/parisc/kernel/syscalls/syscall.tbl | 2 +
arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
arch/s390/kernel/syscalls/syscall.tbl | 2 +
arch/sh/kernel/syscalls/syscall.tbl | 2 +
arch/sparc/kernel/syscalls/syscall.tbl | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 2 +
arch/x86/entry/syscalls/syscall_64.tbl | 2 +
arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
fs/Makefile | 3 +-
fs/ecryptfs/inode.c | 8 +-
fs/file_attr.c | 493 ++++++++++++++++++++++++++++
fs/ioctl.c | 309 -----------------
fs/overlayfs/inode.c | 2 +-
include/linux/fileattr.h | 24 ++
include/linux/lsm_hook_defs.h | 2 +
include/linux/security.h | 16 +
include/linux/syscalls.h | 6 +
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/fs.h | 18 +
scripts/syscall.tbl | 2 +
security/security.c | 30 ++
security/selinux/hooks.c | 14 +
30 files changed, 654 insertions(+), 313 deletions(-)
---
base-commit: d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af
change-id: 20250114-xattrat-syscall-6a1136d2db59
Best regards,
--
Andrey Albershteyn [off-list ref]
@@ -0,0 +1,318 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/fs.h>+#include<linux/security.h>+#include<linux/fscrypt.h>+#include<linux/fileattr.h>++/**+*fileattr_fill_xflags-initializefileattrwithxflags+*@fa:fileattrpointer+*@xflags:FS_XFLAG_*flags+*+*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All+*otherfieldsarezeroed.+*/+voidfileattr_fill_xflags(structfileattr*fa,u32xflags)+{+memset(fa,0,sizeof(*fa));+fa->fsx_valid=true;+fa->fsx_xflags=xflags;+if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)+fa->flags|=FS_IMMUTABLE_FL;+if(fa->fsx_xflags&FS_XFLAG_APPEND)+fa->flags|=FS_APPEND_FL;+if(fa->fsx_xflags&FS_XFLAG_SYNC)+fa->flags|=FS_SYNC_FL;+if(fa->fsx_xflags&FS_XFLAG_NOATIME)+fa->flags|=FS_NOATIME_FL;+if(fa->fsx_xflags&FS_XFLAG_NODUMP)+fa->flags|=FS_NODUMP_FL;+if(fa->fsx_xflags&FS_XFLAG_DAX)+fa->flags|=FS_DAX_FL;+if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)+fa->flags|=FS_PROJINHERIT_FL;+}+EXPORT_SYMBOL(fileattr_fill_xflags);++/**+*fileattr_fill_flags-initializefileattrwithflags+*@fa:fileattrpointer+*@flags:FS_*_FLflags+*+*Set->flags,->flags_validand->fsx_xflags(translatedflags).+*Allotherfieldsarezeroed.+*/+voidfileattr_fill_flags(structfileattr*fa,u32flags)+{+memset(fa,0,sizeof(*fa));+fa->flags_valid=true;+fa->flags=flags;+if(fa->flags&FS_SYNC_FL)+fa->fsx_xflags|=FS_XFLAG_SYNC;+if(fa->flags&FS_IMMUTABLE_FL)+fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;+if(fa->flags&FS_APPEND_FL)+fa->fsx_xflags|=FS_XFLAG_APPEND;+if(fa->flags&FS_NODUMP_FL)+fa->fsx_xflags|=FS_XFLAG_NODUMP;+if(fa->flags&FS_NOATIME_FL)+fa->fsx_xflags|=FS_XFLAG_NOATIME;+if(fa->flags&FS_DAX_FL)+fa->fsx_xflags|=FS_XFLAG_DAX;+if(fa->flags&FS_PROJINHERIT_FL)+fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;+}+EXPORT_SYMBOL(fileattr_fill_flags);++/**+*vfs_fileattr_get-retrievemiscellaneousfileattributes+*@dentry:theobjecttoretrievefrom+*@fa:fileattrpointer+*+*Calli_op->fileattr_get()callback,ifexists.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)+{+structinode*inode=d_inode(dentry);++if(!inode->i_op->fileattr_get)+return-ENOIOCTLCMD;++returninode->i_op->fileattr_get(dentry,fa);+}+EXPORT_SYMBOL(vfs_fileattr_get);++/**+*copy_fsxattr_to_user-copyfsxattrtouserspace.+*@fa:fileattrpointer+*@ufa:fsxattruserpointer+*+*Return:0onsuccess,or-EFAULTonfailure.+*/+intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)+{+structfsxattrxfa;++memset(&xfa,0,sizeof(xfa));+xfa.fsx_xflags=fa->fsx_xflags;+xfa.fsx_extsize=fa->fsx_extsize;+xfa.fsx_nextents=fa->fsx_nextents;+xfa.fsx_projid=fa->fsx_projid;+xfa.fsx_cowextsize=fa->fsx_cowextsize;++if(copy_to_user(ufa,&xfa,sizeof(xfa)))+return-EFAULT;++return0;+}+EXPORT_SYMBOL(copy_fsxattr_to_user);++staticintcopy_fsxattr_from_user(structfileattr*fa,+structfsxattr__user*ufa)+{+structfsxattrxfa;++if(copy_from_user(&xfa,ufa,sizeof(xfa)))+return-EFAULT;++fileattr_fill_xflags(fa,xfa.fsx_xflags);+fa->fsx_extsize=xfa.fsx_extsize;+fa->fsx_nextents=xfa.fsx_nextents;+fa->fsx_projid=xfa.fsx_projid;+fa->fsx_cowextsize=xfa.fsx_cowextsize;++return0;+}++/*+*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject+*anyinvalidconfigurations.+*+*Note:mustbecalledwithinodelockheld.+*/+staticintfileattr_set_prepare(structinode*inode,+conststructfileattr*old_ma,+structfileattr*fa)+{+interr;++/*+*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby+*therelevantcapability.+*/+if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&+!capable(CAP_LINUX_IMMUTABLE))+return-EPERM;++err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);+if(err)+returnerr;++/*+*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit+*namespace.Enforcethatrestrictiononlyifwearetryingtochange+*thequotaIDstate.Everythingelseisallowedinusernamespaces.+*/+if(current_user_ns()!=&init_user_ns){+if(old_ma->fsx_projid!=fa->fsx_projid)+return-EINVAL;+if((old_ma->fsx_xflags^fa->fsx_xflags)&+FS_XFLAG_PROJINHERIT)+return-EINVAL;+}else{+/*+*CallerisallowedtochangetheprojectID.Ifitisbeing+*changed,makesurethatthenewvalueisvalid.+*/+if(old_ma->fsx_projid!=fa->fsx_projid&&+!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))+return-EINVAL;+}++/* Check extent size hints. */+if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&+!S_ISDIR(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&+!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))+return-EINVAL;++/*+*ItisonlyvalidtosettheDAXflagonregularfilesand+*directoriesonfilesystems.+*/+if((fa->fsx_xflags&FS_XFLAG_DAX)&&+!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))+return-EINVAL;++/* Extent size hints of zero turn off the flags. */+if(fa->fsx_extsize==0)+fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);+if(fa->fsx_cowextsize==0)+fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;++return0;+}++/**+*vfs_fileattr_set-changemiscellaneousfileattributes+*@idmap:idmapofthemount+*@dentry:theobjecttochange+*@fa:fileattrpointer+*+*Afterverifyingpermissions,calli_op->fileattr_set()callback,if+*exists.+*+*Verifyingattributesinvolvesretrievingcurrentattributeswith+*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave+*notbeensetbythecallertocurrentvalues.Inodelockisheld+*thoughouttopreventracingwithanotherinstance.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,+structfileattr*fa)+{+structinode*inode=d_inode(dentry);+structfileattrold_ma={};+interr;++if(!inode->i_op->fileattr_set)+return-ENOIOCTLCMD;++if(!inode_owner_or_capable(idmap,inode))+return-EPERM;++inode_lock(inode);+err=vfs_fileattr_get(dentry,&old_ma);+if(!err){+/* initialize missing bits from old_ma */+if(fa->flags_valid){+fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;+fa->fsx_extsize=old_ma.fsx_extsize;+fa->fsx_nextents=old_ma.fsx_nextents;+fa->fsx_projid=old_ma.fsx_projid;+fa->fsx_cowextsize=old_ma.fsx_cowextsize;+}else{+fa->flags|=old_ma.flags&~FS_COMMON_FL;+}+err=fileattr_set_prepare(inode,&old_ma,fa);+if(!err)+err=inode->i_op->fileattr_set(idmap,dentry,fa);+}+inode_unlock(inode);++returnerr;+}+EXPORT_SYMBOL(vfs_fileattr_set);++intioctl_getflags(structfile*file,unsignedint__user*argp)+{+structfileattrfa={.flags_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=put_user(fa.flags,argp);+returnerr;+}+EXPORT_SYMBOL(ioctl_getflags);++intioctl_setflags(structfile*file,unsignedint__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+unsignedintflags;+interr;++err=get_user(flags,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+fileattr_fill_flags(&fa,flags);+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_setflags);++intioctl_fsgetxattr(structfile*file,void__user*argp)+{+structfileattrfa={.fsx_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=copy_fsxattr_to_user(&fa,argp);++returnerr;+}+EXPORT_SYMBOL(ioctl_fsgetxattr);++intioctl_fssetxattr(structfile*file,void__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+interr;++err=copy_fsxattr_from_user(&fa,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_fssetxattr);
@@ -453,315 +453,6 @@ static int ioctl_file_dedupe_range(struct file *file,returnret;}-/**-*fileattr_fill_xflags-initializefileattrwithxflags-*@fa:fileattrpointer-*@xflags:FS_XFLAG_*flags-*-*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All-*otherfieldsarezeroed.-*/-voidfileattr_fill_xflags(structfileattr*fa,u32xflags)-{-memset(fa,0,sizeof(*fa));-fa->fsx_valid=true;-fa->fsx_xflags=xflags;-if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)-fa->flags|=FS_IMMUTABLE_FL;-if(fa->fsx_xflags&FS_XFLAG_APPEND)-fa->flags|=FS_APPEND_FL;-if(fa->fsx_xflags&FS_XFLAG_SYNC)-fa->flags|=FS_SYNC_FL;-if(fa->fsx_xflags&FS_XFLAG_NOATIME)-fa->flags|=FS_NOATIME_FL;-if(fa->fsx_xflags&FS_XFLAG_NODUMP)-fa->flags|=FS_NODUMP_FL;-if(fa->fsx_xflags&FS_XFLAG_DAX)-fa->flags|=FS_DAX_FL;-if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)-fa->flags|=FS_PROJINHERIT_FL;-}-EXPORT_SYMBOL(fileattr_fill_xflags);--/**-*fileattr_fill_flags-initializefileattrwithflags-*@fa:fileattrpointer-*@flags:FS_*_FLflags-*-*Set->flags,->flags_validand->fsx_xflags(translatedflags).-*Allotherfieldsarezeroed.-*/-voidfileattr_fill_flags(structfileattr*fa,u32flags)-{-memset(fa,0,sizeof(*fa));-fa->flags_valid=true;-fa->flags=flags;-if(fa->flags&FS_SYNC_FL)-fa->fsx_xflags|=FS_XFLAG_SYNC;-if(fa->flags&FS_IMMUTABLE_FL)-fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;-if(fa->flags&FS_APPEND_FL)-fa->fsx_xflags|=FS_XFLAG_APPEND;-if(fa->flags&FS_NODUMP_FL)-fa->fsx_xflags|=FS_XFLAG_NODUMP;-if(fa->flags&FS_NOATIME_FL)-fa->fsx_xflags|=FS_XFLAG_NOATIME;-if(fa->flags&FS_DAX_FL)-fa->fsx_xflags|=FS_XFLAG_DAX;-if(fa->flags&FS_PROJINHERIT_FL)-fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;-}-EXPORT_SYMBOL(fileattr_fill_flags);--/**-*vfs_fileattr_get-retrievemiscellaneousfileattributes-*@dentry:theobjecttoretrievefrom-*@fa:fileattrpointer-*-*Calli_op->fileattr_get()callback,ifexists.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)-{-structinode*inode=d_inode(dentry);--if(!inode->i_op->fileattr_get)-return-ENOIOCTLCMD;--returninode->i_op->fileattr_get(dentry,fa);-}-EXPORT_SYMBOL(vfs_fileattr_get);--/**-*copy_fsxattr_to_user-copyfsxattrtouserspace.-*@fa:fileattrpointer-*@ufa:fsxattruserpointer-*-*Return:0onsuccess,or-EFAULTonfailure.-*/-intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)-{-structfsxattrxfa;--memset(&xfa,0,sizeof(xfa));-xfa.fsx_xflags=fa->fsx_xflags;-xfa.fsx_extsize=fa->fsx_extsize;-xfa.fsx_nextents=fa->fsx_nextents;-xfa.fsx_projid=fa->fsx_projid;-xfa.fsx_cowextsize=fa->fsx_cowextsize;--if(copy_to_user(ufa,&xfa,sizeof(xfa)))-return-EFAULT;--return0;-}-EXPORT_SYMBOL(copy_fsxattr_to_user);--staticintcopy_fsxattr_from_user(structfileattr*fa,-structfsxattr__user*ufa)-{-structfsxattrxfa;--if(copy_from_user(&xfa,ufa,sizeof(xfa)))-return-EFAULT;--fileattr_fill_xflags(fa,xfa.fsx_xflags);-fa->fsx_extsize=xfa.fsx_extsize;-fa->fsx_nextents=xfa.fsx_nextents;-fa->fsx_projid=xfa.fsx_projid;-fa->fsx_cowextsize=xfa.fsx_cowextsize;--return0;-}--/*-*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject-*anyinvalidconfigurations.-*-*Note:mustbecalledwithinodelockheld.-*/-staticintfileattr_set_prepare(structinode*inode,-conststructfileattr*old_ma,-structfileattr*fa)-{-interr;--/*-*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby-*therelevantcapability.-*/-if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&-!capable(CAP_LINUX_IMMUTABLE))-return-EPERM;--err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);-if(err)-returnerr;--/*-*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit-*namespace.Enforcethatrestrictiononlyifwearetryingtochange-*thequotaIDstate.Everythingelseisallowedinusernamespaces.-*/-if(current_user_ns()!=&init_user_ns){-if(old_ma->fsx_projid!=fa->fsx_projid)-return-EINVAL;-if((old_ma->fsx_xflags^fa->fsx_xflags)&-FS_XFLAG_PROJINHERIT)-return-EINVAL;-}else{-/*-*CallerisallowedtochangetheprojectID.Ifitisbeing-*changed,makesurethatthenewvalueisvalid.-*/-if(old_ma->fsx_projid!=fa->fsx_projid&&-!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))-return-EINVAL;-}--/* Check extent size hints. */-if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&-!S_ISDIR(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&-!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))-return-EINVAL;--/*-*ItisonlyvalidtosettheDAXflagonregularfilesand-*directoriesonfilesystems.-*/-if((fa->fsx_xflags&FS_XFLAG_DAX)&&-!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))-return-EINVAL;--/* Extent size hints of zero turn off the flags. */-if(fa->fsx_extsize==0)-fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);-if(fa->fsx_cowextsize==0)-fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;--return0;-}--/**-*vfs_fileattr_set-changemiscellaneousfileattributes-*@idmap:idmapofthemount-*@dentry:theobjecttochange-*@fa:fileattrpointer-*-*Afterverifyingpermissions,calli_op->fileattr_set()callback,if-*exists.-*-*Verifyingattributesinvolvesretrievingcurrentattributeswith-*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave-*notbeensetbythecallertocurrentvalues.Inodelockisheld-*thoughouttopreventracingwithanotherinstance.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,-structfileattr*fa)-{-structinode*inode=d_inode(dentry);-structfileattrold_ma={};-interr;--if(!inode->i_op->fileattr_set)-return-ENOIOCTLCMD;--if(!inode_owner_or_capable(idmap,inode))-return-EPERM;--inode_lock(inode);-err=vfs_fileattr_get(dentry,&old_ma);-if(!err){-/* initialize missing bits from old_ma */-if(fa->flags_valid){-fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;-fa->fsx_extsize=old_ma.fsx_extsize;-fa->fsx_nextents=old_ma.fsx_nextents;-fa->fsx_projid=old_ma.fsx_projid;-fa->fsx_cowextsize=old_ma.fsx_cowextsize;-}else{-fa->flags|=old_ma.flags&~FS_COMMON_FL;-}-err=fileattr_set_prepare(inode,&old_ma,fa);-if(!err)-err=inode->i_op->fileattr_set(idmap,dentry,fa);-}-inode_unlock(inode);--returnerr;-}-EXPORT_SYMBOL(vfs_fileattr_set);--staticintioctl_getflags(structfile*file,unsignedint__user*argp)-{-structfileattrfa={.flags_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=put_user(fa.flags,argp);-returnerr;-}--staticintioctl_setflags(structfile*file,unsignedint__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-unsignedintflags;-interr;--err=get_user(flags,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-fileattr_fill_flags(&fa,flags);-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}--staticintioctl_fsgetxattr(structfile*file,void__user*argp)-{-structfileattrfa={.fsx_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=copy_fsxattr_to_user(&fa,argp);--returnerr;-}--staticintioctl_fssetxattr(structfile*file,void__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-interr;--err=copy_fsxattr_from_user(&fa,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}-staticintioctl_getfsuuid(structfile*file,void__user*argp){structsuper_block*sb=file_inode(file)->i_sb;
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ecryptfs/inode.c | 8 +++++++-
fs/file_attr.c | 12 ++++++++++--
fs/overlayfs/inode.c | 2 +-
3 files changed, 18 insertions(+), 4 deletions(-)
These hooks are called on inode extended attribute retrieval/change.
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
security/selinux/hooks.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
From: Andrey Albershteyn <redacted>
Introduce file_getattr() and file_setattr() syscalls to manipulate inode
extended attributes. The syscalls takes pair of file descriptor and
pathname. Then it operates on inode opened accroding to openat()
semantics. The struct fsx_fileattr is passed to obtain/change extended
attributes.
This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
that file don't need to be open as we can reference it with a path
instead of fd. By having this we can manipulated inode extended
attributes not only on regular files but also on special ones. This
is not possible with FS_IOC_FSSETXATTR ioctl as with special files
we can not call ioctl() directly on the filesystem inode using fd.
This patch adds two new syscalls which allows userspace to get/set
extended inode attributes on special files by using parent directory
and a path - *at() like syscall.
CC: linux-api@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-xfs@vger.kernel.org
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/alpha/kernel/syscalls/syscall.tbl | 2 +
arch/arm/tools/syscall.tbl | 2 +
arch/arm64/tools/syscall_32.tbl | 2 +
arch/m68k/kernel/syscalls/syscall.tbl | 2 +
arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
arch/parisc/kernel/syscalls/syscall.tbl | 2 +
arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
arch/s390/kernel/syscalls/syscall.tbl | 2 +
arch/sh/kernel/syscalls/syscall.tbl | 2 +
arch/sparc/kernel/syscalls/syscall.tbl | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 2 +
arch/x86/entry/syscalls/syscall_64.tbl | 2 +
arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
fs/file_attr.c | 148 ++++++++++++++++++++++++++++
include/linux/syscalls.h | 6 ++
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/fs.h | 18 ++++
scripts/syscall.tbl | 2 +
21 files changed, 213 insertions(+), 1 deletion(-)
@@ -507,3 +507,5 @@ 575 common listxattrat sys_listxattrat 576 common removexattrat sys_removexattrat 577 common open_tree_attr sys_open_tree_attr+578 common file_getattr sys_file_getattr+579 common file_setattr sys_file_setattr
@@ -482,3 +482,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -479,3 +479,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -467,3 +467,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -473,3 +473,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -466,3 +466,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -558,3 +558,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -471,3 +471,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -513,3 +513,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -391,6 +391,8 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr # # Due to a historical design error, certain syscalls are numbered differently
@@ -438,3 +438,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -148,6 +148,24 @@ struct fsxattr {unsignedcharfsx_pad[8];};+/*+*Variablesizestructureforfile_[sg]et_attr().+*+*Note.Thisisalternativetothestructure'structfileattr'/'structfsxattr'.+*Asthisstructureispassedto/fromuserspacewithitssize,thiscan+*beversionedbasedonthesize.+*/+structfsx_fileattr{+__u32fsx_xflags;/* xflags field value (get/set) */+__u32fsx_extsize;/* extsize field value (get/set)*/+__u32fsx_nextents;/* nextents field value (get) */+__u32fsx_projid;/* project identifier (get/set) */+__u32fsx_cowextsize;/* CoW extsize field value (get/set) */+};++#define FSX_FILEATTR_SIZE_VER0 20+#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0+/**Flagsforthefsx_xflagsfield*/
@@ -408,3 +408,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
nit: typo in commit subject and description: Missing T in EOPNO*T*SUPP.
But please do not resend whole patch series just because of this.
That is not needed.
On Monday 30 June 2025 18:20:14 Andrey Albershteyn wrote:
quoted hunk
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ecryptfs/inode.c | 8 +++++++-
fs/file_attr.c | 12 ++++++++++--
fs/overlayfs/inode.c | 2 +-
3 files changed, 18 insertions(+), 4 deletions(-)
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-01 05:39:21
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn [off-list ref] wrote:
From: Andrey Albershteyn <aalbersh@kernel.org>
This patch moves function related to file extended attributes
manipulations to separate file. Refactoring only.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
@@ -0,0 +1,318 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/fs.h>+#include<linux/security.h>+#include<linux/fscrypt.h>+#include<linux/fileattr.h>++/**+*fileattr_fill_xflags-initializefileattrwithxflags+*@fa:fileattrpointer+*@xflags:FS_XFLAG_*flags+*+*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All+*otherfieldsarezeroed.+*/+voidfileattr_fill_xflags(structfileattr*fa,u32xflags)+{+memset(fa,0,sizeof(*fa));+fa->fsx_valid=true;+fa->fsx_xflags=xflags;+if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)+fa->flags|=FS_IMMUTABLE_FL;+if(fa->fsx_xflags&FS_XFLAG_APPEND)+fa->flags|=FS_APPEND_FL;+if(fa->fsx_xflags&FS_XFLAG_SYNC)+fa->flags|=FS_SYNC_FL;+if(fa->fsx_xflags&FS_XFLAG_NOATIME)+fa->flags|=FS_NOATIME_FL;+if(fa->fsx_xflags&FS_XFLAG_NODUMP)+fa->flags|=FS_NODUMP_FL;+if(fa->fsx_xflags&FS_XFLAG_DAX)+fa->flags|=FS_DAX_FL;+if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)+fa->flags|=FS_PROJINHERIT_FL;+}+EXPORT_SYMBOL(fileattr_fill_xflags);++/**+*fileattr_fill_flags-initializefileattrwithflags+*@fa:fileattrpointer+*@flags:FS_*_FLflags+*+*Set->flags,->flags_validand->fsx_xflags(translatedflags).+*Allotherfieldsarezeroed.+*/+voidfileattr_fill_flags(structfileattr*fa,u32flags)+{+memset(fa,0,sizeof(*fa));+fa->flags_valid=true;+fa->flags=flags;+if(fa->flags&FS_SYNC_FL)+fa->fsx_xflags|=FS_XFLAG_SYNC;+if(fa->flags&FS_IMMUTABLE_FL)+fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;+if(fa->flags&FS_APPEND_FL)+fa->fsx_xflags|=FS_XFLAG_APPEND;+if(fa->flags&FS_NODUMP_FL)+fa->fsx_xflags|=FS_XFLAG_NODUMP;+if(fa->flags&FS_NOATIME_FL)+fa->fsx_xflags|=FS_XFLAG_NOATIME;+if(fa->flags&FS_DAX_FL)+fa->fsx_xflags|=FS_XFLAG_DAX;+if(fa->flags&FS_PROJINHERIT_FL)+fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;+}+EXPORT_SYMBOL(fileattr_fill_flags);++/**+*vfs_fileattr_get-retrievemiscellaneousfileattributes+*@dentry:theobjecttoretrievefrom+*@fa:fileattrpointer+*+*Calli_op->fileattr_get()callback,ifexists.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)+{+structinode*inode=d_inode(dentry);++if(!inode->i_op->fileattr_get)+return-ENOIOCTLCMD;++returninode->i_op->fileattr_get(dentry,fa);+}+EXPORT_SYMBOL(vfs_fileattr_get);++/**+*copy_fsxattr_to_user-copyfsxattrtouserspace.+*@fa:fileattrpointer+*@ufa:fsxattruserpointer+*+*Return:0onsuccess,or-EFAULTonfailure.+*/+intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)+{+structfsxattrxfa;++memset(&xfa,0,sizeof(xfa));+xfa.fsx_xflags=fa->fsx_xflags;+xfa.fsx_extsize=fa->fsx_extsize;+xfa.fsx_nextents=fa->fsx_nextents;+xfa.fsx_projid=fa->fsx_projid;+xfa.fsx_cowextsize=fa->fsx_cowextsize;++if(copy_to_user(ufa,&xfa,sizeof(xfa)))+return-EFAULT;++return0;+}+EXPORT_SYMBOL(copy_fsxattr_to_user);++staticintcopy_fsxattr_from_user(structfileattr*fa,+structfsxattr__user*ufa)+{+structfsxattrxfa;++if(copy_from_user(&xfa,ufa,sizeof(xfa)))+return-EFAULT;++fileattr_fill_xflags(fa,xfa.fsx_xflags);+fa->fsx_extsize=xfa.fsx_extsize;+fa->fsx_nextents=xfa.fsx_nextents;+fa->fsx_projid=xfa.fsx_projid;+fa->fsx_cowextsize=xfa.fsx_cowextsize;++return0;+}++/*+*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject+*anyinvalidconfigurations.+*+*Note:mustbecalledwithinodelockheld.+*/+staticintfileattr_set_prepare(structinode*inode,+conststructfileattr*old_ma,+structfileattr*fa)+{+interr;++/*+*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby+*therelevantcapability.+*/+if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&+!capable(CAP_LINUX_IMMUTABLE))+return-EPERM;++err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);+if(err)+returnerr;++/*+*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit+*namespace.Enforcethatrestrictiononlyifwearetryingtochange+*thequotaIDstate.Everythingelseisallowedinusernamespaces.+*/+if(current_user_ns()!=&init_user_ns){+if(old_ma->fsx_projid!=fa->fsx_projid)+return-EINVAL;+if((old_ma->fsx_xflags^fa->fsx_xflags)&+FS_XFLAG_PROJINHERIT)+return-EINVAL;+}else{+/*+*CallerisallowedtochangetheprojectID.Ifitisbeing+*changed,makesurethatthenewvalueisvalid.+*/+if(old_ma->fsx_projid!=fa->fsx_projid&&+!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))+return-EINVAL;+}++/* Check extent size hints. */+if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&+!S_ISDIR(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&+!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))+return-EINVAL;++/*+*ItisonlyvalidtosettheDAXflagonregularfilesand+*directoriesonfilesystems.+*/+if((fa->fsx_xflags&FS_XFLAG_DAX)&&+!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))+return-EINVAL;++/* Extent size hints of zero turn off the flags. */+if(fa->fsx_extsize==0)+fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);+if(fa->fsx_cowextsize==0)+fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;++return0;+}++/**+*vfs_fileattr_set-changemiscellaneousfileattributes+*@idmap:idmapofthemount+*@dentry:theobjecttochange+*@fa:fileattrpointer+*+*Afterverifyingpermissions,calli_op->fileattr_set()callback,if+*exists.+*+*Verifyingattributesinvolvesretrievingcurrentattributeswith+*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave+*notbeensetbythecallertocurrentvalues.Inodelockisheld+*thoughouttopreventracingwithanotherinstance.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,+structfileattr*fa)+{+structinode*inode=d_inode(dentry);+structfileattrold_ma={};+interr;++if(!inode->i_op->fileattr_set)+return-ENOIOCTLCMD;++if(!inode_owner_or_capable(idmap,inode))+return-EPERM;++inode_lock(inode);+err=vfs_fileattr_get(dentry,&old_ma);+if(!err){+/* initialize missing bits from old_ma */+if(fa->flags_valid){+fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;+fa->fsx_extsize=old_ma.fsx_extsize;+fa->fsx_nextents=old_ma.fsx_nextents;+fa->fsx_projid=old_ma.fsx_projid;+fa->fsx_cowextsize=old_ma.fsx_cowextsize;+}else{+fa->flags|=old_ma.flags&~FS_COMMON_FL;+}+err=fileattr_set_prepare(inode,&old_ma,fa);+if(!err)+err=inode->i_op->fileattr_set(idmap,dentry,fa);+}+inode_unlock(inode);++returnerr;+}+EXPORT_SYMBOL(vfs_fileattr_set);++intioctl_getflags(structfile*file,unsignedint__user*argp)+{+structfileattrfa={.flags_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=put_user(fa.flags,argp);+returnerr;+}+EXPORT_SYMBOL(ioctl_getflags);++intioctl_setflags(structfile*file,unsignedint__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+unsignedintflags;+interr;++err=get_user(flags,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+fileattr_fill_flags(&fa,flags);+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_setflags);++intioctl_fsgetxattr(structfile*file,void__user*argp)+{+structfileattrfa={.fsx_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=copy_fsxattr_to_user(&fa,argp);++returnerr;+}+EXPORT_SYMBOL(ioctl_fsgetxattr);++intioctl_fssetxattr(structfile*file,void__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+interr;++err=copy_fsxattr_from_user(&fa,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_fssetxattr);
@@ -453,315 +453,6 @@ static int ioctl_file_dedupe_range(struct file *file,returnret;}-/**-*fileattr_fill_xflags-initializefileattrwithxflags-*@fa:fileattrpointer-*@xflags:FS_XFLAG_*flags-*-*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All-*otherfieldsarezeroed.-*/-voidfileattr_fill_xflags(structfileattr*fa,u32xflags)-{-memset(fa,0,sizeof(*fa));-fa->fsx_valid=true;-fa->fsx_xflags=xflags;-if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)-fa->flags|=FS_IMMUTABLE_FL;-if(fa->fsx_xflags&FS_XFLAG_APPEND)-fa->flags|=FS_APPEND_FL;-if(fa->fsx_xflags&FS_XFLAG_SYNC)-fa->flags|=FS_SYNC_FL;-if(fa->fsx_xflags&FS_XFLAG_NOATIME)-fa->flags|=FS_NOATIME_FL;-if(fa->fsx_xflags&FS_XFLAG_NODUMP)-fa->flags|=FS_NODUMP_FL;-if(fa->fsx_xflags&FS_XFLAG_DAX)-fa->flags|=FS_DAX_FL;-if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)-fa->flags|=FS_PROJINHERIT_FL;-}-EXPORT_SYMBOL(fileattr_fill_xflags);--/**-*fileattr_fill_flags-initializefileattrwithflags-*@fa:fileattrpointer-*@flags:FS_*_FLflags-*-*Set->flags,->flags_validand->fsx_xflags(translatedflags).-*Allotherfieldsarezeroed.-*/-voidfileattr_fill_flags(structfileattr*fa,u32flags)-{-memset(fa,0,sizeof(*fa));-fa->flags_valid=true;-fa->flags=flags;-if(fa->flags&FS_SYNC_FL)-fa->fsx_xflags|=FS_XFLAG_SYNC;-if(fa->flags&FS_IMMUTABLE_FL)-fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;-if(fa->flags&FS_APPEND_FL)-fa->fsx_xflags|=FS_XFLAG_APPEND;-if(fa->flags&FS_NODUMP_FL)-fa->fsx_xflags|=FS_XFLAG_NODUMP;-if(fa->flags&FS_NOATIME_FL)-fa->fsx_xflags|=FS_XFLAG_NOATIME;-if(fa->flags&FS_DAX_FL)-fa->fsx_xflags|=FS_XFLAG_DAX;-if(fa->flags&FS_PROJINHERIT_FL)-fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;-}-EXPORT_SYMBOL(fileattr_fill_flags);--/**-*vfs_fileattr_get-retrievemiscellaneousfileattributes-*@dentry:theobjecttoretrievefrom-*@fa:fileattrpointer-*-*Calli_op->fileattr_get()callback,ifexists.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)-{-structinode*inode=d_inode(dentry);--if(!inode->i_op->fileattr_get)-return-ENOIOCTLCMD;--returninode->i_op->fileattr_get(dentry,fa);-}-EXPORT_SYMBOL(vfs_fileattr_get);--/**-*copy_fsxattr_to_user-copyfsxattrtouserspace.-*@fa:fileattrpointer-*@ufa:fsxattruserpointer-*-*Return:0onsuccess,or-EFAULTonfailure.-*/-intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)-{-structfsxattrxfa;--memset(&xfa,0,sizeof(xfa));-xfa.fsx_xflags=fa->fsx_xflags;-xfa.fsx_extsize=fa->fsx_extsize;-xfa.fsx_nextents=fa->fsx_nextents;-xfa.fsx_projid=fa->fsx_projid;-xfa.fsx_cowextsize=fa->fsx_cowextsize;--if(copy_to_user(ufa,&xfa,sizeof(xfa)))-return-EFAULT;--return0;-}-EXPORT_SYMBOL(copy_fsxattr_to_user);--staticintcopy_fsxattr_from_user(structfileattr*fa,-structfsxattr__user*ufa)-{-structfsxattrxfa;--if(copy_from_user(&xfa,ufa,sizeof(xfa)))-return-EFAULT;--fileattr_fill_xflags(fa,xfa.fsx_xflags);-fa->fsx_extsize=xfa.fsx_extsize;-fa->fsx_nextents=xfa.fsx_nextents;-fa->fsx_projid=xfa.fsx_projid;-fa->fsx_cowextsize=xfa.fsx_cowextsize;--return0;-}--/*-*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject-*anyinvalidconfigurations.-*-*Note:mustbecalledwithinodelockheld.-*/-staticintfileattr_set_prepare(structinode*inode,-conststructfileattr*old_ma,-structfileattr*fa)-{-interr;--/*-*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby-*therelevantcapability.-*/-if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&-!capable(CAP_LINUX_IMMUTABLE))-return-EPERM;--err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);-if(err)-returnerr;--/*-*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit-*namespace.Enforcethatrestrictiononlyifwearetryingtochange-*thequotaIDstate.Everythingelseisallowedinusernamespaces.-*/-if(current_user_ns()!=&init_user_ns){-if(old_ma->fsx_projid!=fa->fsx_projid)-return-EINVAL;-if((old_ma->fsx_xflags^fa->fsx_xflags)&-FS_XFLAG_PROJINHERIT)-return-EINVAL;-}else{-/*-*CallerisallowedtochangetheprojectID.Ifitisbeing-*changed,makesurethatthenewvalueisvalid.-*/-if(old_ma->fsx_projid!=fa->fsx_projid&&-!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))-return-EINVAL;-}--/* Check extent size hints. */-if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&-!S_ISDIR(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&-!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))-return-EINVAL;--/*-*ItisonlyvalidtosettheDAXflagonregularfilesand-*directoriesonfilesystems.-*/-if((fa->fsx_xflags&FS_XFLAG_DAX)&&-!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))-return-EINVAL;--/* Extent size hints of zero turn off the flags. */-if(fa->fsx_extsize==0)-fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);-if(fa->fsx_cowextsize==0)-fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;--return0;-}--/**-*vfs_fileattr_set-changemiscellaneousfileattributes-*@idmap:idmapofthemount-*@dentry:theobjecttochange-*@fa:fileattrpointer-*-*Afterverifyingpermissions,calli_op->fileattr_set()callback,if-*exists.-*-*Verifyingattributesinvolvesretrievingcurrentattributeswith-*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave-*notbeensetbythecallertocurrentvalues.Inodelockisheld-*thoughouttopreventracingwithanotherinstance.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,-structfileattr*fa)-{-structinode*inode=d_inode(dentry);-structfileattrold_ma={};-interr;--if(!inode->i_op->fileattr_set)-return-ENOIOCTLCMD;--if(!inode_owner_or_capable(idmap,inode))-return-EPERM;--inode_lock(inode);-err=vfs_fileattr_get(dentry,&old_ma);-if(!err){-/* initialize missing bits from old_ma */-if(fa->flags_valid){-fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;-fa->fsx_extsize=old_ma.fsx_extsize;-fa->fsx_nextents=old_ma.fsx_nextents;-fa->fsx_projid=old_ma.fsx_projid;-fa->fsx_cowextsize=old_ma.fsx_cowextsize;-}else{-fa->flags|=old_ma.flags&~FS_COMMON_FL;-}-err=fileattr_set_prepare(inode,&old_ma,fa);-if(!err)-err=inode->i_op->fileattr_set(idmap,dentry,fa);-}-inode_unlock(inode);--returnerr;-}-EXPORT_SYMBOL(vfs_fileattr_set);--staticintioctl_getflags(structfile*file,unsignedint__user*argp)-{-structfileattrfa={.flags_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=put_user(fa.flags,argp);-returnerr;-}--staticintioctl_setflags(structfile*file,unsignedint__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-unsignedintflags;-interr;--err=get_user(flags,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-fileattr_fill_flags(&fa,flags);-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}--staticintioctl_fsgetxattr(structfile*file,void__user*argp)-{-structfileattrfa={.fsx_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=copy_fsxattr_to_user(&fa,argp);--returnerr;-}--staticintioctl_fssetxattr(structfile*file,void__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-interr;--err=copy_fsxattr_from_user(&fa,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}-staticintioctl_getfsuuid(structfile*file,void__user*argp){structsuper_block*sb=file_inode(file)->i_sb;
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-01 06:05:59
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn [off-list ref] wrote:
quoted hunk
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ecryptfs/inode.c | 8 +++++++-
fs/file_attr.c | 12 ++++++++++--
fs/overlayfs/inode.c | 2 +-
3 files changed, 18 insertions(+), 4 deletions(-)
I think the semantics should be
"This patch converts return code of vfs_fileattr_[gs]et and ->fileattr_[gs]et()
from ENOIOCTLCMD to EOPNOSUPP"
ENOIOCTLCMD belongs only in the ioctl frontend, so above conversion
is not needed.
quoted hunk
static int ecryptfs_fileattr_set(struct mnt_idmap *idmap,
That's the wrong way, because it hides the desired -EOPNOTSUPP
return code from ovl_fileattr_get().
The conversion to -ENOTTY was done for
5b0a414d06c3 ("ovl: fix filattr copy-up failure"),
so please do this instead:
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-01 06:11:46
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn [off-list ref] wrote:
This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
An xfstests test generic/766 with basic coverage is at:
https://github.com/alberand/xfstests/commits/b4/file-attr/
NAME
file_getattr/file_setattr - get/set filesystem inode attributes
SYNOPSIS
#include <sys/syscall.h> /* Definition of SYS_* constants */
#include <unistd.h>
long syscall(SYS_file_getattr, int dirfd, const char *pathname,
struct fsx_fileattr *fsx, size_t size,
unsigned int at_flags);
long syscall(SYS_file_setattr, int dirfd, const char *pathname,
struct fsx_fileattr *fsx, size_t size,
unsigned int at_flags);
Note: glibc doesn't provide for file_getattr()/file_setattr(),
use syscall(2) instead.
DESCRIPTION
The file_getattr()/file_setattr() are used to set extended file
attributes. These syscalls take dirfd in conjunction with the
pathname argument. The syscall then operates on inode opened
according to openat(2) semantics.
This is an alternative to FS_IOC_FSGETXATTR/FS_IOC_FSSETXATTR
ioctl with a difference that file don't need to be open as file
can be referenced with a path instead of fd. By having this one
can manipulated filesystem inode attributes not only on regular
files but also on special ones. This is not possible with
FS_IOC_FSSETXATTR ioctl as ioctl() can not be called on special
files directly for the filesystem inode.
at_flags can be set to AT_SYMLINK_NOFOLLOW or AT_EMPTY_PATH.
RETURN VALUE
On success, 0 is returned. On error, -1 is returned, and errno
is set to indicate the error.
ERRORS
EINVAL Invalid at_flag specified (only
AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH is
supported).
EINVAL Size was smaller than any known version of
struct fsx_fileattr.
EINVAL Invalid combination of parameters provided in
fsx_fileattr for this type of file.
E2BIG Size of input argument struct fsx_fileattr
is too big.
EBADF Invalid file descriptor was provided.
EPERM No permission to change this file.
EOPNOTSUPP Filesystem does not support setting attributes
on this type of inode
HISTORY
Added in Linux 6.16.
EXAMPLE
Create directory and file "mkdir ./dir && touch ./dir/foo" and then
execute the following program:
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <linux/fs.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>
#if !defined(SYS_file_getattr) && defined(__x86_64__)
#define SYS_file_getattr 468
#define SYS_file_setattr 469
struct fsx_fileattr {
__u32 fsx_xflags;
__u32 fsx_extsize;
__u32 fsx_nextents;
__u32 fsx_projid;
__u32 fsx_cowextsize;
};
#endif
int
main(int argc, char **argv) {
int dfd;
int error;
struct fsx_fileattr fsx;
dfd = open("./dir", O_RDONLY);
if (dfd == -1) {
printf("can not open ./dir");
return dfd;
}
error = syscall(SYS_file_getattr, dfd, "./foo", &fsx,
sizeof(struct fsx_fileattr), 0);
if (error) {
printf("can not call SYS_file_getattr: %s",
strerror(errno));
return error;
}
printf("./dir/foo flags: %d\n", fsx.fsx_xflags);
fsx.fsx_xflags |= FS_XFLAG_NODUMP;
error = syscall(SYS_file_setattr, dfd, "./foo", &fsx,
sizeof(struct fsx_fileattr), 0);
if (error) {
printf("can not call SYS_file_setattr: %s",
strerror(errno));
return error;
}
printf("./dir/foo flags: %d\n", fsx.fsx_xflags);
return error;
}
SEE ALSO
ioctl(2), ioctl_iflags(2), ioctl_xfs_fsgetxattr(2), openat(2)
---
Changes in v6:
- Update cover letter example and docs
- Applied __free() attribute for syscall stack objects
- Introduced struct fsx_fileattr
- Replace 'struct fsxattr' with 'struct fsx_fileattr'
- Add helper to fill in fsx_fileattr from fileattr
- Dropped copy_fsx_to_user() header declaration
- Link to v5: https://lore.kernel.org/r/20250513-xattrat-syscall-v5-0-22bb9c6c767f@kernel.org
Series looks good.
For mine and Pali's minor comments on patch 4 no need to resend.
I think they could be fixed on commit.
Thanks,
Amir.
Changes in v5:
- Remove setting of LOOKUP_EMPTY flags which does not have any effect
- Return -ENOSUPP from vfs_fileattr_set()
- Add fsxattr masking (by Amir)
- Fix UAF issue dentry
- Fix getname_maybe_null() issue with NULL path
- Implement file_getattr/file_setattr hooks
- Return LSM return code from file_setattr
- Rename from getfsxattrat/setfsxattrat to file_getattr/file_setattr
- Link to v4: https://lore.kernel.org/r/20250321-xattrat-syscall-v4-0-3e82e6fb3264@kernel.org
Changes in v4:
- Use getname_maybe_null() for correct handling of dfd + path semantic
- Remove restriction for special files on which flags are allowed
- Utilize copy_struct_from_user() for better future compatibility
- Add draft man page to cover letter
- Convert -ENOIOCTLCMD to -EOPNOSUPP as more appropriate for syscall
- Add missing __user to header declaration of syscalls
- Link to v3: https://lore.kernel.org/r/20250211-xattrat-syscall-v3-1-a07d15f898b2@kernel.org
Changes in v3:
- Remove unnecessary "dfd is dir" check as it checked in user_path_at()
- Remove unnecessary "same filesystem" check
- Use CLASS() instead of directly calling fdget/fdput
- Link to v2: https://lore.kernel.org/r/20250122-xattrat-syscall-v2-1-5b360d4fbcb2@kernel.org
v1:
https://lore.kernel.org/linuxppc-dev/20250109174540.893098-1-aalbersh@kernel.org/
Previous discussion:
https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/
---
Amir Goldstein (1):
fs: prepare for extending file_get/setattr()
Andrey Albershteyn (5):
fs: split fileattr related helpers into separate file
lsm: introduce new hooks for setting/getting inode fsxattr
selinux: implement inode_file_[g|s]etattr hooks
fs: make vfs_fileattr_[get|set] return -EOPNOSUPP
fs: introduce file_getattr and file_setattr syscalls
arch/alpha/kernel/syscalls/syscall.tbl | 2 +
arch/arm/tools/syscall.tbl | 2 +
arch/arm64/tools/syscall_32.tbl | 2 +
arch/m68k/kernel/syscalls/syscall.tbl | 2 +
arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
arch/parisc/kernel/syscalls/syscall.tbl | 2 +
arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
arch/s390/kernel/syscalls/syscall.tbl | 2 +
arch/sh/kernel/syscalls/syscall.tbl | 2 +
arch/sparc/kernel/syscalls/syscall.tbl | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 2 +
arch/x86/entry/syscalls/syscall_64.tbl | 2 +
arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
fs/Makefile | 3 +-
fs/ecryptfs/inode.c | 8 +-
fs/file_attr.c | 493 ++++++++++++++++++++++++++++
fs/ioctl.c | 309 -----------------
fs/overlayfs/inode.c | 2 +-
include/linux/fileattr.h | 24 ++
include/linux/lsm_hook_defs.h | 2 +
include/linux/security.h | 16 +
include/linux/syscalls.h | 6 +
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/fs.h | 18 +
scripts/syscall.tbl | 2 +
security/security.c | 30 ++
security/selinux/hooks.c | 14 +
30 files changed, 654 insertions(+), 313 deletions(-)
---
base-commit: d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af
change-id: 20250114-xattrat-syscall-6a1136d2db59
Best regards,
--
Andrey Albershteyn [off-list ref]
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-01 12:29:47
On Mon, Jun 30, 2025 at 06:20:10PM +0200, Andrey Albershteyn wrote:
This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
Only small nits I'm going to comment on that I can fix myself.
Otherwise looks great.
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-01 12:34:54
On Mon, Jun 30, 2025 at 06:20:16PM +0200, Andrey Albershteyn wrote:
quoted hunk
From: Andrey Albershteyn <redacted>
Introduce file_getattr() and file_setattr() syscalls to manipulate inode
extended attributes. The syscalls takes pair of file descriptor and
pathname. Then it operates on inode opened accroding to openat()
semantics. The struct fsx_fileattr is passed to obtain/change extended
attributes.
This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
that file don't need to be open as we can reference it with a path
instead of fd. By having this we can manipulated inode extended
attributes not only on regular files but also on special ones. This
is not possible with FS_IOC_FSSETXATTR ioctl as with special files
we can not call ioctl() directly on the filesystem inode using fd.
This patch adds two new syscalls which allows userspace to get/set
extended inode attributes on special files by using parent directory
and a path - *at() like syscall.
CC: linux-api@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-xfs@vger.kernel.org
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/alpha/kernel/syscalls/syscall.tbl | 2 +
arch/arm/tools/syscall.tbl | 2 +
arch/arm64/tools/syscall_32.tbl | 2 +
arch/m68k/kernel/syscalls/syscall.tbl | 2 +
arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
arch/parisc/kernel/syscalls/syscall.tbl | 2 +
arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
arch/s390/kernel/syscalls/syscall.tbl | 2 +
arch/sh/kernel/syscalls/syscall.tbl | 2 +
arch/sparc/kernel/syscalls/syscall.tbl | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 2 +
arch/x86/entry/syscalls/syscall_64.tbl | 2 +
arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
fs/file_attr.c | 148 ++++++++++++++++++++++++++++
include/linux/syscalls.h | 6 ++
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/fs.h | 18 ++++
scripts/syscall.tbl | 2 +
21 files changed, 213 insertions(+), 1 deletion(-)
@@ -507,3 +507,5 @@ 575 common listxattrat sys_listxattrat 576 common removexattrat sys_removexattrat 577 common open_tree_attr sys_open_tree_attr+578 common file_getattr sys_file_getattr+579 common file_setattr sys_file_setattr
@@ -482,3 +482,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -479,3 +479,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -467,3 +467,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -473,3 +473,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -466,3 +466,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -558,3 +558,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -471,3 +471,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -513,3 +513,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -391,6 +391,8 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr # # Due to a historical design error, certain syscalls are numbered differently
@@ -438,3 +438,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
Fwiw, what also works is:
*fsx = (struct fsx_fileattr){
.fsx_xflags = fa->fsx_xflags & mask,
.fsx_extsize = fa->fsx_extsize,
.fsx_nextents = fa->fsx_nextents,
.fsx_projid = fa->fsx_projid,
.fsx_cowextsize = fa->fsx_cowextsize,
}
avoiding the memset(). Anyway, all minor nits.
Fwiw, cleanup guards should always be grouped together at the top like:
struct path filepath __free(path_put) = {};
struct filename *name __free(putname) = NULL;
struct fileattr fa;
int error;
unsigned int lookup_flags = 0;
This makes it easy to spot them when reading a function with multiple
variables on top.
@@ -148,6 +148,24 @@ struct fsxattr {unsignedcharfsx_pad[8];};+/*+*Variablesizestructureforfile_[sg]et_attr().+*+*Note.Thisisalternativetothestructure'structfileattr'/'structfsxattr'.+*Asthisstructureispassedto/fromuserspacewithitssize,thiscan+*beversionedbasedonthesize.+*/+structfsx_fileattr{+__u32fsx_xflags;/* xflags field value (get/set) */+__u32fsx_extsize;/* extsize field value (get/set)*/+__u32fsx_nextents;/* nextents field value (get) */+__u32fsx_projid;/* project identifier (get/set) */+__u32fsx_cowextsize;/* CoW extsize field value (get/set) */
This misses a:
__u32 __spare;
so there's no holes in the struct. :)
quoted hunk
+};
+
+#define FSX_FILEATTR_SIZE_VER0 20
+#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0
+
/*
* Flags for the fsx_xflags field
*/
@@ -408,3 +408,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
From: Jan Kara <jack@suse.cz> Date: 2025-07-01 12:38:23
On Mon 30-06-25 18:20:11, Andrey Albershteyn wrote:
From: Andrey Albershteyn <aalbersh@kernel.org>
This patch moves function related to file extended attributes
manipulations to separate file. Refactoring only.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
@@ -0,0 +1,318 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/fs.h>+#include<linux/security.h>+#include<linux/fscrypt.h>+#include<linux/fileattr.h>++/**+*fileattr_fill_xflags-initializefileattrwithxflags+*@fa:fileattrpointer+*@xflags:FS_XFLAG_*flags+*+*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All+*otherfieldsarezeroed.+*/+voidfileattr_fill_xflags(structfileattr*fa,u32xflags)+{+memset(fa,0,sizeof(*fa));+fa->fsx_valid=true;+fa->fsx_xflags=xflags;+if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)+fa->flags|=FS_IMMUTABLE_FL;+if(fa->fsx_xflags&FS_XFLAG_APPEND)+fa->flags|=FS_APPEND_FL;+if(fa->fsx_xflags&FS_XFLAG_SYNC)+fa->flags|=FS_SYNC_FL;+if(fa->fsx_xflags&FS_XFLAG_NOATIME)+fa->flags|=FS_NOATIME_FL;+if(fa->fsx_xflags&FS_XFLAG_NODUMP)+fa->flags|=FS_NODUMP_FL;+if(fa->fsx_xflags&FS_XFLAG_DAX)+fa->flags|=FS_DAX_FL;+if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)+fa->flags|=FS_PROJINHERIT_FL;+}+EXPORT_SYMBOL(fileattr_fill_xflags);++/**+*fileattr_fill_flags-initializefileattrwithflags+*@fa:fileattrpointer+*@flags:FS_*_FLflags+*+*Set->flags,->flags_validand->fsx_xflags(translatedflags).+*Allotherfieldsarezeroed.+*/+voidfileattr_fill_flags(structfileattr*fa,u32flags)+{+memset(fa,0,sizeof(*fa));+fa->flags_valid=true;+fa->flags=flags;+if(fa->flags&FS_SYNC_FL)+fa->fsx_xflags|=FS_XFLAG_SYNC;+if(fa->flags&FS_IMMUTABLE_FL)+fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;+if(fa->flags&FS_APPEND_FL)+fa->fsx_xflags|=FS_XFLAG_APPEND;+if(fa->flags&FS_NODUMP_FL)+fa->fsx_xflags|=FS_XFLAG_NODUMP;+if(fa->flags&FS_NOATIME_FL)+fa->fsx_xflags|=FS_XFLAG_NOATIME;+if(fa->flags&FS_DAX_FL)+fa->fsx_xflags|=FS_XFLAG_DAX;+if(fa->flags&FS_PROJINHERIT_FL)+fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;+}+EXPORT_SYMBOL(fileattr_fill_flags);++/**+*vfs_fileattr_get-retrievemiscellaneousfileattributes+*@dentry:theobjecttoretrievefrom+*@fa:fileattrpointer+*+*Calli_op->fileattr_get()callback,ifexists.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)+{+structinode*inode=d_inode(dentry);++if(!inode->i_op->fileattr_get)+return-ENOIOCTLCMD;++returninode->i_op->fileattr_get(dentry,fa);+}+EXPORT_SYMBOL(vfs_fileattr_get);++/**+*copy_fsxattr_to_user-copyfsxattrtouserspace.+*@fa:fileattrpointer+*@ufa:fsxattruserpointer+*+*Return:0onsuccess,or-EFAULTonfailure.+*/+intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)+{+structfsxattrxfa;++memset(&xfa,0,sizeof(xfa));+xfa.fsx_xflags=fa->fsx_xflags;+xfa.fsx_extsize=fa->fsx_extsize;+xfa.fsx_nextents=fa->fsx_nextents;+xfa.fsx_projid=fa->fsx_projid;+xfa.fsx_cowextsize=fa->fsx_cowextsize;++if(copy_to_user(ufa,&xfa,sizeof(xfa)))+return-EFAULT;++return0;+}+EXPORT_SYMBOL(copy_fsxattr_to_user);++staticintcopy_fsxattr_from_user(structfileattr*fa,+structfsxattr__user*ufa)+{+structfsxattrxfa;++if(copy_from_user(&xfa,ufa,sizeof(xfa)))+return-EFAULT;++fileattr_fill_xflags(fa,xfa.fsx_xflags);+fa->fsx_extsize=xfa.fsx_extsize;+fa->fsx_nextents=xfa.fsx_nextents;+fa->fsx_projid=xfa.fsx_projid;+fa->fsx_cowextsize=xfa.fsx_cowextsize;++return0;+}++/*+*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject+*anyinvalidconfigurations.+*+*Note:mustbecalledwithinodelockheld.+*/+staticintfileattr_set_prepare(structinode*inode,+conststructfileattr*old_ma,+structfileattr*fa)+{+interr;++/*+*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby+*therelevantcapability.+*/+if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&+!capable(CAP_LINUX_IMMUTABLE))+return-EPERM;++err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);+if(err)+returnerr;++/*+*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit+*namespace.Enforcethatrestrictiononlyifwearetryingtochange+*thequotaIDstate.Everythingelseisallowedinusernamespaces.+*/+if(current_user_ns()!=&init_user_ns){+if(old_ma->fsx_projid!=fa->fsx_projid)+return-EINVAL;+if((old_ma->fsx_xflags^fa->fsx_xflags)&+FS_XFLAG_PROJINHERIT)+return-EINVAL;+}else{+/*+*CallerisallowedtochangetheprojectID.Ifitisbeing+*changed,makesurethatthenewvalueisvalid.+*/+if(old_ma->fsx_projid!=fa->fsx_projid&&+!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))+return-EINVAL;+}++/* Check extent size hints. */+if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&+!S_ISDIR(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&+!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))+return-EINVAL;++/*+*ItisonlyvalidtosettheDAXflagonregularfilesand+*directoriesonfilesystems.+*/+if((fa->fsx_xflags&FS_XFLAG_DAX)&&+!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))+return-EINVAL;++/* Extent size hints of zero turn off the flags. */+if(fa->fsx_extsize==0)+fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);+if(fa->fsx_cowextsize==0)+fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;++return0;+}++/**+*vfs_fileattr_set-changemiscellaneousfileattributes+*@idmap:idmapofthemount+*@dentry:theobjecttochange+*@fa:fileattrpointer+*+*Afterverifyingpermissions,calli_op->fileattr_set()callback,if+*exists.+*+*Verifyingattributesinvolvesretrievingcurrentattributeswith+*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave+*notbeensetbythecallertocurrentvalues.Inodelockisheld+*thoughouttopreventracingwithanotherinstance.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,+structfileattr*fa)+{+structinode*inode=d_inode(dentry);+structfileattrold_ma={};+interr;++if(!inode->i_op->fileattr_set)+return-ENOIOCTLCMD;++if(!inode_owner_or_capable(idmap,inode))+return-EPERM;++inode_lock(inode);+err=vfs_fileattr_get(dentry,&old_ma);+if(!err){+/* initialize missing bits from old_ma */+if(fa->flags_valid){+fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;+fa->fsx_extsize=old_ma.fsx_extsize;+fa->fsx_nextents=old_ma.fsx_nextents;+fa->fsx_projid=old_ma.fsx_projid;+fa->fsx_cowextsize=old_ma.fsx_cowextsize;+}else{+fa->flags|=old_ma.flags&~FS_COMMON_FL;+}+err=fileattr_set_prepare(inode,&old_ma,fa);+if(!err)+err=inode->i_op->fileattr_set(idmap,dentry,fa);+}+inode_unlock(inode);++returnerr;+}+EXPORT_SYMBOL(vfs_fileattr_set);++intioctl_getflags(structfile*file,unsignedint__user*argp)+{+structfileattrfa={.flags_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=put_user(fa.flags,argp);+returnerr;+}+EXPORT_SYMBOL(ioctl_getflags);++intioctl_setflags(structfile*file,unsignedint__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+unsignedintflags;+interr;++err=get_user(flags,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+fileattr_fill_flags(&fa,flags);+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_setflags);++intioctl_fsgetxattr(structfile*file,void__user*argp)+{+structfileattrfa={.fsx_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=copy_fsxattr_to_user(&fa,argp);++returnerr;+}+EXPORT_SYMBOL(ioctl_fsgetxattr);++intioctl_fssetxattr(structfile*file,void__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+interr;++err=copy_fsxattr_from_user(&fa,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_fssetxattr);
@@ -453,315 +453,6 @@ static int ioctl_file_dedupe_range(struct file *file,returnret;}-/**-*fileattr_fill_xflags-initializefileattrwithxflags-*@fa:fileattrpointer-*@xflags:FS_XFLAG_*flags-*-*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All-*otherfieldsarezeroed.-*/-voidfileattr_fill_xflags(structfileattr*fa,u32xflags)-{-memset(fa,0,sizeof(*fa));-fa->fsx_valid=true;-fa->fsx_xflags=xflags;-if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)-fa->flags|=FS_IMMUTABLE_FL;-if(fa->fsx_xflags&FS_XFLAG_APPEND)-fa->flags|=FS_APPEND_FL;-if(fa->fsx_xflags&FS_XFLAG_SYNC)-fa->flags|=FS_SYNC_FL;-if(fa->fsx_xflags&FS_XFLAG_NOATIME)-fa->flags|=FS_NOATIME_FL;-if(fa->fsx_xflags&FS_XFLAG_NODUMP)-fa->flags|=FS_NODUMP_FL;-if(fa->fsx_xflags&FS_XFLAG_DAX)-fa->flags|=FS_DAX_FL;-if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)-fa->flags|=FS_PROJINHERIT_FL;-}-EXPORT_SYMBOL(fileattr_fill_xflags);--/**-*fileattr_fill_flags-initializefileattrwithflags-*@fa:fileattrpointer-*@flags:FS_*_FLflags-*-*Set->flags,->flags_validand->fsx_xflags(translatedflags).-*Allotherfieldsarezeroed.-*/-voidfileattr_fill_flags(structfileattr*fa,u32flags)-{-memset(fa,0,sizeof(*fa));-fa->flags_valid=true;-fa->flags=flags;-if(fa->flags&FS_SYNC_FL)-fa->fsx_xflags|=FS_XFLAG_SYNC;-if(fa->flags&FS_IMMUTABLE_FL)-fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;-if(fa->flags&FS_APPEND_FL)-fa->fsx_xflags|=FS_XFLAG_APPEND;-if(fa->flags&FS_NODUMP_FL)-fa->fsx_xflags|=FS_XFLAG_NODUMP;-if(fa->flags&FS_NOATIME_FL)-fa->fsx_xflags|=FS_XFLAG_NOATIME;-if(fa->flags&FS_DAX_FL)-fa->fsx_xflags|=FS_XFLAG_DAX;-if(fa->flags&FS_PROJINHERIT_FL)-fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;-}-EXPORT_SYMBOL(fileattr_fill_flags);--/**-*vfs_fileattr_get-retrievemiscellaneousfileattributes-*@dentry:theobjecttoretrievefrom-*@fa:fileattrpointer-*-*Calli_op->fileattr_get()callback,ifexists.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)-{-structinode*inode=d_inode(dentry);--if(!inode->i_op->fileattr_get)-return-ENOIOCTLCMD;--returninode->i_op->fileattr_get(dentry,fa);-}-EXPORT_SYMBOL(vfs_fileattr_get);--/**-*copy_fsxattr_to_user-copyfsxattrtouserspace.-*@fa:fileattrpointer-*@ufa:fsxattruserpointer-*-*Return:0onsuccess,or-EFAULTonfailure.-*/-intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)-{-structfsxattrxfa;--memset(&xfa,0,sizeof(xfa));-xfa.fsx_xflags=fa->fsx_xflags;-xfa.fsx_extsize=fa->fsx_extsize;-xfa.fsx_nextents=fa->fsx_nextents;-xfa.fsx_projid=fa->fsx_projid;-xfa.fsx_cowextsize=fa->fsx_cowextsize;--if(copy_to_user(ufa,&xfa,sizeof(xfa)))-return-EFAULT;--return0;-}-EXPORT_SYMBOL(copy_fsxattr_to_user);--staticintcopy_fsxattr_from_user(structfileattr*fa,-structfsxattr__user*ufa)-{-structfsxattrxfa;--if(copy_from_user(&xfa,ufa,sizeof(xfa)))-return-EFAULT;--fileattr_fill_xflags(fa,xfa.fsx_xflags);-fa->fsx_extsize=xfa.fsx_extsize;-fa->fsx_nextents=xfa.fsx_nextents;-fa->fsx_projid=xfa.fsx_projid;-fa->fsx_cowextsize=xfa.fsx_cowextsize;--return0;-}--/*-*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject-*anyinvalidconfigurations.-*-*Note:mustbecalledwithinodelockheld.-*/-staticintfileattr_set_prepare(structinode*inode,-conststructfileattr*old_ma,-structfileattr*fa)-{-interr;--/*-*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby-*therelevantcapability.-*/-if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&-!capable(CAP_LINUX_IMMUTABLE))-return-EPERM;--err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);-if(err)-returnerr;--/*-*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit-*namespace.Enforcethatrestrictiononlyifwearetryingtochange-*thequotaIDstate.Everythingelseisallowedinusernamespaces.-*/-if(current_user_ns()!=&init_user_ns){-if(old_ma->fsx_projid!=fa->fsx_projid)-return-EINVAL;-if((old_ma->fsx_xflags^fa->fsx_xflags)&-FS_XFLAG_PROJINHERIT)-return-EINVAL;-}else{-/*-*CallerisallowedtochangetheprojectID.Ifitisbeing-*changed,makesurethatthenewvalueisvalid.-*/-if(old_ma->fsx_projid!=fa->fsx_projid&&-!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))-return-EINVAL;-}--/* Check extent size hints. */-if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&-!S_ISDIR(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&-!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))-return-EINVAL;--/*-*ItisonlyvalidtosettheDAXflagonregularfilesand-*directoriesonfilesystems.-*/-if((fa->fsx_xflags&FS_XFLAG_DAX)&&-!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))-return-EINVAL;--/* Extent size hints of zero turn off the flags. */-if(fa->fsx_extsize==0)-fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);-if(fa->fsx_cowextsize==0)-fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;--return0;-}--/**-*vfs_fileattr_set-changemiscellaneousfileattributes-*@idmap:idmapofthemount-*@dentry:theobjecttochange-*@fa:fileattrpointer-*-*Afterverifyingpermissions,calli_op->fileattr_set()callback,if-*exists.-*-*Verifyingattributesinvolvesretrievingcurrentattributeswith-*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave-*notbeensetbythecallertocurrentvalues.Inodelockisheld-*thoughouttopreventracingwithanotherinstance.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,-structfileattr*fa)-{-structinode*inode=d_inode(dentry);-structfileattrold_ma={};-interr;--if(!inode->i_op->fileattr_set)-return-ENOIOCTLCMD;--if(!inode_owner_or_capable(idmap,inode))-return-EPERM;--inode_lock(inode);-err=vfs_fileattr_get(dentry,&old_ma);-if(!err){-/* initialize missing bits from old_ma */-if(fa->flags_valid){-fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;-fa->fsx_extsize=old_ma.fsx_extsize;-fa->fsx_nextents=old_ma.fsx_nextents;-fa->fsx_projid=old_ma.fsx_projid;-fa->fsx_cowextsize=old_ma.fsx_cowextsize;-}else{-fa->flags|=old_ma.flags&~FS_COMMON_FL;-}-err=fileattr_set_prepare(inode,&old_ma,fa);-if(!err)-err=inode->i_op->fileattr_set(idmap,dentry,fa);-}-inode_unlock(inode);--returnerr;-}-EXPORT_SYMBOL(vfs_fileattr_set);--staticintioctl_getflags(structfile*file,unsignedint__user*argp)-{-structfileattrfa={.flags_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=put_user(fa.flags,argp);-returnerr;-}--staticintioctl_setflags(structfile*file,unsignedint__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-unsignedintflags;-interr;--err=get_user(flags,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-fileattr_fill_flags(&fa,flags);-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}--staticintioctl_fsgetxattr(structfile*file,void__user*argp)-{-structfileattrfa={.fsx_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=copy_fsxattr_to_user(&fa,argp);--returnerr;-}--staticintioctl_fssetxattr(structfile*file,void__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-interr;--err=copy_fsxattr_from_user(&fa,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}-staticintioctl_getfsuuid(structfile*file,void__user*argp){structsuper_block*sb=file_inode(file)->i_sb;
From: Jan Kara <jack@suse.cz> Date: 2025-07-01 12:39:52
On Mon 30-06-25 18:20:12, Andrey Albershteyn wrote:
Introduce new hooks for setting and getting filesystem extended
attributes on inode (FS_IOC_FSGETXATTR).
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
From: Jan Kara <jack@suse.cz> Date: 2025-07-01 12:51:58
On Tue 01-07-25 08:05:45, Amir Goldstein wrote:
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn [off-list ref] wrote:
quoted
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
That's the wrong way, because it hides the desired -EOPNOTSUPP
return code from ovl_fileattr_get().
The conversion to -ENOTTY was done for
5b0a414d06c3 ("ovl: fix filattr copy-up failure"),
so please do this instead:
From: Jan Kara <jack@suse.cz> Date: 2025-07-01 12:53:03
On Mon 30-06-25 18:20:14, Andrey Albershteyn wrote:
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Modulo the small nits already pointed out this looks good to me. Feel free
to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
--
Jan Kara [off-list ref]
SUSE Labs, CR
From: Jan Kara <jack@suse.cz> Date: 2025-07-01 13:06:33
On Mon 30-06-25 18:20:15, Andrey Albershteyn wrote:
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
I'd just note that:
This means that the two flags in FS_XFLAG_RDONLY_MASK cannot easily become
writeable in the future due to this. I think it is a sensible compromise
but I wanted to mention it.
Honza
--
Jan Kara [off-list ref]
SUSE Labs, CR
From: Jan Kara <jack@suse.cz> Date: 2025-07-01 13:24:10
On Mon 30-06-25 18:20:16, Andrey Albershteyn wrote:
From: Andrey Albershteyn <redacted>
Introduce file_getattr() and file_setattr() syscalls to manipulate inode
extended attributes. The syscalls takes pair of file descriptor and
pathname. Then it operates on inode opened accroding to openat()
^^^ according
semantics. The struct fsx_fileattr is passed to obtain/change extended
attributes.
This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
that file don't need to be open as we can reference it with a path
^^^ doesn't
instead of fd. By having this we can manipulated inode extended
attributes not only on regular files but also on special ones. This
is not possible with FS_IOC_FSSETXATTR ioctl as with special files
we can not call ioctl() directly on the filesystem inode using fd.
This patch adds two new syscalls which allows userspace to get/set
extended inode attributes on special files by using parent directory
and a path - *at() like syscall.
CC: linux-api@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-xfs@vger.kernel.org
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
There's possible NULL ptr deref bug below (2x) that's easy to fix. Once
done feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-01 14:16:28
On Tue, Jul 1, 2025 at 2:51 PM Jan Kara [off-list ref] wrote:
On Tue 01-07-25 08:05:45, Amir Goldstein wrote:
quoted
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn [off-list ref] wrote:
quoted
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
That's the wrong way, because it hides the desired -EOPNOTSUPP
return code from ovl_fileattr_get().
The conversion to -ENOTTY was done for
5b0a414d06c3 ("ovl: fix filattr copy-up failure"),
so please do this instead:
Is this really needed? AFAICS nobody returns ENOIOCTLCMD after this
patch...
you are right it is not needed
Attaching the patch with missing bits of fuse and overlayfs to make this
conversion complete.
Christian, please squash my patch
and afterward make sure there is no conversion remaining in
ovl_real_fileattr_get() as well as in ecryptfs_fileattr_get()
Both those helpers should return the value they
got from vfs_fileattr_get() as is.
Thanks,
Amir.
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-01 18:13:05
On Mon, Jun 30, 2025 at 06:20:11PM +0200, Andrey Albershteyn wrote:
From: Andrey Albershteyn <aalbersh@kernel.org>
This patch moves function related to file extended attributes
manipulations to separate file. Refactoring only.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Seems fine to me to move that to a separate file.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
@@ -0,0 +1,318 @@+// SPDX-License-Identifier: GPL-2.0+#include<linux/fs.h>+#include<linux/security.h>+#include<linux/fscrypt.h>+#include<linux/fileattr.h>++/**+*fileattr_fill_xflags-initializefileattrwithxflags+*@fa:fileattrpointer+*@xflags:FS_XFLAG_*flags+*+*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All+*otherfieldsarezeroed.+*/+voidfileattr_fill_xflags(structfileattr*fa,u32xflags)+{+memset(fa,0,sizeof(*fa));+fa->fsx_valid=true;+fa->fsx_xflags=xflags;+if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)+fa->flags|=FS_IMMUTABLE_FL;+if(fa->fsx_xflags&FS_XFLAG_APPEND)+fa->flags|=FS_APPEND_FL;+if(fa->fsx_xflags&FS_XFLAG_SYNC)+fa->flags|=FS_SYNC_FL;+if(fa->fsx_xflags&FS_XFLAG_NOATIME)+fa->flags|=FS_NOATIME_FL;+if(fa->fsx_xflags&FS_XFLAG_NODUMP)+fa->flags|=FS_NODUMP_FL;+if(fa->fsx_xflags&FS_XFLAG_DAX)+fa->flags|=FS_DAX_FL;+if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)+fa->flags|=FS_PROJINHERIT_FL;+}+EXPORT_SYMBOL(fileattr_fill_xflags);++/**+*fileattr_fill_flags-initializefileattrwithflags+*@fa:fileattrpointer+*@flags:FS_*_FLflags+*+*Set->flags,->flags_validand->fsx_xflags(translatedflags).+*Allotherfieldsarezeroed.+*/+voidfileattr_fill_flags(structfileattr*fa,u32flags)+{+memset(fa,0,sizeof(*fa));+fa->flags_valid=true;+fa->flags=flags;+if(fa->flags&FS_SYNC_FL)+fa->fsx_xflags|=FS_XFLAG_SYNC;+if(fa->flags&FS_IMMUTABLE_FL)+fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;+if(fa->flags&FS_APPEND_FL)+fa->fsx_xflags|=FS_XFLAG_APPEND;+if(fa->flags&FS_NODUMP_FL)+fa->fsx_xflags|=FS_XFLAG_NODUMP;+if(fa->flags&FS_NOATIME_FL)+fa->fsx_xflags|=FS_XFLAG_NOATIME;+if(fa->flags&FS_DAX_FL)+fa->fsx_xflags|=FS_XFLAG_DAX;+if(fa->flags&FS_PROJINHERIT_FL)+fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;+}+EXPORT_SYMBOL(fileattr_fill_flags);++/**+*vfs_fileattr_get-retrievemiscellaneousfileattributes+*@dentry:theobjecttoretrievefrom+*@fa:fileattrpointer+*+*Calli_op->fileattr_get()callback,ifexists.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)+{+structinode*inode=d_inode(dentry);++if(!inode->i_op->fileattr_get)+return-ENOIOCTLCMD;++returninode->i_op->fileattr_get(dentry,fa);+}+EXPORT_SYMBOL(vfs_fileattr_get);++/**+*copy_fsxattr_to_user-copyfsxattrtouserspace.+*@fa:fileattrpointer+*@ufa:fsxattruserpointer+*+*Return:0onsuccess,or-EFAULTonfailure.+*/+intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)+{+structfsxattrxfa;++memset(&xfa,0,sizeof(xfa));+xfa.fsx_xflags=fa->fsx_xflags;+xfa.fsx_extsize=fa->fsx_extsize;+xfa.fsx_nextents=fa->fsx_nextents;+xfa.fsx_projid=fa->fsx_projid;+xfa.fsx_cowextsize=fa->fsx_cowextsize;++if(copy_to_user(ufa,&xfa,sizeof(xfa)))+return-EFAULT;++return0;+}+EXPORT_SYMBOL(copy_fsxattr_to_user);++staticintcopy_fsxattr_from_user(structfileattr*fa,+structfsxattr__user*ufa)+{+structfsxattrxfa;++if(copy_from_user(&xfa,ufa,sizeof(xfa)))+return-EFAULT;++fileattr_fill_xflags(fa,xfa.fsx_xflags);+fa->fsx_extsize=xfa.fsx_extsize;+fa->fsx_nextents=xfa.fsx_nextents;+fa->fsx_projid=xfa.fsx_projid;+fa->fsx_cowextsize=xfa.fsx_cowextsize;++return0;+}++/*+*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject+*anyinvalidconfigurations.+*+*Note:mustbecalledwithinodelockheld.+*/+staticintfileattr_set_prepare(structinode*inode,+conststructfileattr*old_ma,+structfileattr*fa)+{+interr;++/*+*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby+*therelevantcapability.+*/+if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&+!capable(CAP_LINUX_IMMUTABLE))+return-EPERM;++err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);+if(err)+returnerr;++/*+*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit+*namespace.Enforcethatrestrictiononlyifwearetryingtochange+*thequotaIDstate.Everythingelseisallowedinusernamespaces.+*/+if(current_user_ns()!=&init_user_ns){+if(old_ma->fsx_projid!=fa->fsx_projid)+return-EINVAL;+if((old_ma->fsx_xflags^fa->fsx_xflags)&+FS_XFLAG_PROJINHERIT)+return-EINVAL;+}else{+/*+*CallerisallowedtochangetheprojectID.Ifitisbeing+*changed,makesurethatthenewvalueisvalid.+*/+if(old_ma->fsx_projid!=fa->fsx_projid&&+!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))+return-EINVAL;+}++/* Check extent size hints. */+if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&+!S_ISDIR(inode->i_mode))+return-EINVAL;++if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&+!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))+return-EINVAL;++/*+*ItisonlyvalidtosettheDAXflagonregularfilesand+*directoriesonfilesystems.+*/+if((fa->fsx_xflags&FS_XFLAG_DAX)&&+!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))+return-EINVAL;++/* Extent size hints of zero turn off the flags. */+if(fa->fsx_extsize==0)+fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);+if(fa->fsx_cowextsize==0)+fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;++return0;+}++/**+*vfs_fileattr_set-changemiscellaneousfileattributes+*@idmap:idmapofthemount+*@dentry:theobjecttochange+*@fa:fileattrpointer+*+*Afterverifyingpermissions,calli_op->fileattr_set()callback,if+*exists.+*+*Verifyingattributesinvolvesretrievingcurrentattributeswith+*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave+*notbeensetbythecallertocurrentvalues.Inodelockisheld+*thoughouttopreventracingwithanotherinstance.+*+*Return:0onsuccess,oranegativeerroronfailure.+*/+intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,+structfileattr*fa)+{+structinode*inode=d_inode(dentry);+structfileattrold_ma={};+interr;++if(!inode->i_op->fileattr_set)+return-ENOIOCTLCMD;++if(!inode_owner_or_capable(idmap,inode))+return-EPERM;++inode_lock(inode);+err=vfs_fileattr_get(dentry,&old_ma);+if(!err){+/* initialize missing bits from old_ma */+if(fa->flags_valid){+fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;+fa->fsx_extsize=old_ma.fsx_extsize;+fa->fsx_nextents=old_ma.fsx_nextents;+fa->fsx_projid=old_ma.fsx_projid;+fa->fsx_cowextsize=old_ma.fsx_cowextsize;+}else{+fa->flags|=old_ma.flags&~FS_COMMON_FL;+}+err=fileattr_set_prepare(inode,&old_ma,fa);+if(!err)+err=inode->i_op->fileattr_set(idmap,dentry,fa);+}+inode_unlock(inode);++returnerr;+}+EXPORT_SYMBOL(vfs_fileattr_set);++intioctl_getflags(structfile*file,unsignedint__user*argp)+{+structfileattrfa={.flags_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=put_user(fa.flags,argp);+returnerr;+}+EXPORT_SYMBOL(ioctl_getflags);++intioctl_setflags(structfile*file,unsignedint__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+unsignedintflags;+interr;++err=get_user(flags,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+fileattr_fill_flags(&fa,flags);+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_setflags);++intioctl_fsgetxattr(structfile*file,void__user*argp)+{+structfileattrfa={.fsx_valid=true};/* hint only */+interr;++err=vfs_fileattr_get(file->f_path.dentry,&fa);+if(!err)+err=copy_fsxattr_to_user(&fa,argp);++returnerr;+}+EXPORT_SYMBOL(ioctl_fsgetxattr);++intioctl_fssetxattr(structfile*file,void__user*argp)+{+structmnt_idmap*idmap=file_mnt_idmap(file);+structdentry*dentry=file->f_path.dentry;+structfileattrfa;+interr;++err=copy_fsxattr_from_user(&fa,argp);+if(!err){+err=mnt_want_write_file(file);+if(!err){+err=vfs_fileattr_set(idmap,dentry,&fa);+mnt_drop_write_file(file);+}+}+returnerr;+}+EXPORT_SYMBOL(ioctl_fssetxattr);
@@ -453,315 +453,6 @@ static int ioctl_file_dedupe_range(struct file *file,returnret;}-/**-*fileattr_fill_xflags-initializefileattrwithxflags-*@fa:fileattrpointer-*@xflags:FS_XFLAG_*flags-*-*Set->fsx_xflags,->fsx_validand->flags(translatedxflags).All-*otherfieldsarezeroed.-*/-voidfileattr_fill_xflags(structfileattr*fa,u32xflags)-{-memset(fa,0,sizeof(*fa));-fa->fsx_valid=true;-fa->fsx_xflags=xflags;-if(fa->fsx_xflags&FS_XFLAG_IMMUTABLE)-fa->flags|=FS_IMMUTABLE_FL;-if(fa->fsx_xflags&FS_XFLAG_APPEND)-fa->flags|=FS_APPEND_FL;-if(fa->fsx_xflags&FS_XFLAG_SYNC)-fa->flags|=FS_SYNC_FL;-if(fa->fsx_xflags&FS_XFLAG_NOATIME)-fa->flags|=FS_NOATIME_FL;-if(fa->fsx_xflags&FS_XFLAG_NODUMP)-fa->flags|=FS_NODUMP_FL;-if(fa->fsx_xflags&FS_XFLAG_DAX)-fa->flags|=FS_DAX_FL;-if(fa->fsx_xflags&FS_XFLAG_PROJINHERIT)-fa->flags|=FS_PROJINHERIT_FL;-}-EXPORT_SYMBOL(fileattr_fill_xflags);--/**-*fileattr_fill_flags-initializefileattrwithflags-*@fa:fileattrpointer-*@flags:FS_*_FLflags-*-*Set->flags,->flags_validand->fsx_xflags(translatedflags).-*Allotherfieldsarezeroed.-*/-voidfileattr_fill_flags(structfileattr*fa,u32flags)-{-memset(fa,0,sizeof(*fa));-fa->flags_valid=true;-fa->flags=flags;-if(fa->flags&FS_SYNC_FL)-fa->fsx_xflags|=FS_XFLAG_SYNC;-if(fa->flags&FS_IMMUTABLE_FL)-fa->fsx_xflags|=FS_XFLAG_IMMUTABLE;-if(fa->flags&FS_APPEND_FL)-fa->fsx_xflags|=FS_XFLAG_APPEND;-if(fa->flags&FS_NODUMP_FL)-fa->fsx_xflags|=FS_XFLAG_NODUMP;-if(fa->flags&FS_NOATIME_FL)-fa->fsx_xflags|=FS_XFLAG_NOATIME;-if(fa->flags&FS_DAX_FL)-fa->fsx_xflags|=FS_XFLAG_DAX;-if(fa->flags&FS_PROJINHERIT_FL)-fa->fsx_xflags|=FS_XFLAG_PROJINHERIT;-}-EXPORT_SYMBOL(fileattr_fill_flags);--/**-*vfs_fileattr_get-retrievemiscellaneousfileattributes-*@dentry:theobjecttoretrievefrom-*@fa:fileattrpointer-*-*Calli_op->fileattr_get()callback,ifexists.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_get(structdentry*dentry,structfileattr*fa)-{-structinode*inode=d_inode(dentry);--if(!inode->i_op->fileattr_get)-return-ENOIOCTLCMD;--returninode->i_op->fileattr_get(dentry,fa);-}-EXPORT_SYMBOL(vfs_fileattr_get);--/**-*copy_fsxattr_to_user-copyfsxattrtouserspace.-*@fa:fileattrpointer-*@ufa:fsxattruserpointer-*-*Return:0onsuccess,or-EFAULTonfailure.-*/-intcopy_fsxattr_to_user(conststructfileattr*fa,structfsxattr__user*ufa)-{-structfsxattrxfa;--memset(&xfa,0,sizeof(xfa));-xfa.fsx_xflags=fa->fsx_xflags;-xfa.fsx_extsize=fa->fsx_extsize;-xfa.fsx_nextents=fa->fsx_nextents;-xfa.fsx_projid=fa->fsx_projid;-xfa.fsx_cowextsize=fa->fsx_cowextsize;--if(copy_to_user(ufa,&xfa,sizeof(xfa)))-return-EFAULT;--return0;-}-EXPORT_SYMBOL(copy_fsxattr_to_user);--staticintcopy_fsxattr_from_user(structfileattr*fa,-structfsxattr__user*ufa)-{-structfsxattrxfa;--if(copy_from_user(&xfa,ufa,sizeof(xfa)))-return-EFAULT;--fileattr_fill_xflags(fa,xfa.fsx_xflags);-fa->fsx_extsize=xfa.fsx_extsize;-fa->fsx_nextents=xfa.fsx_nextents;-fa->fsx_projid=xfa.fsx_projid;-fa->fsx_cowextsize=xfa.fsx_cowextsize;--return0;-}--/*-*GenericfunctiontocheckFS_IOC_FSSETXATTR/FS_IOC_SETFLAGSvaluesandreject-*anyinvalidconfigurations.-*-*Note:mustbecalledwithinodelockheld.-*/-staticintfileattr_set_prepare(structinode*inode,-conststructfileattr*old_ma,-structfileattr*fa)-{-interr;--/*-*TheIMMUTABLEandAPPEND_ONLYflagscanonlybechangedby-*therelevantcapability.-*/-if((fa->flags^old_ma->flags)&(FS_APPEND_FL|FS_IMMUTABLE_FL)&&-!capable(CAP_LINUX_IMMUTABLE))-return-EPERM;--err=fscrypt_prepare_setflags(inode,old_ma->flags,fa->flags);-if(err)-returnerr;--/*-*ProjectQuotaIDstateisonlyallowedtochangefromwithintheinit-*namespace.Enforcethatrestrictiononlyifwearetryingtochange-*thequotaIDstate.Everythingelseisallowedinusernamespaces.-*/-if(current_user_ns()!=&init_user_ns){-if(old_ma->fsx_projid!=fa->fsx_projid)-return-EINVAL;-if((old_ma->fsx_xflags^fa->fsx_xflags)&-FS_XFLAG_PROJINHERIT)-return-EINVAL;-}else{-/*-*CallerisallowedtochangetheprojectID.Ifitisbeing-*changed,makesurethatthenewvalueisvalid.-*/-if(old_ma->fsx_projid!=fa->fsx_projid&&-!projid_valid(make_kprojid(&init_user_ns,fa->fsx_projid)))-return-EINVAL;-}--/* Check extent size hints. */-if((fa->fsx_xflags&FS_XFLAG_EXTSIZE)&&!S_ISREG(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_EXTSZINHERIT)&&-!S_ISDIR(inode->i_mode))-return-EINVAL;--if((fa->fsx_xflags&FS_XFLAG_COWEXTSIZE)&&-!S_ISREG(inode->i_mode)&&!S_ISDIR(inode->i_mode))-return-EINVAL;--/*-*ItisonlyvalidtosettheDAXflagonregularfilesand-*directoriesonfilesystems.-*/-if((fa->fsx_xflags&FS_XFLAG_DAX)&&-!(S_ISREG(inode->i_mode)||S_ISDIR(inode->i_mode)))-return-EINVAL;--/* Extent size hints of zero turn off the flags. */-if(fa->fsx_extsize==0)-fa->fsx_xflags&=~(FS_XFLAG_EXTSIZE|FS_XFLAG_EXTSZINHERIT);-if(fa->fsx_cowextsize==0)-fa->fsx_xflags&=~FS_XFLAG_COWEXTSIZE;--return0;-}--/**-*vfs_fileattr_set-changemiscellaneousfileattributes-*@idmap:idmapofthemount-*@dentry:theobjecttochange-*@fa:fileattrpointer-*-*Afterverifyingpermissions,calli_op->fileattr_set()callback,if-*exists.-*-*Verifyingattributesinvolvesretrievingcurrentattributeswith-*i_op->fileattr_get(),thisalsoallowsinitializingattributesthathave-*notbeensetbythecallertocurrentvalues.Inodelockisheld-*thoughouttopreventracingwithanotherinstance.-*-*Return:0onsuccess,oranegativeerroronfailure.-*/-intvfs_fileattr_set(structmnt_idmap*idmap,structdentry*dentry,-structfileattr*fa)-{-structinode*inode=d_inode(dentry);-structfileattrold_ma={};-interr;--if(!inode->i_op->fileattr_set)-return-ENOIOCTLCMD;--if(!inode_owner_or_capable(idmap,inode))-return-EPERM;--inode_lock(inode);-err=vfs_fileattr_get(dentry,&old_ma);-if(!err){-/* initialize missing bits from old_ma */-if(fa->flags_valid){-fa->fsx_xflags|=old_ma.fsx_xflags&~FS_XFLAG_COMMON;-fa->fsx_extsize=old_ma.fsx_extsize;-fa->fsx_nextents=old_ma.fsx_nextents;-fa->fsx_projid=old_ma.fsx_projid;-fa->fsx_cowextsize=old_ma.fsx_cowextsize;-}else{-fa->flags|=old_ma.flags&~FS_COMMON_FL;-}-err=fileattr_set_prepare(inode,&old_ma,fa);-if(!err)-err=inode->i_op->fileattr_set(idmap,dentry,fa);-}-inode_unlock(inode);--returnerr;-}-EXPORT_SYMBOL(vfs_fileattr_set);--staticintioctl_getflags(structfile*file,unsignedint__user*argp)-{-structfileattrfa={.flags_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=put_user(fa.flags,argp);-returnerr;-}--staticintioctl_setflags(structfile*file,unsignedint__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-unsignedintflags;-interr;--err=get_user(flags,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-fileattr_fill_flags(&fa,flags);-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}--staticintioctl_fsgetxattr(structfile*file,void__user*argp)-{-structfileattrfa={.fsx_valid=true};/* hint only */-interr;--err=vfs_fileattr_get(file->f_path.dentry,&fa);-if(!err)-err=copy_fsxattr_to_user(&fa,argp);--returnerr;-}--staticintioctl_fssetxattr(structfile*file,void__user*argp)-{-structmnt_idmap*idmap=file_mnt_idmap(file);-structdentry*dentry=file->f_path.dentry;-structfileattrfa;-interr;--err=copy_fsxattr_from_user(&fa,argp);-if(!err){-err=mnt_want_write_file(file);-if(!err){-err=vfs_fileattr_set(idmap,dentry,&fa);-mnt_drop_write_file(file);-}-}-returnerr;-}-staticintioctl_getfsuuid(structfile*file,void__user*argp){structsuper_block*sb=file_inode(file)->i_sb;
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-01 18:18:13
On Mon, Jun 30, 2025 at 06:20:12PM +0200, Andrey Albershteyn wrote:
Introduce new hooks for setting and getting filesystem extended
attributes on inode (FS_IOC_FSGETXATTR).
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
I wonder, were FS_IOC_FS[GS]ETXATTR already covered by the
security_file_ioctl hook? If so, will an out of date security policy
on a 6.17 kernel now fail to check the new file_[gs]etattr syscalls?
Though AFAICT the future of managing these "extra" file attributes is
the system call so it's probably appropriate to have an explicit
callout to LSMs.
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
--D
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-01 18:18:57
On Mon, Jun 30, 2025 at 06:20:14PM +0200, Andrey Albershteyn wrote:
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
With EOPNOSUPP -> EOPNOTSUPP corrected,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-01 18:31:06
On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote:
quoted hunk
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
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?
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
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-01 18:43:18
On Mon, Jun 30, 2025 at 06:20:16PM +0200, Andrey Albershteyn wrote:
From: Andrey Albershteyn <redacted>
Introduce file_getattr() and file_setattr() syscalls to manipulate inode
extended attributes. The syscalls takes pair of file descriptor and
pathname. Then it operates on inode opened accroding to openat()
semantics. The struct fsx_fileattr is passed to obtain/change extended
attributes.
This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
that file don't need to be open as we can reference it with a path
instead of fd. By having this we can manipulated inode extended
attributes not only on regular files but also on special ones. This
is not possible with FS_IOC_FSSETXATTR ioctl as with special files
we can not call ioctl() directly on the filesystem inode using fd.
This patch adds two new syscalls which allows userspace to get/set
extended inode attributes on special files by using parent directory
and a path - *at() like syscall.
CC: linux-api@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-xfs@vger.kernel.org
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
More whining/bikeshedding to come.
<snip stuff that looks ok to me>
<<well, I still dislike the CLASS(fd, fd)(fd) syntax...>>
@@ -148,6 +148,24 @@ struct fsxattr {unsignedcharfsx_pad[8];};+/*+*Variablesizestructureforfile_[sg]et_attr().+*+*Note.Thisisalternativetothestructure'structfileattr'/'structfsxattr'.+*Asthisstructureispassedto/fromuserspacewithitssize,thiscan+*beversionedbasedonthesize.+*/+structfsx_fileattr{+__u32fsx_xflags;/* xflags field value (get/set) */
Should this to be __u64 from the start? Seeing as (a) this struct is
not already a multiple of 8 bytes and (b) it's likely that we'll have to
add a u64 field at some point. That would also address brauner's
comment about padding.
--D
quoted hunk
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */
+};
+
+#define FSX_FILEATTR_SIZE_VER0 20
+#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0
+
/*
* Flags for the fsx_xflags field
*/
@@ -408,3 +408,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -148,6 +148,24 @@ struct fsxattr {unsignedcharfsx_pad[8];};+/*+*Variablesizestructureforfile_[sg]et_attr().+*+*Note.Thisisalternativetothestructure'structfileattr'/'structfsxattr'.+*Asthisstructureispassedto/fromuserspacewithitssize,thiscan+*beversionedbasedonthesize.+*/+structfsx_fileattr{+__u32fsx_xflags;/* xflags field value (get/set) */
Should this to be __u64 from the start? Seeing as (a) this struct is
not already a multiple of 8 bytes and (b) it's likely that we'll have to
add a u64 field at some point. That would also address brauner's
comment about padding.
Hello!
As I have already mentioned, after this syscall API/ABI is finished, I'm
planning to prepare patches for changing just selected fields / flags by
introducing a new mask field, and support for additional flags used by
existing filesystems (like windows flags).
My idea is extending this structure for a new "u32 fsx_xflags_mask"
and new "u32 fsx_xflags2" + "u32 fsx_xflags2_mask". (field names are
just examples).
So in case you are extending the structure now, please consider if it
makes sense to add all members, so we do not have to define 2 or 3
structure versions in near feature.
Your idea of __u64 for fsx_xflags means that it will already cover the
"u32 fsx_xflags2" field.
--D
quoted
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */
+};
+
+#define FSX_FILEATTR_SIZE_VER0 20
+#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0
+
/*
* Flags for the fsx_xflags field
*/
@@ -408,3 +408,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -148,6 +148,24 @@ struct fsxattr {unsignedcharfsx_pad[8];};+/*+*Variablesizestructureforfile_[sg]et_attr().+*+*Note.Thisisalternativetothestructure'structfileattr'/'structfsxattr'.+*Asthisstructureispassedto/fromuserspacewithitssize,thiscan+*beversionedbasedonthesize.+*/+structfsx_fileattr{+__u32fsx_xflags;/* xflags field value (get/set) */
Should this to be __u64 from the start? Seeing as (a) this struct is
not already a multiple of 8 bytes and (b) it's likely that we'll have to
add a u64 field at some point. That would also address brauner's
comment about padding.
Hello!
As I have already mentioned, after this syscall API/ABI is finished, I'm
planning to prepare patches for changing just selected fields / flags by
introducing a new mask field, and support for additional flags used by
existing filesystems (like windows flags).
My idea is extending this structure for a new "u32 fsx_xflags_mask"
and new "u32 fsx_xflags2" + "u32 fsx_xflags2_mask". (field names are
just examples).
So in case you are extending the structure now, please consider if it
makes sense to add all members, so we do not have to define 2 or 3
structure versions in near feature.
Your idea of __u64 for fsx_xflags means that it will already cover the
"u32 fsx_xflags2" field.
Ah, ok, so that work *is* still coming. :)
Are you still planning to add masks for xflags bits that are clearable
and settable? i.e.
__u64 fa_xflags; /* state */
...
<end of V0 structure>
__u64 fa_xflags_mask; /* bits for setattr to examine */
__u64 fa_xflags_clearable; /* clearable bits */
__u64 fa_xflags_settable; /* settable bits */
I think it's easier just to define u64 in the V0 structure and then add
the three new fields in V1. What do you think?
--D
quoted
--D
quoted
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */
+};
+
+#define FSX_FILEATTR_SIZE_VER0 20
+#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0
+
/*
* Flags for the fsx_xflags field
*/
@@ -408,3 +408,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
@@ -148,6 +148,24 @@ struct fsxattr {unsignedcharfsx_pad[8];};+/*+*Variablesizestructureforfile_[sg]et_attr().+*+*Note.Thisisalternativetothestructure'structfileattr'/'structfsxattr'.+*Asthisstructureispassedto/fromuserspacewithitssize,thiscan+*beversionedbasedonthesize.+*/+structfsx_fileattr{+__u32fsx_xflags;/* xflags field value (get/set) */
Should this to be __u64 from the start? Seeing as (a) this struct is
not already a multiple of 8 bytes and (b) it's likely that we'll have to
add a u64 field at some point. That would also address brauner's
comment about padding.
Hello!
As I have already mentioned, after this syscall API/ABI is finished, I'm
planning to prepare patches for changing just selected fields / flags by
introducing a new mask field, and support for additional flags used by
existing filesystems (like windows flags).
My idea is extending this structure for a new "u32 fsx_xflags_mask"
and new "u32 fsx_xflags2" + "u32 fsx_xflags2_mask". (field names are
just examples).
So in case you are extending the structure now, please consider if it
makes sense to add all members, so we do not have to define 2 or 3
structure versions in near feature.
Your idea of __u64 for fsx_xflags means that it will already cover the
"u32 fsx_xflags2" field.
Ah, ok, so that work *is* still coming. :)
Yes. I'm just waiting until this patch series is accepted.
In past I have already sent RFC patches to the list which modifies the
existing ioctl interface. So you can look at it if you want :-)
Are you still planning to add masks for xflags bits that are clearable
and settable? i.e.
__u64 fa_xflags; /* state */
...
<end of V0 structure>
__u64 fa_xflags_mask; /* bits for setattr to examine */
__u64 fa_xflags_clearable; /* clearable bits */
__u64 fa_xflags_settable; /* settable bits */
I think it's easier just to define u64 in the V0 structure and then add
the three new fields in V1. What do you think?
I wanted the interface which would allow to atomically change specified
bit/flag without the need for get-modify-set. And I think that this
would not work as the fa_xflags requires the state.
My idea is following:
__u64 fa_xflags;
...
<end of V0 structure>
__u64 fa_xflags_mask;
The fa_xflags_mask will specify which bits from the fa_xflags and from
other fa_* fields in V0 struct are going to be changed.
--D
quoted
quoted
--D
quoted
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */
+};
+
+#define FSX_FILEATTR_SIZE_VER0 20
+#define FSX_FILEATTR_SIZE_LATEST FSX_FILEATTR_SIZE_VER0
+
/*
* Flags for the fsx_xflags field
*/
@@ -408,3 +408,5 @@ 465 common listxattrat sys_listxattrat 466 common removexattrat sys_removexattrat 467 common open_tree_attr sys_open_tree_attr+468 common file_getattr sys_file_getattr+469 common file_setattr sys_file_setattr
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-01 19:27:52
On Tue, Jul 1, 2025 at 8:31 PM Darrick J. Wong [off-list ref] wrote:
On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote:
quoted
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
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. ;)
I think Pali has a plan on how to ensure that later
when the mask is provided via the API.
Though couldn't that be:
xfa.fsx_xflags = fa->fsx_xflags & FS_XFLAGS_MASK;
instead? And same below?
Indeed. There is a reason for the var, because the next series
by Pali will use a user provided mask, which defaults to FS_XFLAGS_MASK,
so I left it this way.
I don't see a problem with it keeping as is, but if it bothers you
I guess we can re-add the var later.
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
We have beaten this API almost to death for sure ;)
I don't remember if we discussed this specific aspect,
but I am personally in favor of
EOPNOTSUPP := the fs does not support the set/get operation
EINVAL := some flags provided as value is invalid
For example, if the get API provides you with a mask of the
valid flags that you can set, if you try to set flags outside of
that mask you get EINVAL.
That's my interpretation, but I agree that EOPNOTSUPP can also
make sense in this situation.
Thanks,
Amir.
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-01 19:40:03
On Tue, Jul 01, 2025 at 09:27:38PM +0200, Amir Goldstein wrote:
On Tue, Jul 1, 2025 at 8:31 PM Darrick J. Wong [off-list ref] wrote:
quoted
On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote:
quoted
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
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. ;)
I think Pali has a plan on how to ensure that later
when the mask is provided via the API.
quoted
Though couldn't that be:
xfa.fsx_xflags = fa->fsx_xflags & FS_XFLAGS_MASK;
instead? And same below?
Indeed. There is a reason for the var, because the next series
by Pali will use a user provided mask, which defaults to FS_XFLAGS_MASK,
so I left it this way.
I don't see a problem with it keeping as is, but if it bothers you
I guess we can re-add the var later.
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
We have beaten this API almost to death for sure ;)
I don't remember if we discussed this specific aspect,
but I am personally in favor of
EOPNOTSUPP := the fs does not support the set/get operation
EINVAL := some flags provided as value is invalid
For example, if the get API provides you with a mask of the
valid flags that you can set, if you try to set flags outside of
that mask you get EINVAL.
That's my interpretation, but I agree that EOPNOTSUPP can also
make sense in this situation.
<nod> I think I'd rather EOPNOTSUPP for "bits are set that the kernel
doesn't recognize" and EINVAL (or maybe something else like
EPROTONOSUPPORT) for "fs driver will not let you change this bit".
At least for the syscall interface; we probably have to flatten that to
EOPNOTSUPP for both legacy ioctls.
--D
On Tuesday 01 July 2025 12:40:02 Darrick J. Wong wrote:
On Tue, Jul 01, 2025 at 09:27:38PM +0200, Amir Goldstein wrote:
quoted
On Tue, Jul 1, 2025 at 8:31 PM Darrick J. Wong [off-list ref] wrote:
quoted
On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote:
quoted
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
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. ;)
I think Pali has a plan on how to ensure that later
when the mask is provided via the API.
quoted
Though couldn't that be:
xfa.fsx_xflags = fa->fsx_xflags & FS_XFLAGS_MASK;
instead? And same below?
Indeed. There is a reason for the var, because the next series
by Pali will use a user provided mask, which defaults to FS_XFLAGS_MASK,
so I left it this way.
I don't see a problem with it keeping as is, but if it bothers you
I guess we can re-add the var later.
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
We have beaten this API almost to death for sure ;)
I don't remember if we discussed this specific aspect,
but I am personally in favor of
EOPNOTSUPP := the fs does not support the set/get operation
EINVAL := some flags provided as value is invalid
For example, if the get API provides you with a mask of the
valid flags that you can set, if you try to set flags outside of
that mask you get EINVAL.
That's my interpretation, but I agree that EOPNOTSUPP can also
make sense in this situation.
<nod> I think I'd rather EOPNOTSUPP for "bits are set that the kernel
doesn't recognize" and EINVAL (or maybe something else like
EPROTONOSUPPORT) for "fs driver will not let you change this bit".
At least for the syscall interface; we probably have to flatten that to
EOPNOTSUPP for both legacy ioctls.
... and this starting to be complicated if the "fs driver" is network
based (as fs driver can support, but remote server not). See also:
https://lore.kernel.org/linux-fsdevel/20241224160535.pi6nazpugqkhvfns@pali/t/#u
For backup/restore application it would be very useful to distinguish between:
- "kernel does not support flag X"
- "target filesystem does not support flag X"
- "wrong structure was passed / syscall incorrectly called"
third option is bug in application - fatal error. second option is just
a warning for user (sorry, we cannot set NEW FEATURE on FAT32, but if
you would do restore to other fs, it is supported). and first option
happens when you run new application on older kernel version, it is an
recoverable error (or warning to user, but with more important level
then second option as switching to different FS would not help).
Could we return different errnos for these 3 situations?
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-02 07:03:43
On Tue, Jul 1, 2025 at 9:54 PM Pali Rohár [off-list ref] wrote:
On Tuesday 01 July 2025 12:40:02 Darrick J. Wong wrote:
quoted
On Tue, Jul 01, 2025 at 09:27:38PM +0200, Amir Goldstein wrote:
quoted
On Tue, Jul 1, 2025 at 8:31 PM Darrick J. Wong [off-list ref] wrote:
quoted
On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote:
quoted
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
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. ;)
I think Pali has a plan on how to ensure that later
when the mask is provided via the API.
quoted
Though couldn't that be:
xfa.fsx_xflags = fa->fsx_xflags & FS_XFLAGS_MASK;
instead? And same below?
Indeed. There is a reason for the var, because the next series
by Pali will use a user provided mask, which defaults to FS_XFLAGS_MASK,
so I left it this way.
I don't see a problem with it keeping as is, but if it bothers you
I guess we can re-add the var later.
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
We have beaten this API almost to death for sure ;)
I don't remember if we discussed this specific aspect,
but I am personally in favor of
EOPNOTSUPP := the fs does not support the set/get operation
EINVAL := some flags provided as value is invalid
For example, if the get API provides you with a mask of the
valid flags that you can set, if you try to set flags outside of
that mask you get EINVAL.
That's my interpretation, but I agree that EOPNOTSUPP can also
make sense in this situation.
<nod> I think I'd rather EOPNOTSUPP for "bits are set that the kernel
doesn't recognize" and EINVAL (or maybe something else like
EPROTONOSUPPORT) for "fs driver will not let you change this bit".
At least for the syscall interface; we probably have to flatten that to
EOPNOTSUPP for both legacy ioctls.
Given the precedents of returning EOPNOTSUPP in xfs_fileattr_set()
and ext4_ioctl_setflags() for flags that cannot be set, I agree.
... and this starting to be complicated if the "fs driver" is network
based (as fs driver can support, but remote server not). See also:
https://lore.kernel.org/linux-fsdevel/20241224160535.pi6nazpugqkhvfns@pali/t/#u
For backup/restore application it would be very useful to distinguish between:
- "kernel does not support flag X"
- "target filesystem does not support flag X"
- "wrong structure was passed / syscall incorrectly called"
third option is bug in application - fatal error. second option is just
a warning for user (sorry, we cannot set NEW FEATURE on FAT32, but if
you would do restore to other fs, it is supported). and first option
happens when you run new application on older kernel version, it is an
recoverable error (or warning to user, but with more important level
then second option as switching to different FS would not help).
Could we return different errnos for these 3 situations?
That would be nice, but actually according to your plan
the get API returns the mask of flags supported by the filesystem
(on that specific object even), so userspace in fact has a way to
distinguish between the first two EOPNOTSUPP cases.
Thanks,
Amir.
On Mon, Jun 30, 2025 at 06:20:12PM +0200, Andrey Albershteyn wrote:
quoted
Introduce new hooks for setting and getting filesystem extended
attributes on inode (FS_IOC_FSGETXATTR).
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
I wonder, were FS_IOC_FS[GS]ETXATTR already covered by the
security_file_ioctl hook?
looks like
If so, will an out of date security policy
on a 6.17 kernel now fail to check the new file_[gs]etattr syscalls?
Yeah, probably, not sure if policies can have 'don't allow unknown'
but this is probably will need to be updated in the policy
Though AFAICT the future of managing these "extra" file attributes is
the system call so it's probably appropriate to have an explicit
callout to LSMs.
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
--D
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-02 09:14:01
quoted
+/*
+ * Variable size structure for file_[sg]et_attr().
+ *
+ * Note. This is alternative to the structure 'struct fileattr'/'struct fsxattr'.
+ * As this structure is passed to/from userspace with its size, this can
+ * be versioned based on the size.
+ */
+struct fsx_fileattr {
+ __u32 fsx_xflags; /* xflags field value (get/set) */
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */
This misses a:
__u32 __spare;
so there's no holes in the struct. :)
Adding __spare and not verifying that it is zeroed gets us to the
point that we are not able to replace __spare with a real field later.
I suggest to resolve this hole as Darrick and Pali suggested by making it
__u64 fsx_xflags
w.r.t Darrick's comment, I kind of like it that the name for the UAPI
struct (fsxattr)
differs from the name of the kernel internal representation (fileattr), but
I agree that fsx_fileattr does not give a good hint on what it is.
I think that renaming struct fsx_fileattr to struct fsxattr64 along
with changing the
width of fsx_xflags will help reduce the confusion of users.
What do you guys think?
Thanks,
Amir.
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-02 09:48:53
On Wed, Jul 2, 2025 at 9:03 AM Amir Goldstein [off-list ref] wrote:
On Tue, Jul 1, 2025 at 9:54 PM Pali Rohár [off-list ref] wrote:
quoted
On Tuesday 01 July 2025 12:40:02 Darrick J. Wong wrote:
quoted
On Tue, Jul 01, 2025 at 09:27:38PM +0200, Amir Goldstein wrote:
quoted
On Tue, Jul 1, 2025 at 8:31 PM Darrick J. Wong [off-list ref] wrote:
quoted
On Mon, Jun 30, 2025 at 06:20:15PM +0200, Andrey Albershteyn wrote:
quoted
From: Amir Goldstein <amir73il@gmail.com>
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@kernel.org/
Cc: Pali Rohár <pali@kernel.org>
Cc: Andrey Albershteyn <redacted>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/file_attr.c | 8 +++++++-
include/linux/fileattr.h | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
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. ;)
I think Pali has a plan on how to ensure that later
when the mask is provided via the API.
quoted
Though couldn't that be:
xfa.fsx_xflags = fa->fsx_xflags & FS_XFLAGS_MASK;
instead? And same below?
Indeed. There is a reason for the var, because the next series
by Pali will use a user provided mask, which defaults to FS_XFLAGS_MASK,
so I left it this way.
I don't see a problem with it keeping as is, but if it bothers you
I guess we can re-add the var later.
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
We have beaten this API almost to death for sure ;)
I don't remember if we discussed this specific aspect,
but I am personally in favor of
EOPNOTSUPP := the fs does not support the set/get operation
EINVAL := some flags provided as value is invalid
For example, if the get API provides you with a mask of the
valid flags that you can set, if you try to set flags outside of
that mask you get EINVAL.
That's my interpretation, but I agree that EOPNOTSUPP can also
make sense in this situation.
<nod> I think I'd rather EOPNOTSUPP for "bits are set that the kernel
doesn't recognize" and EINVAL (or maybe something else like
EPROTONOSUPPORT) for "fs driver will not let you change this bit".
At least for the syscall interface; we probably have to flatten that to
EOPNOTSUPP for both legacy ioctls.
Given the precedents of returning EOPNOTSUPP in xfs_fileattr_set()
and ext4_ioctl_setflags() for flags that cannot be set, I agree.
Wait, I misparsed what you wrote, so I think I "agreed" only to the
first part of your suggestion.
My claim is that unlike the xfs_has_v3inodes() check in
xfs_ioctl_setattr_xflags(),
ext4/f2fs etc return EOPNOTSUPP for various flags depending on supported fs
features (e.g. casefold,dax,encryption), so I think it will be hard to
impose a strict rule
where "fs does not support the feature" returns EINVAL in the syscalls API.
Therefore, I propose to change the code in this patch to
return EOPNOTSUPP for flags that kernel does not support
and with coming changes from Pali, it will also return the same
EOPNOTSUPP for flags that the fs instance does not support.
Christian,
Can you please amend the return value in the following chunk:
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-02 12:40:24
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
More whining/bikeshedding to come.
<snip stuff that looks ok to me>
<<well, I still dislike the CLASS(fd, fd)(fd) syntax...>>
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-02 13:43:41
On Wed, Jul 2, 2025 at 2:40 PM Christian Brauner [off-list ref] wrote:
quoted
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
file_attr
I can see the allure, but we have a long history here with fsxattr,
so I think it serves the users better to reference this history with
fsxattr64.
That, and also, avoid the churn of s/fileattr/file_kattr/
If you want to do this renaming, please do it in the same PR
because I don't like the idea of having both file_attr and fileattr
in the tree for an unknown period.
Thanks,
Amir.
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-02 18:37:50
On Wed, Jul 02, 2025 at 03:43:28PM +0200, Amir Goldstein wrote:
On Wed, Jul 2, 2025 at 2:40 PM Christian Brauner [off-list ref] wrote:
quoted
quoted
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
file_attr
I can see the allure, but we have a long history here with fsxattr,
so I think it serves the users better to reference this history with
fsxattr64.
<shrug> XFS has a long history with 'struct fsxattr' (the structure you
passed to XFS_IOC_FSGETXATTR) but the rest of the kernel needn't be so
fixated upon the historical name. ext4/f2fs/overlay afaict are just
going along for the ride.
IOWs I like brauner's struct file_attr and struct file_kattr
suggestions.
That, and also, avoid the churn of s/fileattr/file_kattr/
If you want to do this renaming, please do it in the same PR
because I don't like the idea of having both file_attr and fileattr
in the tree for an unknown period.
But yeah, that ought to be a treewide change done at the same time.
--D
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-03 08:28:18
On Wed, Jul 02, 2025 at 11:37:50AM -0700, Darrick J. Wong wrote:
On Wed, Jul 02, 2025 at 03:43:28PM +0200, Amir Goldstein wrote:
quoted
On Wed, Jul 2, 2025 at 2:40 PM Christian Brauner [off-list ref] wrote:
quoted
quoted
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
file_attr
I can see the allure, but we have a long history here with fsxattr,
so I think it serves the users better to reference this history with
fsxattr64.
<shrug> XFS has a long history with 'struct fsxattr' (the structure you
passed to XFS_IOC_FSGETXATTR) but the rest of the kernel needn't be so
fixated upon the historical name. ext4/f2fs/overlay afaict are just
going along for the ride.
IOWs I like brauner's struct file_attr and struct file_kattr
suggestions.
quoted
That, and also, avoid the churn of s/fileattr/file_kattr/
If you want to do this renaming, please do it in the same PR
because I don't like the idea of having both file_attr and fileattr
in the tree for an unknown period.
But yeah, that ought to be a treewide change done at the same time.
Why do you all hate me? ;)
See the appended patch.
From: Amir Goldstein <amir73il@gmail.com> Date: 2025-07-03 08:42:41
On Thu, Jul 3, 2025 at 10:28 AM Christian Brauner [off-list ref] wrote:
On Wed, Jul 02, 2025 at 11:37:50AM -0700, Darrick J. Wong wrote:
quoted
On Wed, Jul 02, 2025 at 03:43:28PM +0200, Amir Goldstein wrote:
quoted
On Wed, Jul 2, 2025 at 2:40 PM Christian Brauner [off-list ref] wrote:
quoted
quoted
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
file_attr
I can see the allure, but we have a long history here with fsxattr,
so I think it serves the users better to reference this history with
fsxattr64.
<shrug> XFS has a long history with 'struct fsxattr' (the structure you
passed to XFS_IOC_FSGETXATTR) but the rest of the kernel needn't be so
fixated upon the historical name. ext4/f2fs/overlay afaict are just
going along for the ride.
IOWs I like brauner's struct file_attr and struct file_kattr
suggestions.
quoted
That, and also, avoid the churn of s/fileattr/file_kattr/
If you want to do this renaming, please do it in the same PR
because I don't like the idea of having both file_attr and fileattr
in the tree for an unknown period.
But yeah, that ought to be a treewide change done at the same time.
Why do you all hate me? ;)
See the appended patch.
This looks obviously fine, but I wonder how much conflicts that would
cause in linux-next?
It may just be small enough to get by.
Thanks,
Amir.
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-03 08:46:36
On Thu, Jul 03, 2025 at 10:42:27AM +0200, Amir Goldstein wrote:
On Thu, Jul 3, 2025 at 10:28 AM Christian Brauner [off-list ref] wrote:
quoted
On Wed, Jul 02, 2025 at 11:37:50AM -0700, Darrick J. Wong wrote:
quoted
On Wed, Jul 02, 2025 at 03:43:28PM +0200, Amir Goldstein wrote:
quoted
On Wed, Jul 2, 2025 at 2:40 PM Christian Brauner [off-list ref] wrote:
quoted
quoted
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
file_attr
I can see the allure, but we have a long history here with fsxattr,
so I think it serves the users better to reference this history with
fsxattr64.
<shrug> XFS has a long history with 'struct fsxattr' (the structure you
passed to XFS_IOC_FSGETXATTR) but the rest of the kernel needn't be so
fixated upon the historical name. ext4/f2fs/overlay afaict are just
going along for the ride.
IOWs I like brauner's struct file_attr and struct file_kattr
suggestions.
quoted
That, and also, avoid the churn of s/fileattr/file_kattr/
If you want to do this renaming, please do it in the same PR
because I don't like the idea of having both file_attr and fileattr
in the tree for an unknown period.
But yeah, that ought to be a treewide change done at the same time.
Why do you all hate me? ;)
See the appended patch.
This looks obviously fine, but I wonder how much conflicts that would
cause in linux-next?
It may just be small enough to get by.
With such changes that's always a possibility but really I'll just
provide a branch with the resolutions for Linus to pull.
From: "Darrick J. Wong" <djwong@kernel.org> Date: 2025-07-03 22:35:50
On Thu, Jul 03, 2025 at 10:46:30AM +0200, Christian Brauner wrote:
On Thu, Jul 03, 2025 at 10:42:27AM +0200, Amir Goldstein wrote:
quoted
On Thu, Jul 3, 2025 at 10:28 AM Christian Brauner [off-list ref] wrote:
quoted
On Wed, Jul 02, 2025 at 11:37:50AM -0700, Darrick J. Wong wrote:
quoted
On Wed, Jul 02, 2025 at 03:43:28PM +0200, Amir Goldstein wrote:
quoted
On Wed, Jul 2, 2025 at 2:40 PM Christian Brauner [off-list ref] wrote:
quoted
quoted
Er... "fsx_fileattr" is the struct that the system call uses?
That's a little confusing considering that xfs already has a
xfs_fill_fsxattr function that actually fills a struct fileattr.
That could be renamed xfs_fill_fileattr.
I dunno. There's a part of me that would really rather that the
file_getattr and file_setattr syscalls operate on a struct file_attr.
Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:
struct mount_attr vs struct mount_kattr
struct clone_args vs struct kernel_clone_kargs
etc.
file_attr
I can see the allure, but we have a long history here with fsxattr,
so I think it serves the users better to reference this history with
fsxattr64.
<shrug> XFS has a long history with 'struct fsxattr' (the structure you
passed to XFS_IOC_FSGETXATTR) but the rest of the kernel needn't be so
fixated upon the historical name. ext4/f2fs/overlay afaict are just
going along for the ride.
IOWs I like brauner's struct file_attr and struct file_kattr
suggestions.
quoted
That, and also, avoid the churn of s/fileattr/file_kattr/
If you want to do this renaming, please do it in the same PR
because I don't like the idea of having both file_attr and fileattr
in the tree for an unknown period.
But yeah, that ought to be a treewide change done at the same time.
Why do you all hate me? ;)
See the appended patch.
This looks obviously fine, but I wonder how much conflicts that would
cause in linux-next?
It may just be small enough to get by.
With such changes that's always a possibility but really I'll just
provide a branch with the resolutions for Linus to pull.
<nod> That looks good to me. :)
At worst you can always ask Linus "Hey I want to do a treewide name
change of $X to $Y, can I stuff that in at the very end of the merge
window?" and IME he'll let you do that. Even better if someone keeps
him supplied with fresh change patches.
--D
On Mon, Jun 30, 2025 at 06:20:10PM +0200, Andrey Albershteyn wrote:
quoted
This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
Only small nits I'm going to comment on that I can fix myself.
Otherwise looks great.
Hi Christian,
Let me know if you would like a new revision with all the comments
included (and your patch on file_kattr rename) or you good with
applying them while commit
--
- Andrey
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-07 12:19:30
On Mon, Jul 07, 2025 at 02:05:10PM +0200, Andrey Albershteyn wrote:
On 2025-07-01 14:29:42, Christian Brauner wrote:
quoted
On Mon, Jun 30, 2025 at 06:20:10PM +0200, Andrey Albershteyn wrote:
quoted
This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
Only small nits I'm going to comment on that I can fix myself.
Otherwise looks great.
Hi Christian,
Let me know if you would like a new revision with all the comments
included (and your patch on file_kattr rename) or you good with
applying them while commit
From: Christian Brauner <brauner@kernel.org> Date: 2025-07-07 12:20:21
On Mon, 30 Jun 2025 18:20:10 +0200, Andrey Albershteyn wrote:
This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
[...]
On Mon, Jul 07, 2025 at 02:05:10PM +0200, Andrey Albershteyn wrote:
quoted
On 2025-07-01 14:29:42, Christian Brauner wrote:
quoted
On Mon, Jun 30, 2025 at 06:20:10PM +0200, Andrey Albershteyn wrote:
quoted
This patchset introduced two new syscalls file_getattr() and
file_setattr(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get a fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
Only small nits I'm going to comment on that I can fix myself.
Otherwise looks great.
Hi Christian,
Let me know if you would like a new revision with all the comments
included (and your patch on file_kattr rename) or you good with
applying them while commit
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
...
quoted hunk
@@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) fileattr_fill_flags(&fa, flags); err = vfs_fileattr_set(idmap, dentry, &fa); mnt_drop_write_file(file);+ if (err == -EOPNOTSUPP)+ err = -ENOIOCTLCMD;
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
...
dumps in 6.16:
sf: ioctl: Operation not supported
with the above patch:
sf: ioctl: Inappropriate ioctl for device
Is this expected?
This does look like an unintentional bug: As far as I can see, the
-ENOIOCTLCMD was previously used to indicate that a particular filesystem
does not have a fileattr_{get,set} callback at all, while individual
filesystems used EOPNOSUPP to indicate that a particular attribute
flag is unsupported. With the double conversion, both error codes
get turned into a single one.
Arnd
From: Jan Kara <jack@suse.cz> Date: 2025-10-06 15:39:53
On Mon 06-10-25 13:09:05, Jiri Slaby wrote:
On 30. 06. 25, 18:20, Andrey Albershteyn wrote:
quoted
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
...
quoted
@@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) fileattr_fill_flags(&fa, flags); err = vfs_fileattr_set(idmap, dentry, &fa); mnt_drop_write_file(file);+ if (err == -EOPNOTSUPP)+ err = -ENOIOCTLCMD;
No, that's a bug and a clear userspace regression so we need to fix it. I
think we need to revert this commit and instead convert ENOIOCTLCMD from
vfs_fileattr_get/set() to EOPNOTSUPP in appropriate places. Andrey?
Honza
--
Jan Kara [off-list ref]
SUSE Labs, CR
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
...
quoted
@@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) fileattr_fill_flags(&fa, flags); err = vfs_fileattr_set(idmap, dentry, &fa); mnt_drop_write_file(file);+ if (err == -EOPNOTSUPP)+ err = -ENOIOCTLCMD;
No, that's a bug and a clear userspace regression so we need to fix it. I
think we need to revert this commit and instead convert ENOIOCTLCMD from
vfs_fileattr_get/set() to EOPNOTSUPP in appropriate places. Andrey?
From: Christian Brauner <brauner@kernel.org> Date: 2025-10-07 11:00:16
On Mon, Oct 06, 2025 at 08:52:32PM +0200, Andrey Albershteyn wrote:
On 2025-10-06 17:39:46, Jan Kara wrote:
quoted
On Mon 06-10-25 13:09:05, Jiri Slaby wrote:
quoted
On 30. 06. 25, 18:20, Andrey Albershteyn wrote:
quoted
Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
...
quoted
@@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) fileattr_fill_flags(&fa, flags); err = vfs_fileattr_set(idmap, dentry, &fa); mnt_drop_write_file(file);+ if (err == -EOPNOTSUPP)+ err = -ENOIOCTLCMD;
No, that's a bug and a clear userspace regression so we need to fix it. I
think we need to revert this commit and instead convert ENOIOCTLCMD from
vfs_fileattr_get/set() to EOPNOTSUPP in appropriate places. Andrey?