linux-kernel - Re: [PATCH v4 3/3] 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: <rbzlwvecvrp4xawwp5nywdq6wp5hgjhrtrabpszv74xmfqbj4f@x7v6eqfc5gcd>
Date: Wed, 23 Apr 2025 11:53:25 +0200
From: Jan Kara <jack@...e.cz>
To: Christian Brauner <brauner@...nel.org>
Cc: Andrey Albershteyn <aalbersh@...hat.com>, 
	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>, 
	Arnd Bergmann <arnd@...db.de>, Pali Rohár <pali@...nel.org>, 
	Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.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@...r.kernel.org, 
	linux-xfs@...r.kernel.org
Subject: Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat
 syscalls

On Tue 22-04-25 16:59:02, Christian Brauner wrote:
> On Fri, Mar 21, 2025 at 08:48:42PM +0100, Andrey Albershteyn wrote:
> > From: Andrey Albershteyn <aalbersh@...hat.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.
> > 
> > CC: linux-api@...r.kernel.org
> > CC: linux-fsdevel@...r.kernel.org
> > CC: linux-xfs@...r.kernel.org
> > Signed-off-by: Andrey Albershteyn <aalbersh@...hat.com>
> > Acked-by: Arnd Bergmann <arnd@...db.de>
...
> > +		struct fsxattr __user *, ufsx, size_t, usize,
> > +		unsigned int, at_flags)
> > +{
> > +	struct fileattr fa = {};
> > +	struct path filepath;
> > +	int error;
> > +	unsigned int lookup_flags = 0;
> > +	struct filename *name;
> > +	struct fsxattr fsx = {};
> > +
> > +	BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > +	BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > +
> > +	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > +		return -EINVAL;
> > +
> > +	if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > +		lookup_flags |= LOOKUP_FOLLOW;
> > +
> > +	if (at_flags & AT_EMPTY_PATH)
> > +		lookup_flags |= LOOKUP_EMPTY;
> > +
> > +	if (usize > PAGE_SIZE)
> > +		return -E2BIG;
> > +
> > +	if (usize < FSXATTR_SIZE_VER0)
> > +		return -EINVAL;
> > +
> > +	name = getname_maybe_null(filename, at_flags);
> > +	if (!name) {
> 
> This is broken as it doesn't handle AT_FDCWD correctly. You need:
> 
>         name = getname_maybe_null(filename, at_flags);
>         if (IS_ERR(name))
>                 return PTR_ERR(name);
> 
>         if (!name && dfd >= 0) {
> 		CLASS(fd, f)(dfd);

Ah, you're indeed right that if dfd == AT_FDCWD and filename == NULL, the
we should operate on cwd but we'd bail with error here. I've missed that
during my review. But as far as I've checked the same bug is there in
path_setxattrat() and path_getxattrat() so we should fix this there as
well?

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR