linux-kernel - Re: [PATCH bpf-next 1/4] kernfs: Add __kernfs_xattr

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: <20250619-kaulquappen-absagen-27377e154bc0@brauner>
Date: Thu, 19 Jun 2025 12:01:19 +0200
From: Christian Brauner <brauner@...nel.org>
To: Song Liu <song@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Tejun Heo <tj@...nel.org>
Cc: bpf@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, kernel-team@...a.com, 
	andrii@...nel.org, eddyz87@...il.com, ast@...nel.org, daniel@...earbox.net, 
	martin.lau@...ux.dev, viro@...iv.linux.org.uk, jack@...e.cz, kpsingh@...nel.org, 
	mattbobrowski@...gle.com, amir73il@...il.com, gregkh@...uxfoundation.org, tj@...nel.org, 
	daan.j.demeyer@...il.com
Subject: Re: [PATCH bpf-next 1/4] kernfs: Add __kernfs_xattr_get for RCU
 protected access

On Wed, Jun 18, 2025 at 04:37:36PM -0700, Song Liu wrote:
> Existing kernfs_xattr_get() locks iattr_mutex, so it cannot be used in
> RCU critical sections. Introduce __kernfs_xattr_get(), which reads xattr
> under RCU read lock. This can be used by BPF programs to access cgroupfs
> xattrs.
> 
> Signed-off-by: Song Liu <song@...nel.org>
> ---
>  fs/kernfs/inode.c      | 14 ++++++++++++++
>  include/linux/kernfs.h |  2 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index b83054da68b3..0ca231d2012c 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -302,6 +302,20 @@ int kernfs_xattr_get(struct kernfs_node *kn, const char *name,
>  	return simple_xattr_get(&attrs->xattrs, name, value, size);
>  }
>  
> +int __kernfs_xattr_get(struct kernfs_node *kn, const char *name,
> +		       void *value, size_t size)
> +{
> +	struct kernfs_iattrs *attrs;
> +
> +	WARN_ON_ONCE(!rcu_read_lock_held());
> +
> +	attrs = rcu_dereference(kn->iattr);
> +	if (!attrs)
> +		return -ENODATA;

Hm, that looks a bit silly. Which isn't your fault. I'm looking at the
kernfs code that does the xattr allocations and I think that's the
origin of the silliness. It uses a single global mutex for all kernfs
users thus serializing all allocations for kernfs->iattr. That seems
crazy but maybe I'm missing a good reason.

I'm appending a patch to remove that mutex. @Greg, @Tejun, can you take
a look whether that makes sense to you. Then I can take that patch and
you can build yours on top of the series and I'll pick it all up in one
go.

You should then just use READ_ONCE(kn->iattr) or the
kernfs_iattrs_noalloc(kn) helper in your kfunc.

View attachment "0001-kernfs-remove-iattr_mutex.patch" of type "text/x-diff" (5029 bytes)