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

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

[quanyeyang via B4 Relay]

From: quanyeyang <quanyemostima@gmail.com>

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: syzbot+ef8d17bae14efb960935@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
Assisted-by: Cursor:GPT-5.6 Sol
Signed-off-by: quanyeyang <quanyemostima@gmail.com>
---
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
meaningfully -- here it only produces a false positive.

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/178572243204.3252194.4367547703856027885@noble.neil.brown.name
---
 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 <quanyemostima@gmail.com>

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

[Justin Suess]

On Mon, Aug 03, 2026 at 06:37:45PM +0800, quanyeyang via B4 Relay wrote:
> From: quanyeyang <quanyemostima@gmail.com>
> 
> 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: syzbot+ef8d17bae14efb960935@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
> Assisted-by: Cursor:GPT-5.6 Sol
> Signed-off-by: quanyeyang <quanyemostima@gmail.com>
> ---
> 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/178572243204.3252194.4367547703856027885@noble.neil.brown.name
> ---
>  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 <quanyemostima@gmail.com>
> 
> 

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

[NeilBrown]

On Tue, 04 Aug 2026, Justin Suess wrote:
> On Mon, Aug 03, 2026 at 06:37:45PM +0800, quanyeyang via B4 Relay wrote:
> > From: quanyeyang <quanyemostima@gmail.com>
> > 
> > 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: syzbot+ef8d17bae14efb960935@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
> > Assisted-by: Cursor:GPT-5.6 Sol
> > Signed-off-by: quanyeyang <quanyemostima@gmail.com>
> > ---
> > 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.

I know close enough to nothing about BPF or trace points...

Can BPF programs ever block waiting for a lock that is held across a
tracepoint?
If so, then attaching a BPF program to that trace point could
trivially cause a deadlock.
If not - then how is that ensured?

Thanks,
NeilBrown


> 
> > 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/178572243204.3252194.4367547703856027885@noble.neil.brown.name
> > ---
> >  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 <quanyemostima@gmail.com>
> > 
> > 
> 
> 

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

[Quanye Yang]

On Tue, Aug 4, 2026 at 4:34 PM NeilBrown <neilb@ownmail.net> wrote:
> I know close enough to nothing about BPF or trace points...
>
> Can BPF programs ever block waiting for a lock that is held across a
> tracepoint?
I investigate this and I think the answer could splits in two:

Mutex (sleeping locks): no, this is prevented at verification time. A
BPF program attached to a tracepoint is non-sleepable --
can_be_sleepable() returns false for BPF_PROG_TYPE_TRACEPOINT, so the
verifier rejects any sleeping helper; and at runtime it runs under
rcu_read_lock() with cant_sleep(). It cannot block on a mutex held
across the tracepoint.

Spinlock: yes, in principle. Spinlocks busy-wait rather than sleep, so
the mutex protection does not apply. BPF programs do take spinlocks --
each map lookup/update/delete takes the map's bucket lock, and
bpf_spin_lock is a spinlock -- so a program attached to a tracepoint
held across a spinlock can spin on another spinlock, and if those form a
cycle (or are the same lock) it is a real deadlock.

That is exactly the hazard BPF already hardened its own locks against:
the hashtab bucket lock and bpf_spin_lock detect recursive acquisition
and return an error instead of deadlocking (rqspinlock, succeeding the
map_locked approach in 20b6cc34ea74). The rhashtable bucket bitlock has
no such protection -- which is the gap I think.

quanyeyang
> If so, then attaching a BPF program to that trace point could
> trivially cause a deadlock.
> If not - then how is that ensured?
>
> Thanks,
> NeilBrown

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

[NeilBrown]

On Wed, 05 Aug 2026, Quanye Yang wrote:
> On Tue, Aug 4, 2026 at 4:34 PM NeilBrown <neilb@ownmail.net> wrote:
> > I know close enough to nothing about BPF or trace points...
> >
> > Can BPF programs ever block waiting for a lock that is held across a
> > tracepoint?
> I investigate this and I think the answer could splits in two:
> 
> Mutex (sleeping locks): no, this is prevented at verification time. A
> BPF program attached to a tracepoint is non-sleepable --
> can_be_sleepable() returns false for BPF_PROG_TYPE_TRACEPOINT, so the
> verifier rejects any sleeping helper; and at runtime it runs under
> rcu_read_lock() with cant_sleep(). It cannot block on a mutex held
> across the tracepoint.
> 
> Spinlock: yes, in principle. Spinlocks busy-wait rather than sleep, so
> the mutex protection does not apply. BPF programs do take spinlocks --
> each map lookup/update/delete takes the map's bucket lock, and
> bpf_spin_lock is a spinlock -- so a program attached to a tracepoint
> held across a spinlock can spin on another spinlock, and if those form a
> cycle (or are the same lock) it is a real deadlock.

