Re: [PATCH] bpf: disable lockdep while running BPF on lock_release

On Mon, Aug 03, 2026 at 06:37:45PM +0800, quanyeyang via B4 Relay wrote:
> From: quanyeyang <[email protected]>
> 
> trace_lock_release() runs before __lock_release(), so the lock is
> still on the held stack when attached BPF programs execute.  If those
> programs take another lock of the same class, lockdep reports a false
> recursive locking warning.
> 
> Mark lock_release with TRACE_EVENT_FL_BPF_NO_LOCKDEP and temporarily
> disable lockdep around bpf_prog_run_array() for that event.
> 
> Fixes: 149212f07856 ("rhashtable: add lockdep tracking to bucket bit-spin-locks.")
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
> Assisted-by: Cursor:GPT-5.6 Sol
> Signed-off-by: quanyeyang <[email protected]>
> ---
> Hi,
> 
> Small RFC to align on the approach before widening scope. This is the
> alternative to the rhashtable per-init-site lock-class patch [1], taking
> the direction NeilBrown floated in that thread [2].
> 
> The problem: trace_lock_release() runs before __lock_release(), so the
> lock is still on lockdep's held stack when an attached BPF program runs.
> If that program takes another lock whose class collides with a held
> lock, lockdep reports a false "possible recursive locking".
> 
> syzbot hits this via pidfs + a BPF hash map, because all rhashtable
> bucket locks share one lock_class:
> 
>   copy_process -> alloc_pid -> pidfs_add_pid   [pidfs bucket bitlock held]
>     lock_release tracepoint
>       trace_call_bpf -> bpf_prog_run_array
>         rhtab_map_delete_elem -> rhashtable_remove_fast -> rht_lock
>           [same "rhashtable_bucket" class -> false recursion]
> 
> Why I pivoted from the per-class rhashtable fix: NeilBrown argued (a)
> sharing one lock_class across instances is common practice (d_lock,
> bd_holder_lock, kobject list_lock), and (b) BPF on lock_release() can
> perturb lockdep for *any* lock the program takes, not only rhashtable
> [2]. Disabling lockdep around the BPF handler addresses that broader
> surface, not just rhashtable.
> 
> On the concern that this hides real lock-order bugs: BPF programs are
> user-supplied, sandboxed code; their internal lock ordering is not part
> of the kernel's lock contract, and lockdep cannot validate it

BPF programs are not sandboxed. If a BPF program is able to break the
kernels locking semantics and trigger a true, non-recoverable deadlock
like this, that's a bug in the kernel.

> meaningfully -- here it only produces a false positive.
>
This doesn't seem like the correct fix. And I'd argue it's a true
positive.

What happens if the cpu gets interrupted while lockdep is disabled?

Then we become blind to any other locking issues happening in whatever
NMI context we got plopped into because we disabled lockdep here.

It seems more prudent to fix this in rhashtable.

Like what 20b6cc34ea74 ("bpf: Avoid hashtab deadlock with map_locked") did
for hashtab and the subsequent move to rqspinlock did. Basically make
the implementation tolerant to temporary recursive deadlocks like this
by detecting it and returning an error.

Which is going to be a bit more of an endevour than is done in this
patch.

