[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOQ4uxgEK7zQFBTmg1cK=LWfBmZk7FhceHjJROP0RWP00--Tvg@mail.gmail.com>
Date: Tue, 1 Jul 2025 08:11:33 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Andrey Albershteyn <aalbersh@...hat.com>
Cc: Arnd Bergmann <arnd@...db.de>, Casey Schaufler <casey@...aufler-ca.com>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Pali Rohár <pali@...nel.org>,
Paul Moore <paul@...l-moore.com>, linux-api@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-xfs@...r.kernel.org, selinux@...r.kernel.org,
Andrey Albershteyn <aalbersh@...nel.org>
Subject: Re: [PATCH v6 0/6] fs: introduce file_getattr and file_setattr syscalls
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn <aalbersh@...hat.com> 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 <aalbersh@...nel.org>
>
Powered by blists - more mailing lists