Thanks - this is helpful.

> 
> That is exactly the hazard BPF already hardened its own locks against:
> the hashtab bucket lock and bpf_spin_lock detect recursive acquisition
> and return an error instead of deadlocking (rqspinlock, succeeding the
> map_locked approach in 20b6cc34ea74). The rhashtable bucket bitlock has
> no such protection -- which is the gap I think.

I don't think that detecting recursive acquisition is a scalable
solution.  You'll keep finding new locks that you need to enhance.

If I step back a bit, it looks like TP-BPF (BPF attached to tracepoints)
introduces a new execution context, similar to SOFTIRQ and HARDIRQ.
i.e.  it is something that can start running at almost any point and
blocks the currently running code until it completes.
I suspect it would be good to handle it in a similar way.

So I suggest adding a new
  LOCKDEP_STATE(TP_BPF)
to kernel/locking/lockdep_states.h, and teaching lockdep to understand
it.  Then it could help avoid all these problems.

An important part of this would be the ability to temporarily disable
TP_BPF much as we can disable interrupts.  What happens at present if
there is tracepoint that happens in BPF code, and a BPF handler is attached
to that.  Does it get called recursively?
Would there be a problem with disabling new TP_BPF handlers while TP_BPF
code is running?  Maybe this is already done?

Circling back the original patch to rhashtables that was proposed - had
you said that you needed to use rhashtables in a different context
similar to HARDIRQ or SOFTIRQ, then I think I would have had a different
response.  That is a well-specified problem with well-understood
solutions.

I would likely be OK with making is possible for lockdep to see locks
from different tables as different locks, because they need to be taken
in different lockdep contexts.

I would then see if the new lock_class_key could be declared in the
rhashtable_params struct.  This is already passed around everywhere, so
this would be much less intrusive.  We would have to use some cast to
get rid of the 'const' attribute when passing the lock_class_key to
lockdep, but I think that is justifiable.

NeilBrown


> 
> quanyeyang
> > If so, then attaching a BPF program to that trace point could
> > trivially cause a deadlock.
> > If not - then how is that ensured?
> >
> > Thanks,
> > NeilBrown
> 

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

[quanyeyang]

On Wed, Aug 5, 2026 at 6:49 AM NeilBrown <neilb@ownmail.net> wrote:
> I don't think that detecting recursive acquisition is a scalable
> solution.  You'll keep finding new locks that you need to enhance.

Yes, it makes sense.

>
> If I step back a bit, it looks like TP-BPF (BPF attached to tracepoints)
> introduces a new execution context, similar to SOFTIRQ and HARDIRQ.
> i.e.  it is something that can start running at almost any point and
> blocks the currently running code until it completes.
> I suspect it would be good to handle it in a similar way.

Thanks for the detailed explanation.

>
> So I suggest adding a new
>   LOCKDEP_STATE(TP_BPF)
> to kernel/locking/lockdep_states.h, and teaching lockdep to understand
> it.  Then it could help avoid all these problems.
>
> An important part of this would be the ability to temporarily disable
> TP_BPF much as we can disable interrupts.  What happens at present if
> there is tracepoint that happens in BPF code, and a BPF handler is attached
> to that.  Does it get called recursively?
> Would there be a problem with disabling new TP_BPF handlers while TP_BPF
> code is running?  Maybe this is already done?

It's already partly done, but it differs by attach type.

The perf-event tracepoint path -- trace_call_bpf(), and the perf-overflow
(PMI) path bpf_overflow_handler() -- share a global per-CPU counter,
bpf_prog_active: while a BPF program is running on a CPU, any further
BPF on that CPU is skipped (the prog_array is re-entered only to bump
the miss counter). So a tracepoint firing inside BPF code does not
recurse there; the handler is dropped. That is already the "disable
TP_BPF while TP_BPF runs" behaviour, analogous to disabling interrupts.

