Received-SPF: pass (google.com: domain of linux-kernel+bounces-13020-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Sender: Tejun Heo <htejun@gmail.com>
Date: Fri, 29 Dec 2023 18:14:22 +0900
From: Tejun Heo <tj@kernel.org>
To: Andrea Righi <andrea.righi@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alexei Starovoitov <ast@kernel.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kernfs: convert kernfs_idr_lock to an irq safe raw
 spinlock
Message-ID: <ZY6N7pX4voA6OPbh@mtj.duckdns.org>
References: <20231229074916.53547-1-andrea.righi@canonical.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231229074916.53547-1-andrea.righi@canonical.com>

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@canonical.com>

Acked-by: Tejun Heo <tj@kernel.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