[PATCH v2] rhashtable: use private lockdep class for all locks.

NeilBrown <[email protected]>
Newsgroups org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
This patch builds on
 Commit: 060d4e94b8d4 ("rhashtable: give each instance its own lockdep class")

to allow lockdep to see each rhashtable as unique with respect to all of
the locks, not just the ht->mutex.

This is needed if rhashtable is to be used tracepoint BPF which could
run while an rhashtable lock (in a different table) is held - see Link
below.

Rather then adding extra keys for the other locks, we use distinct
sub-classes for the different locks.
rhashtable->lock uses class 0
rhashtable->mutex uses class 1
bucket bitlocks, which are the only ones that are ever nested, use
 classes 2 and 3.

Currently rht_lock() and rht_lock_nested() are quite separate code
despite the near-identical function.  This patch moves rht_lock() to
after rht_lock_nested(), and simply calls that other function with a
nesting level of zero.

Link: https://lore.kernel.org/all/20260801-fix-rhashtable-bucket-lockdep-v1-1-15a0f8ae094c@gmail.com/
Closes: https://syzkaller.appspot.com/bug?extid=ef8d17bae14efb960935
Tested-by: quanyeyang <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---

v2 includes a change to test_rhashtable.c as advised by sashiko.dev
NeilBrown


 include/linux/rhashtable-types.h |  3 +++
 include/linux/rhashtable.h       | 22 +++++++++-------------
 lib/rhashtable.c                 | 14 +++++++++++---
 lib/test_rhashtable.c            |  2 +-
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-types.h
index 57c11ec9dc64..0e1b172a4f6c 100644
--- a/include/linux/rhashtable-types.h
+++ b/include/linux/rhashtable-types.h
@@ -97,6 +97,9 @@ struct rhashtable {
 #ifdef CONFIG_MEM_ALLOC_PROFILING
 	struct alloc_tag		*alloc_tag;
 #endif
+#ifdef CONFIG_LOCKDEP
+	struct lock_class_key		*lockdep_key;
+#endif
 };
 
 /**
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 79f83b6eec27..f8358d43691b 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -320,18 +320,6 @@ static inline struct rhash_lock_head __rcu **rht_bucket_insert(
  * When we write to a bucket without unlocking, we use rht_assign_locked().
  */
 
-static inline unsigned long rht_lock(struct bucket_table *tbl,
-				     struct rhash_lock_head __rcu **bkt)
-	__acquires(__bitlock(0, bkt))
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	bit_spin_lock(0, (unsigned long *)bkt);
-	lock_map_acquire(&tbl->dep_map);
-	return flags;
-}
-
 static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
 					struct rhash_lock_head __rcu **bucket,
 					unsigned int subclass)
@@ -341,10 +329,18 @@ static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
 
 	local_irq_save(flags);
 	bit_spin_lock(0, (unsigned long *)bucket);
-	lock_acquire_exclusive(&tbl->dep_map, subclass, 0, NULL, _THIS_IP_);
+	/* subclass 0 is used for ->lock and 1 for ->mutex. 2+ for bitlocks */
+	lock_acquire_exclusive(&tbl->dep_map, subclass+2, 0, NULL, _THIS_IP_);
 	return flags;
 }
 
+static inline unsigned long rht_lock(struct bucket_table *tbl,
+				     struct rhash_lock_head __rcu **bkt)
+	__acquires(__bitlock(0, bkt))
+{
+	return rht_lock_nested(tbl, bkt, 0);
+}
+
 static inline void rht_unlock(struct bucket_table *tbl,
 			      struct rhash_lock_head __rcu **bkt,
 			      unsigned long flags)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 8b2c405e7a66..a99283a887ea 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,10 @@ 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);
+#ifdef CONFIG_LOCKDEP
+	/* bitlocks must use nesting level 2 or more */
+	lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", ht->lockdep_key, 0);
+#endif
 
 	tbl->size = size;
 
@@ -428,7 +430,7 @@ static void rht_deferred_worker(struct work_struct *work)
 	int err = 0;
 
 	ht = container_of(work, struct rhashtable, run_work);
-	mutex_lock(&ht->mutex);
+	mutex_lock_nested(&ht->mutex, 1);
 
 	tbl = rht_dereference(ht->tbl, ht);
 	tbl = rhashtable_last_table(ht, tbl);
@@ -1172,8 +1174,14 @@ int __rhashtable_init_noprof(struct rhashtable *ht,
 		return -EINVAL;
 
 	memset(ht, 0, sizeof(*ht));
+	/* mutex_lock must use nesting level 1 */
 	mutex_init_with_key(&ht->mutex, key);
 	spin_lock_init(&ht->lock);
+	/* spin_lock can use nesting level 0 */
+	lockdep_set_class(&ht->lock, key);
+#ifdef CONFIG_LOCKDEP
+	ht->lockdep_key = key;
+#endif
 	memcpy(&ht->p, params, sizeof(*params));
 
 	alloc_tag_record(ht->alloc_tag);
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index b767a38a74f9..85a615e74591 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -477,7 +477,7 @@ static unsigned int __init print_ht(struct rhltable *rhlt)
 
 	ht = &rhlt->ht;
 	/* Take the mutex to avoid RCU warning */
-	mutex_lock(&ht->mutex);
+	mutex_lock_nested(&ht->mutex, 1);
 	tbl = rht_dereference(ht->tbl, ht);
 	for (i = 0; i < tbl->size; i++) {
 		struct rhash_head *pos, *next;

base-commit: dac3e89a2c90c2feeb471e1f22a2512ad424b792
-- 
2.50.0.107.gf914562f5916.dirty
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.