Justin
> Scope of this patch (deliberately minimal):
>   - only lock_release is tagged;
>   - only the perf-event attach path (trace_call_bpf) is covered.
> 
> Open questions I'd like to align on before doing more:
>   - lock_acquire can produce a (different, ABBA-shaped) false positive
>     by the same mechanism -- tag it too?
>   - raw_tracepoint attaches go through __bpf_trace_run and are not
>     covered -- extend there too?
>   - flag vs always-off: should trace_call_bpf disable lockdep for all
>     BPF programs? The flag keeps blast radius small, but the rationale
>     applies generally.
> 
> This fixes the reported syzbot path (perf-event attach to lock_release).
> 
> [1]  https://lore.kernel.org/all/20260801-fix-rhashtable-bucket-lockdep-v1-1-15a0f8ae094c@gmail.com/
> [2]  https://lore.kernel.org/r/[email protected]
> ---
>  include/linux/trace_events.h | 3 +++
>  include/trace/events/lock.h  | 2 ++
>  kernel/trace/bpf_trace.c     | 6 ++++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 308c76b57d13..6f67b5e9e38d 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -330,6 +330,7 @@ enum {
>  	TRACE_EVENT_FL_FPROBE_BIT,
>  	TRACE_EVENT_FL_CUSTOM_BIT,
>  	TRACE_EVENT_FL_TEST_STR_BIT,
> +	TRACE_EVENT_FL_BPF_NO_LOCKDEP_BIT,
>  };
>  
>  /*
> @@ -347,6 +348,7 @@ enum {
>   *                   This is set when the custom event has not been attached
>   *                   to a tracepoint yet, then it is cleared when it is.
>   *  TEST_STR      - The event has a "%s" that points to a string outside the event
> + *  BPF_NO_LOCKDEP - Disable lockdep while running attached BPF programs
>   */
>  enum {
>  	TRACE_EVENT_FL_CAP_ANY		= (1 << TRACE_EVENT_FL_CAP_ANY_BIT),
> @@ -360,6 +362,7 @@ enum {
>  	TRACE_EVENT_FL_FPROBE		= (1 << TRACE_EVENT_FL_FPROBE_BIT),
>  	TRACE_EVENT_FL_CUSTOM		= (1 << TRACE_EVENT_FL_CUSTOM_BIT),
>  	TRACE_EVENT_FL_TEST_STR		= (1 << TRACE_EVENT_FL_TEST_STR_BIT),
> +	TRACE_EVENT_FL_BPF_NO_LOCKDEP	= (1 << TRACE_EVENT_FL_BPF_NO_LOCKDEP_BIT),
>  };
>  
>  #define TRACE_EVENT_FL_UKPROBE (TRACE_EVENT_FL_KPROBE | TRACE_EVENT_FL_UPROBE)
> diff --git a/include/trace/events/lock.h b/include/trace/events/lock.h
> index 1ded869cd619..5ccf5c54e3d2 100644
> --- a/include/trace/events/lock.h
> +++ b/include/trace/events/lock.h
> @@ -72,6 +72,8 @@ DEFINE_EVENT(lock, lock_release,
>  	TP_ARGS(lock, ip)
>  );
>  
> +TRACE_EVENT_FLAGS(lock_release, TRACE_EVENT_FL_BPF_NO_LOCKDEP);
> +
>  #ifdef CONFIG_LOCK_STAT
>  
>  DEFINE_EVENT(lock, lock_contended,
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 75495a5c3507..f2460f3c860e 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -24,6 +24,7 @@
>  #include <linux/key.h>
>  #include <linux/namei.h>
>  #include <linux/file.h>
> +#include <linux/lockdep.h>
>  
>  #include <net/bpf_sk_storage.h>
>  
> @@ -110,6 +111,7 @@ static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
>   */
>  unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
>  {
> +	bool no_lockdep = call->flags & TRACE_EVENT_FL_BPF_NO_LOCKDEP;
>  	unsigned int ret;
>  
>  	cant_sleep();
> @@ -144,8 +146,12 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
>  	 * rcu_dereference() which is accepted risk.
>  	 */
>  	rcu_read_lock();
> +	if (no_lockdep)
> +		lockdep_off();
>  	ret = bpf_prog_run_array(rcu_dereference(call->prog_array),
>  				 ctx, bpf_prog_run);
> +	if (no_lockdep)
> +		lockdep_on();
>  	rcu_read_unlock();
>  
>   out:
> 
> ---
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> change-id: 20260803-fix-lock-tracepoint-bpf-lockdep-f93e32ea6346
> 
> Best regards,
> --  
> quanyeyang <[email protected]>
> 
>