The raw_tracepoint path is different: __bpf_trace_run() uses
bpf_prog_get_recursion_context(), which is per-program -- only the *same*
program is prevented from recursing; *different* programs can nest. So a
raw_tp handler can run nested inside another raw_tp handler on the same
CPU.

So the recursion/disabling mechanism exists, but it isn't uniform: the
perf path is already "disable-like", raw_tp is not. That asymmetry is
itself part of what makes the lockdep picture messy.

>
> Circling back the original patch to rhashtables that was proposed - had
> you said that you needed to use rhashtables in a different context
> similar to HARDIRQ or SOFTIRQ, then I think I would have had a different
> response.  That is a well-specified problem with well-understood
> solutions.
>
> I would likely be OK with making is possible for lockdep to see locks
> from different tables as different locks, because they need to be taken
> in different lockdep contexts.
>
> I would then see if the new lock_class_key could be declared in the
> rhashtable_params struct.  This is already passed around everywhere, so
> this would be much less intrusive.  We would have to use some cast to
> get rid of the 'const' attribute when passing the lock_class_key to
> lockdep, but I think that is justifiable.

On the per-table lock_class_key in rhashtable_params: that sounds right
to me. I'll rework the rhashtable patch so each table can declare its own
key in params -- framed as "different tables live in different lockdep
contexts" (not per-init-site, as in my first attempt) -- and resend.

Two small notes:
 - if the field is a `struct lock_class_key *` pointer, the params struct
   can stay const: lockdep writes to the pointed-to key, not the struct,
   so no cast-away-const is needed;
 - callers that don't set it would fall back to the shared class (current
   behaviour), so it's opt-in and doesn't churn existing users.

On LOCKDEP_STATE(TP_BPF): it reads as the more general fix, but it's a
larger change in the locking core and would need the recursion gate made
uniform across attach types first (raw_tp currently runs despite the
global gate). I'd lean toward starting with the per-table-key change for
the reported false positive and treating the TP_BPF-context modelling as
the longer-term direction -- happy to take guidance from you and the
locking folks on scope.

>
> NeilBrown
Note: Sending from an alternate address because Gmail is
temporarily rejecting my mailing-list replies.
Thanks,
quanyeyang

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

[NeilBrown]

On Wed, 05 Aug 2026, quanyeyang wrote:
> On Wed, Aug 5, 2026 at 6:49 AM NeilBrown <neilb@ownmail.net> wrote:
> > I don't think that detecting recursive acquisition is a scalable
> > solution.  You'll keep finding new locks that you need to enhance.
> 
> Yes, it makes sense.
> 
> >
> > If I step back a bit, it looks like TP-BPF (BPF attached to tracepoints)
> > introduces a new execution context, similar to SOFTIRQ and HARDIRQ.
> > i.e.  it is something that can start running at almost any point and
> > blocks the currently running code until it completes.
> > I suspect it would be good to handle it in a similar way.
> 
> Thanks for the detailed explanation.
> 
> >
> > So I suggest adding a new
> >   LOCKDEP_STATE(TP_BPF)
> > to kernel/locking/lockdep_states.h, and teaching lockdep to understand
> > it.  Then it could help avoid all these problems.
> >
> > An important part of this would be the ability to temporarily disable
> > TP_BPF much as we can disable interrupts.  What happens at present if
> > there is tracepoint that happens in BPF code, and a BPF handler is attached
> > to that.  Does it get called recursively?
> > Would there be a problem with disabling new TP_BPF handlers while TP_BPF
> > code is running?  Maybe this is already done?
> 
> It's already partly done, but it differs by attach type.
> 
> The perf-event tracepoint path -- trace_call_bpf(), and the perf-overflow
> (PMI) path bpf_overflow_handler() -- share a global per-CPU counter,
> bpf_prog_active: while a BPF program is running on a CPU, any further
> BPF on that CPU is skipped (the prog_array is re-entered only to bump
> the miss counter). So a tracepoint firing inside BPF code does not
> recurse there; the handler is dropped. That is already the "disable
> TP_BPF while TP_BPF runs" behaviour, analogous to disabling interrupts.
> 
> The raw_tracepoint path is different: __bpf_trace_run() uses
> bpf_prog_get_recursion_context(), which is per-program -- only the *same*
> program is prevented from recursing; *different* programs can nest. So a
> raw_tp handler can run nested inside another raw_tp handler on the same
> CPU.
> 
> So the recursion/disabling mechanism exists, but it isn't uniform: the
> perf path is already "disable-like", raw_tp is not. That asymmetry is
> itself part of what makes the lockdep picture messy.
> 
> >
> > Circling back the original patch to rhashtables that was proposed - had
> > you said that you needed to use rhashtables in a different context
> > similar to HARDIRQ or SOFTIRQ, then I think I would have had a different
> > response.  That is a well-specified problem with well-understood
> > solutions.
> >
> > I would likely be OK with making is possible for lockdep to see locks
> > from different tables as different locks, because they need to be taken
> > in different lockdep contexts.
> >
> > I would then see if the new lock_class_key could be declared in the
> > rhashtable_params struct.  This is already passed around everywhere, so
> > this would be much less intrusive.  We would have to use some cast to
> > get rid of the 'const' attribute when passing the lock_class_key to
> > lockdep, but I think that is justifiable.
> 
> On the per-table lock_class_key in rhashtable_params: that sounds right
> to me. I'll rework the rhashtable patch so each table can declare its own
> key in params -- framed as "different tables live in different lockdep
> contexts" (not per-init-site, as in my first attempt) -- and resend.
> 
> Two small notes:
>  - if the field is a `struct lock_class_key *` pointer, the params struct
>    can stay const: lockdep writes to the pointed-to key, not the struct,
>    so no cast-away-const is needed;

