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

NeilBrown <[email protected]> Tue, 04 Aug 2026 18:34:26 +1000
Newsgroups org.kernel.vger.linux-trace-kernel,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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]>
> >=20
> > 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.
> >=20
> > Mark lock_release with TRACE_EVENT_FL_BPF_NO_LOCKDEP and temporarily
> > disable lockdep around bpf_prog_run_array() for that event.
> >=20
> > Fixes: 149212f07856 ("rhashtable: add lockdep tracking to bucket bit-spin=
-locks.")
> > Reported-by: [email protected]
> > Closes: https://syzkaller.appspot.com/bug?extid=3Def8d17bae14efb960935
> > Assisted-by: Cursor:GPT-5.6 Sol
> > Signed-off-by: quanyeyang <[email protected]>
> > ---
> > Hi,
> >=20
> > 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].
> >=20
> > 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".
> >=20
> > syzbot hits this via pidfs + a BPF hash map, because all rhashtable
> > bucket locks share one lock_class:
> >=20
> >   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]
> >=20
> > 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.
> >=20
> > 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
>=20
> 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


>=20
> > 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.
>=20
> What happens if the cpu gets interrupted while lockdep is disabled?
>=20
> Then we become blind to any other locking issues happening in whatever
> NMI context we got plopped into because we disabled lockdep here.
>=20
> It seems more prudent to fix this in rhashtable.
>=20
> 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.
>=20
> Which is going to be a bit more of an endevour than is done in this
> patch.
>=20
> Justin
> > Scope of this patch (deliberately minimal):
> >   - only lock_release is tagged;
> >   - only the perf-event attach path (trace_call_bpf) is covered.
> >=20
> > 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.
> >=20
> > This fixes the reported syzbot path (perf-event attach to lock_release).
> >=20
> > [1]  https://lore.kernel.org/all/20260801-fix-rhashtable-bucket-lockdep-v=
[email protected]/
> > [2]  https://lore.kernel.org/r/178572243204.3252194.4367547703856027885@n=
oble.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(+)
> >=20
> > 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,
> >  };
> > =20
> >  /*
> > @@ -347,6 +348,7 @@ enum {
> >   *                   This is set when the custom event has not been atta=
ched
> >   *                   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		=3D (1 << TRACE_EVENT_FL_CAP_ANY_BIT),
> > @@ -360,6 +362,7 @@ enum {
> >  	TRACE_EVENT_FL_FPROBE		=3D (1 << TRACE_EVENT_FL_FPROBE_BIT),
> >  	TRACE_EVENT_FL_CUSTOM		=3D (1 << TRACE_EVENT_FL_CUSTOM_BIT),
> >  	TRACE_EVENT_FL_TEST_STR		=3D (1 << TRACE_EVENT_FL_TEST_STR_BIT),
> > +	TRACE_EVENT_FL_BPF_NO_LOCKDEP	=3D (1 << TRACE_EVENT_FL_BPF_NO_LOCKDEP_B=
IT),
> >  };
> > =20
> >  #define TRACE_EVENT_FL_UKPROBE (TRACE_EVENT_FL_KPROBE | TRACE_EVENT_FL_U=
PROBE)
> > 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)
> >  );
> > =20
> > +TRACE_EVENT_FLAGS(lock_release, TRACE_EVENT_FL_BPF_NO_LOCKDEP);
> > +
> >  #ifdef CONFIG_LOCK_STAT
> > =20
> >  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>
> > =20
> >  #include <net/bpf_sk_storage.h>
> > =20
> > @@ -110,6 +111,7 @@ static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_c=
tx *ctx);
> >   */
> >  unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
> >  {
> > +	bool no_lockdep =3D call->flags & TRACE_EVENT_FL_BPF_NO_LOCKDEP;
> >  	unsigned int ret;
> > =20
> >  	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 =3D bpf_prog_run_array(rcu_dereference(call->prog_array),
> >  				 ctx, bpf_prog_run);
> > +	if (no_lockdep)
> > +		lockdep_on();
> >  	rcu_read_unlock();
> > =20
> >   out:
> >=20
> > ---
> > base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> > change-id: 20260803-fix-lock-tracepoint-bpf-lockdep-f93e32ea6346
> >=20
> > Best regards,
> > -- =20
> > quanyeyang <[email protected]>
> >=20
> >=20
>=20
>=20