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: <6b51ffa2-9d67-4466-865e-e703c1243352@app.fastmail.com>
Date: Tue, 25 Feb 2025 09:02:04 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Christian Brauner" <brauner@...nel.org>
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 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.

    Arnd