From: "Pali Rohár" <pali@kernel.org>
To: Andrey Albershteyn <aalbersh@redhat.com>
Cc: "Amir Goldstein" <amir73il@gmail.com>,
"Darrick J. Wong" <djwong@kernel.org>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Matt Turner" <mattst88@gmail.com>,
"Russell King" <linux@armlinux.org.uk>,
"Catalin Marinas" <catalin.marinas@arm.com>,
"Will Deacon" <will@kernel.org>,
"Geert Uytterhoeven" <geert@linux-m68k.org>,
"Michal Simek" <monstr@monstr.eu>,
"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
"Helge Deller" <deller@gmx.de>,
"Madhavan Srinivasan" <maddy@linux.ibm.com>,
"Michael Ellerman" <mpe@ellerman.id.au>,
"Nicholas Piggin" <npiggin@gmail.com>,
"Christophe Leroy" <christophe.leroy@csgroup.eu>,
"Naveen N Rao" <naveen@kernel.org>,
"Heiko Carstens" <hca@linux.ibm.com>,
"Vasily Gorbik" <gor@linux.ibm.com>,
"Alexander Gordeev" <agordeev@linux.ibm.com>,
"Christian Borntraeger" <borntraeger@linux.ibm.com>,
"Sven Schnelle" <svens@linux.ibm.com>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Rich Felker" <dalias@libc.org>,
"John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>,
"David S. Miller" <davem@davemloft.net>,
"Andreas Larsson" <andreas@gaisler.com>,
"Andy Lutomirski" <luto@kernel.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Ingo Molnar" <mingo@redhat.com>,
"Borislav Petkov" <bp@alien8.de>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
"Chris Zankel" <chris@zankel.net>,
"Max Filippov" <jcmvbkbc@gmail.com>,
"Alexander Viro" <viro@zeniv.linux.org.uk>,
"Christian Brauner" <brauner@kernel.org>,
"Jan Kara" <jack@suse.cz>, "Mickaël Salaün" <mic@digikod.net>,
"Günther Noack" <gnoack@google.com>,
"Arnd Bergmann" <arnd@arndb.de>,
linux-alpha@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org,
linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-s390@vger.kernel.org, linux-sh@vger.kernel.org,
sparclinux@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-api@vger.kernel.org,
linux-arch@vger.kernel.org, linux-xfs@vger.kernel.org,
"Theodore Tso" <tytso@mit.edu>
Subject: Re: [PATCH v3] fs: introduce getfsxattrat and setfsxattrat syscalls
Date: Sun, 2 Mar 2025 13:20:07 +0100 [thread overview]
Message-ID: <20250302122007.4oxtugidf4vxx3vy@pali> (raw)
In-Reply-To: <ihkez5xfcuocis7cmipvts2vxnfan2ub5kcpvsrnzm37glwnax@nxp72byvetye>
On Friday 28 February 2025 09:30:38 Andrey Albershteyn wrote:
> On 2025-02-21 20:15:24, Amir Goldstein wrote:
> > On Fri, Feb 21, 2025 at 7:13 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > On Tue, Feb 11, 2025 at 06:22:47PM +0100, Andrey Albershteyn wrote:
> > > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > >
> > > > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > > > extended attributes/flags. The syscalls take parent directory fd and
> > > > path to the child together with struct fsxattr.
> > > >
> > > > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > > > that file don't need to be open as we can reference it with a path
> > > > instead of fd. By having this we can manipulated inode extended
> > > > attributes not only on regular files but also on special ones. This
> > > > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > > > we can not call ioctl() directly on the filesystem inode using fd.
> > > >
> > > > This patch adds two new syscalls which allows userspace to get/set
> > > > extended inode attributes on special files by using parent directory
> > > > and a path - *at() like syscall.
> > > >
> > > > Also, as vfs_fileattr_set() is now will be called on special files
> > > > too, let's forbid any other attributes except projid and nextents
> > > > (symlink can have an extent).
> > > >
> > > > CC: linux-api@vger.kernel.org
> > > > CC: linux-fsdevel@vger.kernel.org
> > > > CC: linux-xfs@vger.kernel.org
> > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > > > ---
> > > > 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/
> > > >
> > > > 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. Moreover, in the case
> > > > when special files are created in the directory with already
> > > > existing project quota, these inode inherit extended attributes.
> > > > This than leaves them with these attributes without the possibility
> > > > to clear them out. This, in turn, prevents userspace from
> > > > re-creating quota project on these existing files.
> > > > ---
> > > > 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
> > > > ---
> > > > 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/inode.c | 75 +++++++++++++++++++++++++++++
> > > > fs/ioctl.c | 16 +++++-
> > > > include/linux/fileattr.h | 1 +
> > > > include/linux/syscalls.h | 4 ++
> > > > include/uapi/asm-generic/unistd.h | 8 ++-
> > > > 21 files changed, 133 insertions(+), 3 deletions(-)
> > > >
> > >
> > > <cut to the syscall definitions>
> > >
> > > > diff --git a/fs/inode.c b/fs/inode.c
> > > > index 6b4c77268fc0ecace4ac78a9ca777fbffc277f4a..b2dddd9db4fabaf67a6cbf541a86978b290411ec 100644
> > > > --- a/fs/inode.c
> > > > +++ b/fs/inode.c
> > > > @@ -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.
> > >
> >
> > This makes sense to me, but I see that Andreas proposed other ways,
> > as long as we have a plan on how to extend the struct if we need more space.
> >
> > Andrey, I am sorry to bring this up in v3, but I would like to request
> > two small changes before merging this API.
> >
> > This patch by Pali [1] adds fsx_xflags_mask for the filesystem to
> > report the supported set of xflags.
> >
> > It was argued that we can make this change with the existing ioctl,
> > because it is not going to break xfs_io -c lsattr/chattr, which is fine,
> > but I think that we should merge the fsx_xflags_mask change along
> > with getfsxattrat() which is a new UAPI.
> >
> > The second request is related to setfsxattrat().
> > With current FS_IOC_FSSETXATTR, IIUC, xfs ignores unsupported
> > fsx_xflags. I think this needs to be fixed before merging setfsxattrat().
> > It's ok that a program calling FS_IOC_FSSETXATTR will not know
> > if unsupported flags will be ignored, because that's the way it is,
> > but I think that setfsxattrat() must return -EINVAL for trying to
> > set unsupported xflags.
> >
> > As I explained in [2] I think it is fine if FS_IOC_FSSETXATTR
> > will also start returning -EINVAL for unsupported flags, but I would
> > like setfsxattrat() to make that a guarantee.
> >
> > There was an open question, what does fsx_xflags_mask mean
> > for setfsxattrat() - it is a mask like in inode_set_flags() as Andreas
> > suggested? I think that would be a good idea.
> >
> > Thanks,
> > Amir.
> >
> > [1] https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@kernel.org/
> > [2] https://lore.kernel.org/linux-fsdevel/CAOQ4uxjwQJiKAqyjEmKUnq-VihyeSsxyEy2F+J38NXwrAXurFQ@mail.gmail.com/
> >
>
> I'm fine with making Pali's patchset a dependency for this syscall,
> as if vfs_fileattr_set() will start returning EINVAL on unsupported
> flags this syscall will pass it through (ioctls will need to ignore
> it). And as these syscalls use fsxattr anyway the fsx_xflags_mask
> field will be here.
>
> --
> - Andrey
>
Hello Andrey, if I understand correctly then it is needed for new
setfsxattrat() call to return EINVAL on any unsupported flags since
beginning.
Then I could extend it for new flags without breaking backward
or forward compatibility of the setfsxattrat() call.
next prev parent reply other threads:[~2025-03-02 12:20 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-11 17:22 [PATCH v3] fs: introduce getfsxattrat and setfsxattrat syscalls Andrey Albershteyn
2025-02-11 19:09 ` H. Peter Anvin
2025-02-11 19:24 ` Arnd Bergmann
2025-02-18 10:47 ` Andrey Albershteyn
2025-02-21 15:08 ` Mickaël Salaün
2025-02-22 0:33 ` Paul Moore
2025-02-24 16:00 ` Andrey Albershteyn
2025-02-25 2:37 ` Paul Moore
2025-02-21 18:11 ` Darrick J. Wong
2025-02-21 19:10 ` Andreas Dilger
2025-02-21 19:15 ` Amir Goldstein
2025-02-24 11:32 ` Christian Brauner
2025-02-24 16:21 ` Andrey Albershteyn
2025-02-25 8:02 ` Arnd Bergmann
2025-02-25 10:22 ` Christian Brauner
2025-02-25 10:40 ` Arnd Bergmann
2025-02-25 11:24 ` Christian Brauner
2025-02-25 15:59 ` Darrick J. Wong
2025-02-25 20:34 ` Pali Rohár
2025-02-28 8:30 ` Andrey Albershteyn
2025-03-02 12:20 ` Pali Rohár [this message]
2025-02-24 10:54 ` Jan Kara
2025-02-24 16:38 ` Andrey Albershteyn
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=20250302122007.4oxtugidf4vxx3vy@pali \
--to=pali@kernel.org \
--cc=James.Bottomley@hansenpartnership.com \
--cc=aalbersh@redhat.com \
--cc=agordeev@linux.ibm.com \
--cc=amir73il@gmail.com \
--cc=andreas@gaisler.com \
--cc=arnd@arndb.de \
--cc=borntraeger@linux.ibm.com \
--cc=bp@alien8.de \
--cc=brauner@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=chris@zankel.net \
--cc=christophe.leroy@csgroup.eu \
--cc=dalias@libc.org \
--cc=dave.hansen@linux.intel.com \
--cc=davem@davemloft.net \
--cc=deller@gmx.de \
--cc=djwong@kernel.org \
--cc=geert@linux-m68k.org \
--cc=glaubitz@physik.fu-berlin.de \
--cc=gnoack@google.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=hpa@zytor.com \
--cc=jack@suse.cz \
--cc=jcmvbkbc@gmail.com \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=luto@kernel.org \
--cc=maddy@linux.ibm.com \
--cc=mattst88@gmail.com \
--cc=mic@digikod.net \
--cc=mingo@redhat.com \
--cc=monstr@monstr.eu \
--cc=mpe@ellerman.id.au \
--cc=naveen@kernel.org \
--cc=npiggin@gmail.com \
--cc=richard.henderson@linaro.org \
--cc=sparclinux@vger.kernel.org \
--cc=svens@linux.ibm.com \
--cc=tglx@linutronix.de \
--cc=tsbogend@alpha.franken.de \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=ysato@users.sourceforge.jp \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).