On Tue, Feb 25, 2025 at 09:02:04AM +0100, Arnd Bergmann wrote:

On Mon, Feb 24, 2025, at 12:32, Christian Brauner wrote:

quoted

On Fri, Feb 21, 2025 at 08:15:24PM +0100, Amir Goldstein wrote:

quoted

On Fri, Feb 21, 2025 at 7:13 PM Darrick J. Wong [off-list ref] wrote:

quoted

quoted

quoted

quoted

@@ -23,6 +23,9 @@
 #include <linux/rw_hint.h>
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
+#include <linux/syscalls.h>
+#include <linux/fileattr.h>
+#include <linux/namei.h>
 #include <trace/events/writeback.h>
 #define CREATE_TRACE_POINTS
 #include <trace/events/timestamp.h>

@@ -2953,3 +2956,75 @@ umode_t mode_strip_sgid(struct mnt_idmap *idmap,
      return mode & ~S_ISGID;
 }
 EXPORT_SYMBOL(mode_strip_sgid);
+
+SYSCALL_DEFINE4(getfsxattrat, int, dfd, const char __user *, filename,
+             struct fsxattr __user *, fsx, unsigned int, at_flags)

Should the kernel require userspace to pass the size of the fsx buffer?
That way we avoid needing to rev the interface when we decide to grow
the structure.

Please version the struct by size as we do for clone3(),
mount_setattr(), listmount()'s struct mnt_id_req, sched_setattr(), all
the new xattrat*() system calls and a host of others. So laying out the
struct 64bit and passing a size alongside it.

This is all handled by copy_struct_from_user() and copy_struct_to_user()
so nothing to reinvent. And it's easy to copy from existing system
calls.

I don't think that works in this case, because 'struct fsxattr'
is an existing structure that is defined with a fixed size of
28 bytes. If we ever need more than 8 extra bytes, then the
existing ioctl commands are also broken.

Replacing fsxattr with an extensible structure of the same contents
would work, but I feel that just adds more complication for little
gain.

On the other hand, there is an open question about how unknown
flags and fields in this structure. FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR
treats them as optional and just ignores anything it doesn't
understand, while copy_struct_from_user() would treat any unknown
but set bytes as -E2BIG.

The ioctl interface relies on the existing behavior, see
0a6eab8bd4e0 ("vfs: support FS_XFLAG_COWEXTSIZE and get/set of
CoW extent size hint") for how it was previously extended
with an optional flag/word. I think that is fine for the syscall
as well, but should be properly documented since it is different
from how most syscalls work.

If we're doing a new system call I see no reason to limit us to a
pre-existing structure or structure layout.