I hoped that the keys would be embedded in the params.  That would work
nicely if all users simply used a global params structure.  But a few
copy the structures around (I cannot imaging why, but maybe there is a
good reason).  Copying lockdep keys doesn't work.

(And some use extern references....  I wonder home much performance they
lose because the hash/cmp functions cannot be inlined....)

One thing I didn't like about the original patch is that it seemed to add
a lot of noise to the code, passing keys around in multiple places.
Maybe the cleanest approach would be to embed the keys in the "struct
rhashtable" so each table gets its own key.
Then use lockdep_register_key() in rhashtable_init and use it as needed.

You could even have just one key and use the different subclasses for
the different locks.
 0 for rhashtable.mutex
 1 for rhashtable.lock
 2 for the bit locks

but maybe that is needless complexity.

All of the rest of your email is interesting and useful - thanks.

NeilBrown


>  - callers that don't set it would fall back to the shared class (current
>    behaviour), so it's opt-in and doesn't churn existing users.
> 
> On LOCKDEP_STATE(TP_BPF): it reads as the more general fix, but it's a
> larger change in the locking core and would need the recursion gate made
> uniform across attach types first (raw_tp currently runs despite the
> global gate). I'd lean toward starting with the per-table-key change for
> the reported false positive and treating the TP_BPF-context modelling as
> the longer-term direction -- happy to take guidance from you and the
> locking folks on scope.
> 
> >
> > NeilBrown
> Note: Sending from an alternate address because Gmail is
> temporarily rejecting my mailing-list replies.
> Thanks,
> quanyeyang
> 

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

[NeilBrown]

On Wed, 05 Aug 2026, NeilBrown wrote:
> 
> One thing I didn't like about the original patch is that it seemed to add
> a lot of noise to the code, passing keys around in multiple places.
> Maybe the cleanest approach would be to embed the keys in the "struct
> rhashtable" so each table gets its own key.
> Then use lockdep_register_key() in rhashtable_init and use it as needed.
> 
> You could even have just one key and use the different subclasses for
> the different locks.
>  0 for rhashtable.mutex
>  1 for rhashtable.lock
>  2 for the bit locks
> 
> but maybe that is needless complexity.
> 

So I thought about this some more and came up with this approach which
might be a good compromise.  I builds but I haven't tested it.
What do you thing?

NeilBrown

diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-types.h
index 57c11ec9dc64..2029fba216a8 100644
--- a/include/linux/rhashtable-types.h
+++ b/include/linux/rhashtable-types.h
@@ -97,6 +97,9 @@ struct rhashtable {
 #ifdef CONFIG_MEM_ALLOC_PROFILING
 	struct alloc_tag		*alloc_tag;
 #endif
+#ifdef CONFIG_LOCKDEP
+o	struct lock_class_key		*lockdep_key;
+#endif
 };
 
 /**
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 79f83b6eec27..f8358d43691b 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -320,18 +320,6 @@ static inline struct rhash_lock_head __rcu **rht_bucket_insert(
  * When we write to a bucket without unlocking, we use rht_assign_locked().
  */
 
