Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls

[Christian Brauner]

On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote:
> On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote:
> 
> >
> >     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;
 }