Re: [PATCH] rhashtable: use per-init-site lockdep classes for bucket locks
NeilBrown <[email protected]> Sat, 01 Aug 2026 21:12:31 +1000
| Newsgroups | org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Sat, 01 Aug 2026, [email protected] wrote: > From: quanyeyang <[email protected]> >=20 > All bucket tables currently share a single lockdep class. This makes > lockdep conflate bucket locks from unrelated rhashtable instances. >=20 > 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 >=20 > 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. >=20 > 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. >=20 > Fixes: 149212f07856 ("rhashtable: add lockdep tracking to bucket bit-spin-l= ocks.") > 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(-) >=20 > diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-ty= pes.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 { > =20 > 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_ARGS_= _)) > =20 > 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_ARGS__)) > =20 > 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(struct r= hashtable *ht, > struct bucket_table *tbl =3D NULL; > size_t size; > int i; > - static struct lock_class_key __key; > =20 > tbl =3D alloc_hooks_tag(ht->alloc_tag, > kvmalloc_node_align_noprof(struct_size(tbl, buckets, nbuckets), > @@ -205,7 +204,12 @@ static struct bucket_table *bucket_table_alloc(struct = rhashtable *ht, > if (tbl =3D=3D NULL) > return NULL; > =20 > - 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 resize. > + */ > + lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", > + ht->bucket_lock_key, 0); > =20 > tbl->size =3D size; > =20 > @@ -1162,7 +1166,8 @@ static u32 rhashtable_jhash2(const void *key, u32 len= gth, 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 *ht, > return -EINVAL; > =20 > 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)); > =20 > @@ -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; > =20 > - err =3D __rhashtable_init_noprof(&hlt->ht, params, key); > + err =3D __rhashtable_init_noprof(&hlt->ht, params, mutex_key, bucket_key); > hlt->ht.rhlist =3D true; > return err; > } >=20 > --- > base-commit: 0131b508c0e2489eac6e121135988f6eeb716f19 > change-id: 20260801-fix-rhashtable-bucket-lockdep-95e25abebeea >=20 > Best regards, > -- =20 > quanyeyang <[email protected]> >=20 >=20 >=20 >=20