linux-kernel - Re: [PATCH] kernfs: convert kernfs_idr

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: <ZY6N7pX4voA6OPbh@mtj.duckdns.org>
Date: Fri, 29 Dec 2023 18:14:22 +0900
From: Tejun Heo <tj@...nel.org>
To: Andrea Righi <andrea.righi@...onical.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Alexei Starovoitov <ast@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kernfs: convert kernfs_idr_lock to an irq safe raw
 spinlock

On Fri, Dec 29, 2023 at 08:49:16AM +0100, Andrea Righi wrote:
> bpf_cgroup_from_id() is basically a wrapper to cgroup_get_from_id(),
> that is relying on kernfs to determine the right cgroup associated to
> the target id.
> 
> As a kfunc, it has the potential to be attached to any function through
> BPF, particularly in contexts where certain locks are held.
> 
> However, kernfs is not using an irq safe spinlock for kernfs_idr_lock,
> that means any kernfs function that is acquiring this lock can be
> interrupted and potentially hit bpf_cgroup_from_id() in the process,
> triggering a deadlock.
> 
> For example, it is really easy to trigger a lockdep splat between
> kernfs_idr_lock and rq->_lock, attaching a small BPF program to
> __set_cpus_allowed_ptr_locked() that just calls bpf_cgroup_from_id():
...
> Fixes: 332ea1f697be ("bpf: Add bpf_cgroup_from_id() kfunc")
> Signed-off-by: Andrea Righi <andrea.righi@...onical.com>

Acked-by: Tejun Heo <tj@...nel.org>

As an immediate fix, this looks great. In the longer term, it probably would
make sense to make that idr RCU protected so that lookup path doesn't have
to worry about locking order.

Thanks.

-- 
tejun