Re: [PATCH] rhashtable: use per-init-site lockdep classes for bucket locks

On Sat, Aug 01, 2026 at 07:12:00PM +0000, NeilBrown wrote:
> This seems like a band-aid rather than a proper fix.
>
> Surely attaching a BPF program to lock_release() has potential for
> causing all sorts of lockdep related problems.  Any lock that the BPF
> program takes can trigger something, and I find it unlikely that
> rhashtable is the only part of BPF code that takes a lock.
>
> Maybe the tracepoint needs to disable lockdep while the BPF handler is
> running, or something like that.
>
> But I would need a much stronger argument before I could be happy with
> this patch.

Thanks for the review.

I agree that attaching BPF to lock_release() is a sharp edge: the
tracepoint runs before __lock_release(), so the lock is still on the
lockdep held stack, and any lock taken by the BPF program becomes a
dependency edge.  Whether that instrumentation path should be isolated
from lockdep is a broader question, and I don't claim this patch solves
it.

But the syzbot warning at hand is not BPF-specific.  It is a false
positive from modeling every rhashtable bucket lock with one global
lock_class_key.  Different rhashtable instances have different physical
bitlocks; nesting them is not recursive locking of the same lock.

I can reproduce the same "possible recursive locking detected" without
any BPF / lock tracepoint:

  1. init two tables at distinct rhashtable_init() call sites (ht_a, ht_b)
  2. prefill so the insert path walks a non-empty bucket chain
  3. in ht_a's obj_cmpfn (called under ht_a's bucket bitlock), call
     rhashtable_lookup_insert_key() on ht_b

That is enough to hit the warning on an unpatched kernel.  So disabling
lockdep around the BPF handler would only silence one trigger for this
class-modeling bug; it would not fix the underlying incorrect lock class
sharing, and it would also hide real lock-order problems on that path
(including the callback-under-bucket-lock cases that 149212f07856 wanted
lockdep to see).

The per-init-site bucket key follows the same approach as 060d4e94b8d4
did for ht->mutex: keep one class per init site (and across resize of
that table via SINGLE_DEPTH_NESTING), rather than one class for the
entire kernel.

Happy to share the small local reproducer if that helps.