-static inline unsigned long rht_lock(struct bucket_table *tbl,
-				     struct rhash_lock_head __rcu **bkt)
-	__acquires(__bitlock(0, bkt))
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	bit_spin_lock(0, (unsigned long *)bkt);
-	lock_map_acquire(&tbl->dep_map);
-	return flags;
-}
-
 static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
 					struct rhash_lock_head __rcu **bucket,
 					unsigned int subclass)
@@ -341,10 +329,18 @@ static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
 
 	local_irq_save(flags);
 	bit_spin_lock(0, (unsigned long *)bucket);
-	lock_acquire_exclusive(&tbl->dep_map, subclass, 0, NULL, _THIS_IP_);
+	/* subclass 0 is used for ->lock and 1 for ->mutex. 2+ for bitlocks */
+	lock_acquire_exclusive(&tbl->dep_map, subclass+2, 0, NULL, _THIS_IP_);
 	return flags;
 }
 
+static inline unsigned long rht_lock(struct bucket_table *tbl,
+				     struct rhash_lock_head __rcu **bkt)
+	__acquires(__bitlock(0, bkt))
+{
+	return rht_lock_nested(tbl, bkt, 0);
+}
+
 static inline void rht_unlock(struct bucket_table *tbl,
 			      struct rhash_lock_head __rcu **bkt,
 			      unsigned long flags)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index d459bef245f4..17340433d983 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -189,7 +189,6 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
 	struct bucket_table *tbl = NULL;
 	size_t size;
 	int i;
-	static struct lock_class_key __key;
 
 	tbl = alloc_hooks_tag(ht->alloc_tag,
 			kvmalloc_node_align_noprof(struct_size(tbl, buckets, nbuckets),
@@ -205,7 +204,10 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
 	if (tbl == NULL)
 		return NULL;
 
-	lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", &__key, 0);
+#ifdef CONFIG_LOCKDEP
+	/* bitlocks must use nesting level 2 or more */
+	lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", ht->lockdep_key, 0);
+#endif
 
 	tbl->size = size;
 
@@ -428,7 +430,7 @@ static void rht_deferred_worker(struct work_struct *work)
 	int err = 0;
 
 	ht = container_of(work, struct rhashtable, run_work);
-	mutex_lock(&ht->mutex);
+	mutex_lock_nested(&ht->mutex, 1);
 
 	tbl = rht_dereference(ht->tbl, ht);
 	tbl = rhashtable_last_table(ht, tbl);
@@ -1172,8 +1174,14 @@ int __rhashtable_init_noprof(struct rhashtable *ht,
 		return -EINVAL;
 
 	memset(ht, 0, sizeof(*ht));
+	/* mutex_lock must use nesting level 1 */
 	mutex_init_with_key(&ht->mutex, key);
 	spin_lock_init(&ht->lock);
+	/* spin_lock can use nesting level 0 */
+	lockdep_set_class(&ht->lock, key);
+#ifdef CONFIG_LOCKDEP
+	ht->lockdep_key = key;
+#endif
 	memcpy(&ht->p, params, sizeof(*params));
 
 	alloc_tag_record(ht->alloc_tag);

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

[quanyeyang]

On Thursday, August 6th, 2026 at AM 6:44, NeilBrown <neilb@ownmail.net> wrote:

> So I thought about this some more and came up with this approach which
> might be a good compromise.  I builds but I haven't tested it.
> What do you thing?
> 
> NeilBrown

Tested it now (PROVE_LOCKING + the syzbot reproducer at the Closes:
link). It fixes the false positive cleanly:

  before: "WARNING: possible recursive locking" on rhashtable_bucket,
          5 instances during the reproducer's run;
  after:  no warning; new lockdep complaints.

The subclass layout reads right, and rhashtable.c has the single
mutex_lock(&ht->mutex) (rht_deferred_worker, which you moved to
_nested(1)), so nothing else is left at the old subclass.

One nit: the new field line in your mail shows a stray leading "o"

  o      struct lock_class_key           *lockdep_key;

probably a paste artifact; the actual field is fine.

Thanks for working this through -- reusing the existing per-init-site
key with subclasses is much cleaner than my params suggestion.

quanyeyang