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

[NeilBrown]

On Sat, 01 Aug 2026, quanyemostima@xxxxxxxxx wrote:
> From: quanyeyang <quanyemostima@xxxxxxxxx>
> 
> 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-spin-locks.")
> Reported-by: syzbot+ef8d17bae14efb960935@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
> Assisted-by: Cursor:GPT-5.6 Sol
> Signed-off-by: quanyeyang <quanyemostima@xxxxxxxxx>
> ---
>  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/rhashtable-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_ARGS__))
>  
>  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__))
>  
> 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 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,12 @@ 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);
> +	/*
> +	 * 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);
>  
>  	tbl->size = 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 *ht,
>  		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 = 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 = __rhashtable_init_noprof(&hlt->ht, params, key);
> +	err = __rhashtable_init_noprof(&hlt->ht, params, mutex_key, bucket_key);
>  	hlt->ht.rhlist = true;
>  	return err;
>  }
> 
> ---
> base-commit: 0131b508c0e2489eac6e121135988f6eeb716f19
> change-id: 20260801-fix-rhashtable-bucket-lockdep-95e25abebeea
> 
> Best regards,
> --  
> quanyeyang <quanyemostima@xxxxxxxxx>
> 
> 
> 
>