On Sat, Aug 1, 2026 at 7:12=E2=80=AFPM NeilBrown <[email protected]> wrote:
>
> On Sat, 01 Aug 2026, [email protected] wrote:
> > From: quanyeyang <[email protected]>
> >
> > All bucket tables currently share a single lockdep class. This makes
> > lockdep conflate bucket locks from unrelated rhashtable instances.
> >
> > A BPF program attached to lock_release can expose this when pidfs
> > inserts a pid. The tracepoint runs before lockdep removes the pidfs
> > bucket lock from the task's held-lock stack. Deleting an element from
> > a BPF RHASH map then acquires a bucket lock belonging to a different
> > rhashtable. Since both tables use the same class, lockdep reports
> > possible recursive locking.
>
> This seems like a band-aid rather than a proper fix.
>
> Surely attaching a BPF program to lock_release() has potential for
> causing all sorts of lockdep related problems.  Any lock that the BPF
> program takes can trigger something, and I find it unlikely that
> rhashtable is the only part of BPF code that takes a lock.
>
> Maybe the tracepoint needs to disable lockdep while the BPF handler is
> running, or something like that.
>
> But I would need a much stronger argument before I could be happy with
> this patch.
>
> NeilBrown
>
>
> >
> > Declare a separate bucket lock class key at each rhashtable_init() and
> > rhltable_init() call site, alongside the mutex class key. Store the
> > bucket key in struct rhashtable so tables created during resize keep
> > using the same class.
> >
> > A targeted reproducer triggers the warning reliably before this change.
> > After the change, the nested BPF RHASH deletion still executes, but
> > lockdep no longer reports recursive locking.
> >
> > Fixes: 149212f07856 ("rhashtable: add lockdep tracking to bucket bit-sp=
in-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]>
> > ---
> >  include/linux/rhashtable-types.h | 20 ++++++++++++++------
> >  lib/rhashtable.c                 | 19 +++++++++++++------
> >  2 files changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtabl=
e-types.h
> > index 57c11ec9dc64..4dea91a49ec8 100644
> > --- a/include/linux/rhashtable-types.h
> > +++ b/include/linux/rhashtable-types.h
> > @@ -82,6 +82,7 @@ struct rhashtable_params {
> >   * @mutex: Mutex to protect current/future table swapping
> >   * @lock: Spin lock to protect walker list
> >   * @nelems: Number of elements in table
> > + * @bucket_lock_key: Per-init-site lockdep class for bucket bit-locks
> >   */
> >  struct rhashtable {
> >       struct bucket_table __rcu       *tbl;
> > @@ -94,6 +95,7 @@ struct rhashtable {
> >       struct mutex                    mutex;
> >       spinlock_t                      lock;
> >       atomic_t                        nelems;
> > +     struct lock_class_key           *bucket_lock_key;
> >  #ifdef CONFIG_MEM_ALLOC_PROFILING
> >       struct alloc_tag                *alloc_tag;
> >  #endif
> > @@ -138,23 +140,29 @@ struct rhashtable_iter {
> >
> >  int __rhashtable_init_noprof(struct rhashtable *ht,
> >                   const struct rhashtable_params *params,
> > -                 struct lock_class_key *key);
> > +                 struct lock_class_key *mutex_key,
> > +                 struct lock_class_key *bucket_key);
> >  #define rhashtable_init_noprof(ht, params)                           \
> >  ({                                                                   \
> > -     static struct lock_class_key __key;                             \
> > +     static struct lock_class_key __mutex_key;                       \
> > +     static struct lock_class_key __bucket_key;                      \
> >                                                                       \
> > -     __rhashtable_init_noprof(ht, params, &__key);                   \
> > +     __rhashtable_init_noprof(ht, params, &__mutex_key,              \
> > +                              &__bucket_key);                        \
> >  })
> >  #define rhashtable_init(...) alloc_hooks(rhashtable_init_noprof(__VA_A=
RGS__))
> >
> >  int __rhltable_init_noprof(struct rhltable *hlt,
> >                 const struct rhashtable_params *params,
> > -               struct lock_class_key *key);
> > +               struct lock_class_key *mutex_key,
> > +               struct lock_class_key *bucket_key);
> >  #define rhltable_init_noprof(hlt, params)                            \
> >  ({                                                                   \
> > -     static struct lock_class_key __key;                             \
> > +     static struct lock_class_key __mutex_key;                       \
> > +     static struct lock_class_key __bucket_key;                      \
> >                                                                       \
> > -     __rhltable_init_noprof(hlt, params, &__key);                    \
> > +     __rhltable_init_noprof(hlt, params, &__mutex_key,               \
> > +                            &__bucket_key);                          \
> >  })
> >  #define rhltable_init(...)   alloc_hooks(rhltable_init_noprof(__VA_ARG=
S__))
> >
> > diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> > index d459bef245f4..e047ad912f0e 100644
> > --- a/lib/rhashtable.c
> > +++ b/lib/rhashtable.c
> > @@ -189,7 +189,6 @@ static struct bucket_table *bucket_table_alloc(stru=
ct rhashtable *ht,
> >       struct bucket_table *tbl =3D NULL;
> >       size_t size;
> >       int i;
> > -     static struct lock_class_key __key;
> >
> >       tbl =3D alloc_hooks_tag(ht->alloc_tag,
> >                       kvmalloc_node_align_noprof(struct_size(tbl, bucke=
ts, nbuckets),
> > @@ -205,7 +204,12 @@ static struct bucket_table *bucket_table_alloc(str=
uct rhashtable *ht,
> >       if (tbl =3D=3D NULL)
> >               return NULL;
> >
> > -     lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", &__key, 0);
> > +     /*
> > +      * Keep all bucket tables belonging to the same rhashtable in the
> > +      * per-init-site lock class, including tables created during resi=
ze.
> > +      */
> > +     lockdep_init_map(&tbl->dep_map, "rhashtable_bucket",
> > +                      ht->bucket_lock_key, 0);
> >
> >       tbl->size =3D size;
> >
> > @@ -1162,7 +1166,8 @@ static u32 rhashtable_jhash2(const void *key, u32=
 length, u32 seed)
> >   */
> >  int __rhashtable_init_noprof(struct rhashtable *ht,
> >                   const struct rhashtable_params *params,
> > -                 struct lock_class_key *key)
> > +                 struct lock_class_key *mutex_key,
> > +                 struct lock_class_key *bucket_key)
> >  {
> >       struct bucket_table *tbl;
> >       size_t size;
> > @@ -1172,7 +1177,8 @@ int __rhashtable_init_noprof(struct rhashtable *h=
t,
> >               return -EINVAL;
> >
> >       memset(ht, 0, sizeof(*ht));
> > -     mutex_init_with_key(&ht->mutex, key);
> > +     mutex_init_with_key(&ht->mutex, mutex_key);
> > +     ht->bucket_lock_key =3D bucket_key;
> >       spin_lock_init(&ht->lock);
> >       memcpy(&ht->p, params, sizeof(*params));
> >
> > @@ -1237,11 +1243,12 @@ EXPORT_SYMBOL_GPL(__rhashtable_init_noprof);
> >   */
> >  int __rhltable_init_noprof(struct rhltable *hlt,
> >                          const struct rhashtable_params *params,
> > -                        struct lock_class_key *key)
> > +                        struct lock_class_key *mutex_key,
> > +                        struct lock_class_key *bucket_key)
> >  {
> >       int err;
> >
> > -     err =3D __rhashtable_init_noprof(&hlt->ht, params, key);
> > +     err =3D __rhashtable_init_noprof(&hlt->ht, params, mutex_key, buc=
ket_key);
> >       hlt->ht.rhlist =3D true;
> >       return err;
> >  }
> >
> > ---
> > base-commit: 0131b508c0e2489eac6e121135988f6eeb716f19
> > change-id: 20260801-fix-rhashtable-bucket-lockdep-95e25abebeea
> >
> > Best regards,
> > --
> > quanyeyang <[email protected]>
> >
> >
> >
> >
>