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 <[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.
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-15a0f8ae0...@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]>
> >
> >
>
>
