linux-kernel - Re: [PATCH v3] fs: introduce getfsxattrat and setfsxattrat syscalls

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: <20250225-strom-kopflos-32062347cd13@brauner>
Date: Tue, 25 Feb 2025 11:22:17 +0100
From: Christian Brauner <brauner@...nel.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Amir Goldstein <amir73il@...il.com>, 
	Andrey Albershteyn <aalbersh@...hat.com>, "Darrick J. Wong" <djwong@...nel.org>, 
	Richard Henderson <richard.henderson@...aro.org>, Matt Turner <mattst88@...il.com>, 
	Russell King <linux@...linux.org.uk>, Catalin Marinas <catalin.marinas@....com>, 
	Will Deacon <will@...nel.org>, Geert Uytterhoeven <geert@...ux-m68k.org>, 
	Michal Simek <monstr@...str.eu>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>, 
	"James E . J . Bottomley" <James.Bottomley@...senpartnership.com>, Helge Deller <deller@....de>, 
	Madhavan Srinivasan <maddy@...ux.ibm.com>, Michael Ellerman <mpe@...erman.id.au>, 
	Nicholas Piggin <npiggin@...il.com>, Christophe Leroy <christophe.leroy@...roup.eu>, 
	Naveen N Rao <naveen@...nel.org>, Heiko Carstens <hca@...ux.ibm.com>, 
	Vasily Gorbik <gor@...ux.ibm.com>, Alexander Gordeev <agordeev@...ux.ibm.com>, 
	Christian Borntraeger <borntraeger@...ux.ibm.com>, Sven Schnelle <svens@...ux.ibm.com>, 
	Yoshinori Sato <ysato@...rs.sourceforge.jp>, Rich Felker <dalias@...c.org>, 
	John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>, "David S . Miller" <davem@...emloft.net>, 
	Andreas Larsson <andreas@...sler.com>, Andy Lutomirski <luto@...nel.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, 
	Chris Zankel <chris@...kel.net>, Max Filippov <jcmvbkbc@...il.com>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, 
	Mickaël Salaün <mic@...ikod.net>, Günther Noack <gnoack@...gle.com>, 
	linux-alpha@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org, 
	linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org, 
	linux-sh@...r.kernel.org, sparclinux@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-security-module@...r.kernel.org, linux-api@...r.kernel.org, 
	Linux-Arch <linux-arch@...r.kernel.org>, linux-xfs@...r.kernel.org, 
	Pali Rohár <pali@...nel.org>, Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH v3] fs: introduce getfsxattrat and setfsxattrat syscalls

On Tue, Feb 25, 2025 at 09:02:04AM +0100, Arnd Bergmann wrote:
> On Mon, Feb 24, 2025, at 12:32, Christian Brauner wrote:
> > On Fri, Feb 21, 2025 at 08:15:24PM +0100, Amir Goldstein wrote:
> >> On Fri, Feb 21, 2025 at 7:13 PM Darrick J. Wong <djwong@...nel.org> wrote:
> 
> >> > > @@ -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.