linux-hardening - Re: [PATCH 3/4] xattr: Use dedicated slab buckets for setxattr()

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: <202403041330.06842D397@keescook>
Date: Mon, 4 Mar 2024 13:32:20 -0800
From: Kees Cook <keescook@...omium.org>
To: Dave Chinner <david@...morbit.com>
Cc: Vlastimil Babka <vbabka@...e.cz>,
	Christian Brauner <brauner@...nel.org>,
	Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>,
	linux-fsdevel@...r.kernel.org,
	"GONG, Ruiqi" <gongruiqi@...weicloud.com>,
	Xiu Jianfeng <xiujianfeng@...wei.com>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Kent Overstreet <kent.overstreet@...ux.dev>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
	David Rientjes <rientjes@...gle.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Roman Gushchin <roman.gushchin@...ux.dev>,
	Hyeonggon Yoo <42.hyeyoo@...il.com>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 3/4] xattr: Use dedicated slab buckets for setxattr()

On Tue, Mar 05, 2024 at 08:16:30AM +1100, Dave Chinner wrote:
> On Mon, Mar 04, 2024 at 10:49:31AM -0800, Kees Cook wrote:
> > The setxattr() API can be used for exploiting[1][2][3] use-after-free
> > type confusion flaws in the kernel. Avoid having a user-controlled size
> > cache share the global kmalloc allocator by using a separate set of
> > kmalloc buckets.
> > 
> > Link: https://duasynt.com/blog/linux-kernel-heap-spray [1]
> > Link: https://etenal.me/archives/1336 [2]
> > Link: https://github.com/a13xp0p0v/kernel-hack-drill/blob/master/drill_exploit_uaf.c [3]
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> > ---
> > Cc: Christian Brauner <brauner@...nel.org>
> > Cc: Alexander Viro <viro@...iv.linux.org.uk>
> > Cc: Jan Kara <jack@...e.cz>
> > Cc: linux-fsdevel@...r.kernel.org
> > ---
> >  fs/xattr.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index 09d927603433..2b06316f1d1f 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -821,6 +821,16 @@ SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
> >  	return error;
> >  }
> >  
> > +static struct kmem_buckets *xattr_buckets;
> > +static int __init init_xattr_buckets(void)
> > +{
> > +	xattr_buckets = kmem_buckets_create("xattr", 0, 0, 0,
> > +					    XATTR_LIST_MAX, NULL);
> > +
> > +	return 0;
> > +}
> > +subsys_initcall(init_xattr_buckets);
> > +
> >  /*
> >   * Extended attribute LIST operations
> >   */
> > @@ -833,7 +843,7 @@ listxattr(struct dentry *d, char __user *list, size_t size)
> >  	if (size) {
> >  		if (size > XATTR_LIST_MAX)
> >  			size = XATTR_LIST_MAX;
> > -		klist = kvmalloc(size, GFP_KERNEL);
> > +		klist = kmem_buckets_alloc(xattr_buckets, size, GFP_KERNEL);
> 
> There's a reason this uses kvmalloc() - allocations can be up to
> 64kB in size and it's not uncommon for large slab allocation to
> fail on long running machines. hence this needs to fall back to
> vmalloc() to ensure that large xattrs can always be read.

I can add a vmalloc fallback interface too. It looked like the larger
xattr usage (8k-64k) was less common, but yeah, let's not remove the
correct allocation fallback here. I'll fix this for v2.

Thanks!

-Kees

-- 
Kees Cook