Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls
From: Christian Brauner <brauner@kernel.org>
Date: 2025-05-15 09:02:56
Also in:
linux-alpha, linux-arch, linux-fsdevel, linux-m68k, linux-mips, linux-s390, linux-security-module, linux-sh, linux-unionfs, linux-xfs, linuxppc-dev, lkml, selinux, sparclinux
Subsystem:
filesystems (vfs and infrastructure), landlock security module, the rest · Maintainers:
Alexander Viro, Christian Brauner, Mickaël Salaün, Linus Torvalds
On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote:
On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote:quoted
long syscall(SYS_file_getattr, int dirfd, const char *pathname, struct fsxattr *fsx, size_t size, unsigned int at_flags); long syscall(SYS_file_setattr, int dirfd, const char *pathname, struct fsxattr *fsx, size_t size, unsigned int at_flags);I don't think we can have both the "struct fsxattr" from the uapi headers, and a variable size as an additional argument. I would still prefer not having the extensible structure at all and just
We're not going to add new interfaces that are fixed size unless for the very basic cases. I don't care if we're doing that somewhere else in the kernel but we're not doing that for vfs apis.
use fsxattr, but if you want to make it extensible in this way, it should use a different structure (name). Otherwise adding fields after fsx_pad[] would break the ioctl interface.
Would that really be a problem? Just along the syscall simply add something like:
diff --git a/fs/ioctl.c b/fs/ioctl.c
index c91fd2b46a77..d3943805c4be 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c@@ -868,12 +868,6 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd, case FS_IOC_SETFLAGS: return ioctl_setflags(filp, argp); - case FS_IOC_FSGETXATTR: - return ioctl_fsgetxattr(filp, argp); - - case FS_IOC_FSSETXATTR: - return ioctl_fssetxattr(filp, argp); - case FS_IOC_GETFSUUID: return ioctl_getfsuuid(filp, argp);
@@ -886,6 +880,20 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd, break; } + switch (_IOC_NR(cmd)) { + case _IOC_NR(FS_IOC_FSGETXATTR): + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_FSGETXATTR))) + return SOMETHING_SOMETHING; + /* Only handle original size. */ + return ioctl_fsgetxattr(filp, argp); + + case _IOC_NR(FFS_IOC_FSSETXATTR): + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FFS_IOC_FSSETXATTR))) + return SOMETHING_SOMETHING; + /* Only handle original size. */ + return ioctl_fssetxattr(filp, argp); + } + return -ENOIOCTLCMD; }