lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