From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B46E267F6E; Tue, 25 Feb 2025 10:22:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740478953; cv=none; b=gU0X2jbBotf4O+UZXRoLT3r+wW2ChV/PIDX5L1bsgNUqHh9pS0FBb2fdQm1Ut+IcH/g9KdZ1Ojpp5M27TJEDAqeefjKjUVx8OILKnhCfNQmpsNte42effAhP+ajLNQ9CHSlpGmYHbSM74GDaE85ZDCeGSCc7DHVk5XgRDgvj2qI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740478953; c=relaxed/simple; bh=5HOkE2YqQG0nj7GKm31VM9cru6617nBuKvCY1GMnDrc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=EUneP6Ybxe55qtdPa+mxyT/qeIJfev0wzLKS9rqATdj9W+JG282UM7GCxIofyR1HmFgu3qpQSpjQRN48RCvnpdqe6D6knpmjC7M/BXFgB+B1N22pu7iZr1sVfbAvhhNNvV5qnuAWUrAw7MZti12Vuzt0946DiVhtyPiKb99hkYc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=QLdVe9QX; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="QLdVe9QX" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5F22EC4CEDD; Tue, 25 Feb 2025 10:22:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1740478952; bh=5HOkE2YqQG0nj7GKm31VM9cru6617nBuKvCY1GMnDrc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QLdVe9QX36ZATBZopGaFS7xncV75FrVifGUmO8YFcq0fsqRsaJ22u5r4/RZQOJ3Sp yMEl/oKfTZihFmWVtEKikhl/HfJsgdlkCrNq55W9eyV/njTytIaMD9k7zFgv1x4TaI I2IADYz/CQiWBy3kUYdqd4/6a+d9wa1OJ63tEswPaPSmhJ9mvyvjSLPOfrMnC+lJSK LaosnjwljP1fU1QllOJcPmhDfG/AbY9BKKl5qhnGon0GvGM+IMN1WKFPOeumOwG/Tb 8wfL7+oJ4DopnrX8olKXaLI8joRoQgVk7Ykgfi+0FSJvIaTDPVShiCTVbRLd3HNrPp 6XmUqt0gTmi9g== Date: Tue, 25 Feb 2025 11:22:17 +0100 From: Christian Brauner To: Arnd Bergmann Cc: Amir Goldstein , Andrey Albershteyn , "Darrick J. Wong" , Richard Henderson , Matt Turner , Russell King , Catalin Marinas , Will Deacon , Geert Uytterhoeven , Michal Simek , Thomas Bogendoerfer , "James E . J . Bottomley" , Helge Deller , Madhavan Srinivasan , Michael Ellerman , Nicholas Piggin , Christophe Leroy , Naveen N Rao , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Yoshinori Sato , Rich Felker , John Paul Adrian Glaubitz , "David S . Miller" , Andreas Larsson , Andy Lutomirski , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Chris Zankel , Max Filippov , Alexander Viro , Jan Kara , =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= , =?utf-8?Q?G=C3=BCnther?= Noack , 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 , linux-xfs@vger.kernel.org, Pali =?utf-8?B?Um9ow6Fy?= , Theodore Ts'o Subject: Re: [PATCH v3] fs: introduce getfsxattrat and setfsxattrat syscalls Message-ID: <20250225-strom-kopflos-32062347cd13@brauner> References: <20250211-xattrat-syscall-v3-1-a07d15f898b2@kernel.org> <20250221181135.GW21808@frogsfrogsfrogs> <20250224-klinke-hochdekoriert-3f6be89005a8@brauner> <6b51ffa2-9d67-4466-865e-e703c1243352@app.fastmail.com> Precedence: bulk X-Mailing-List: linux-api@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6b51ffa2-9d67-4466-865e-e703c1243352@app.fastmail.com> 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 wrote: > > >> > > @@ -23,6 +23,9 @@ > >> > > #include > >> > > #include > >> > > #include > >> > > +#include > >> > > +#include > >> > > +#include > >> > > #include > >> > > #define CREATE_TRACE_POINTS > >> > > #include > >> > > @@ -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.