linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Andrey Albershteyn <aalbersh@redhat.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	"Casey Schaufler" <casey@schaufler-ca.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>, "Pali Rohár" <pali@kernel.org>,
	"Paul Moore" <paul@paul-moore.com>,
	linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org,
	selinux@vger.kernel.org,
	"Andrey Albershteyn" <aalbersh@kernel.org>
Subject: Re: [PATCH v6 0/6] fs: introduce file_getattr and file_setattr syscalls
Date: Tue, 1 Jul 2025 08:11:33 +0200	[thread overview]
Message-ID: <CAOQ4uxgEK7zQFBTmg1cK=LWfBmZk7FhceHjJROP0RWP00--Tvg@mail.gmail.com> (raw)
In-Reply-To: <20250630-xattrat-syscall-v6-0-c4e3bc35227b@kernel.org>

On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn <aalbersh@redhat.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@kernel.org>
>

  parent reply	other threads:[~2025-07-01  6:11 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-30 16:20 [PATCH v6 0/6] fs: introduce file_getattr and file_setattr syscalls Andrey Albershteyn
2025-06-30 16:20 ` [PATCH v6 1/6] fs: split fileattr related helpers into separate file Andrey Albershteyn
2025-07-01  5:39   ` Amir Goldstein
2025-07-01 12:38   ` Jan Kara
2025-07-01 18:13   ` Darrick J. Wong
2025-06-30 16:20 ` [PATCH v6 2/6] lsm: introduce new hooks for setting/getting inode fsxattr Andrey Albershteyn
2025-07-01 12:39   ` Jan Kara
2025-07-01 18:18   ` Darrick J. Wong
2025-07-02  8:47     ` Andrey Albershteyn
2025-06-30 16:20 ` [PATCH v6 3/6] selinux: implement inode_file_[g|s]etattr hooks Andrey Albershteyn
2025-06-30 16:20 ` [PATCH v6 4/6] fs: make vfs_fileattr_[get|set] return -EOPNOSUPP Andrey Albershteyn
2025-06-30 18:05   ` Pali Rohár
2025-07-01  6:05   ` Amir Goldstein
2025-07-01 12:51     ` Jan Kara
2025-07-01 14:16       ` Amir Goldstein
2025-07-01 12:52   ` Jan Kara
2025-07-01 18:18   ` Darrick J. Wong
2025-06-30 16:20 ` [PATCH v6 5/6] fs: prepare for extending file_get/setattr() Andrey Albershteyn
2025-07-01 13:06   ` Jan Kara
2025-07-01 18:31   ` Darrick J. Wong
2025-07-01 19:27     ` Amir Goldstein
2025-07-01 19:40       ` Darrick J. Wong
2025-07-01 19:54         ` Pali Rohár
2025-07-02  7:03           ` Amir Goldstein
2025-07-02  9:48             ` Amir Goldstein
2025-07-02 12:24               ` Christian Brauner
2025-06-30 16:20 ` [PATCH v6 6/6] fs: introduce file_getattr and file_setattr syscalls Andrey Albershteyn
2025-07-01 12:34   ` Christian Brauner
2025-07-02  9:13     ` Amir Goldstein
2025-07-01 13:24   ` Jan Kara
2025-07-01 18:43   ` Darrick J. Wong
2025-07-01 18:54     ` Pali Rohár
2025-07-01 19:08       ` Darrick J. Wong
2025-07-01 19:17         ` Pali Rohár
2025-07-02 12:40     ` Christian Brauner
2025-07-02 13:43       ` Amir Goldstein
2025-07-02 18:37         ` Darrick J. Wong
2025-07-03  8:28           ` Christian Brauner
2025-07-03  8:42             ` Amir Goldstein
2025-07-03  8:46               ` Christian Brauner
2025-07-03 22:35                 ` Darrick J. Wong
2025-07-01  6:11 ` Amir Goldstein [this message]
2025-07-01 12:29 ` [PATCH v6 0/6] " Christian Brauner
2025-07-07 12:05   ` Andrey Albershteyn
2025-07-07 12:19     ` Christian Brauner
2025-07-07 12:27       ` Andrey Albershteyn
2025-07-07 12:19 ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOQ4uxgEK7zQFBTmg1cK=LWfBmZk7FhceHjJROP0RWP00--Tvg@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=aalbersh@kernel.org \
    --cc=aalbersh@redhat.com \
    --cc=arnd@arndb.de \
    --cc=brauner@kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=jack@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=pali@kernel.org \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).