Re: [PATCH bpf] bpf: Fail rhtab inserts and deletes with IRQs disabled

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Tue, Nov 03, 2026 at 10:46 AM Mani Tofigh via B4 Relay <devnull+mt4022.columbia.edu@kernel.org> wrote:
> A bucket lock is only ever held with IRQs disabled, so return -EBUSY
> from deletes and from inserts of new keys when IRQs are already
> disabled, like the other hash maps do on re-entry.  Lookups and updates

The other hash maps fail on re-entry only. rqspinlock detects it.
This check fails every insert and delete with irqs disabled.
That's perf_event progs, kprobes via int3, sched_switch and
sched_wakeup tracepoints, sched_ext ops under rq lock.
imo it's not worth breaking all of them for a rare deadlock.

> This is the check Alexei suggested on the RFC of the series [1].  It also
> stops perf_event and other programs that run with IRQs disabled from
> inserting or deleting keys.  I can turn the reproducer into a selftest
> if that helps.

It was my suggestion, sorry. It's too broad.
v4 of rhash picked bpf_disable_instrumentation() to keep these callers:
https://lore.kernel.org/bpf/20260513-rhash-v4-0-dd3d541ccb0b@meta.com/

Only a caller with irqs disabled can run inside the locked section of
this cpu. Lets trylock the bucket in that case and spin otherwise.

  caller with irqs disabled    today      this patch   trylock
  -------------------------    --------   ----------   -------
  bucket is not locked         works      -EBUSY       works
  locked by another cpu        spins      -EBUSY       -EBUSY
  locked by this cpu           deadlock   -EBUSY       -EBUSY

Then no need for bpf_disable_instrumentation() in rhtab.
params.trylock_irqsoff is a compile time constant in the fast path,
like insecure_elasticity. The other rhashtable users pay nothing.

The diff leaves one more bug: rhashtable_insert_rehash() calls
kvmalloc(GFP_ATOMIC). That's not ok in NMI.

Something like this (compile tested only):

diff --git a/include/linux/rhashtable-types.h b/include/linux/rhashtable-types.h
index 3576b8f..a5c4e69 100644
--- a/include/linux/rhashtable-types.h
+++ b/include/linux/rhashtable-types.h
@@ -52,6 +52,8 @@ typedef int (*rht_obj_cmpfn_t)(struct rhashtable_compare_arg *arg,
  * @min_size: Minimum size while shrinking
  * @insecure_elasticity: Set to true to disable chain length checks
  * @automatic_shrinking: Enable automatic shrinking of tables
+ * @trylock_irqsoff: With irqs disabled insert and remove don't spin on
+ *		     the bucket lock and return -EBUSY
  * @hashfn: Hash function (default: jhash2 if !(key_len % 4), or jhash)
  * @obj_hashfn: Function to hash object
  * @obj_cmpfn: Function to compare key with object
@@ -65,6 +67,7 @@ struct rhashtable_params {
 	u16			min_size;
 	bool			insecure_elasticity;
 	bool			automatic_shrinking;
+	bool			trylock_irqsoff;
 	rht_hashfn_t		hashfn;
 	rht_obj_hashfn_t	obj_hashfn;
 	rht_obj_cmpfn_t		obj_cmpfn;
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 70dac92..ba2c594 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -339,6 +339,33 @@ static inline unsigned long rht_lock(struct bucket_table *tbl,
 	return rht_lock_nested(tbl, bkt, 0);
 }

+/*
+ * The bucket lock is held with irqs disabled. A caller with irqs disabled
+ * may run inside the locked section of this cpu (NMI, kprobe, tracepoint).
+ * It cannot spin.
+ */
+static inline bool rht_lock_or_busy(struct bucket_table *tbl,
+				    struct rhash_lock_head __rcu **bkt,
+				    const struct rhashtable_params params,
+				    unsigned long *flags)
+	__cond_acquires(true, __bitlock(0, bkt))
+{
+	if (!params.trylock_irqsoff || !irqs_disabled()) {
+		*flags = rht_lock(tbl, bkt);
+		return true;
+	}
+	/* bit_spin_trylock() always succeeds on UP */
+	if (!IS_ENABLED(CONFIG_SMP))
+		return false;
+	local_irq_save(*flags);
+	if (!bit_spin_trylock(0, (unsigned long *)bkt)) {
+		local_irq_restore(*flags);
+		return false;
+	}
+	lock_acquire_exclusive(&tbl->dep_map, 0, 1, NULL, _THIS_IP_);
+	return true;
+}
+
 static inline void rht_unlock(struct bucket_table *tbl,
 			      struct rhash_lock_head __rcu **bkt,
 			      unsigned long flags)
@@ -785,7 +812,9 @@ static __always_inline void *__rhashtable_insert_fast(
 	if (!bkt)
 		goto out;
 	pprev = NULL;
-	flags = rht_lock(tbl, bkt);
+	data = ERR_PTR(-EBUSY);
+	if (!rht_lock_or_busy(tbl, bkt, params, &flags))
+		goto out;

 	if (unlikely(rcu_access_pointer(tbl->future_tbl))) {
 slow_path:
@@ -1066,7 +1095,8 @@ static __always_inline int __rhashtable_remove_fast_one(
 	if (!bkt)
 		return -ENOENT;
 	pprev = NULL;
-	flags = rht_lock(tbl, bkt);
+	if (!rht_lock_or_busy(tbl, bkt, params, &flags))
+		return -EBUSY;

 	rht_for_each_from(he, rht_ptr(bkt, tbl, hash), tbl, hash) {
 		struct rhlist_head *list;
@@ -1149,6 +1179,7 @@ static __always_inline int __rhashtable_remove_fast(
 	 */
 	while ((err = __rhashtable_remove_fast_one(ht, tbl, obj, params,
 						   rhlist)) &&
+	       err != -EBUSY &&
 	       (tbl = rht_dereference_rcu(tbl->future_tbl, ht)))
 		;

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 6e21154..edc7572 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2983,6 +2983,7 @@ struct bpf_rhtab {
 static const struct rhashtable_params rhtab_params = {
 	.head_offset = offsetof(struct rhtab_elem, node),
 	.key_offset  = offsetof(struct rhtab_elem, data),
+	.trylock_irqsoff = true,
 };

 static inline void *rhtab_elem_value(struct rhtab_elem *l, u32 key_size)
@@ -3011,6 +3012,7 @@ static const struct rhashtable_params rhtab_params_long = {
 	.head_offset = offsetof(struct rhtab_elem, node),
 	.key_offset  = offsetof(struct rhtab_elem, data),
 	.key_len     = sizeof(long),
+	.trylock_irqsoff = true,
 	.hashfn      = rhtab_hashfn_long,
 	.obj_cmpfn   = rhtab_key_cmp_long,
 };
@@ -3143,21 +3145,11 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
 {
 	int err;

-	/*
-	 * disable_instrumentation() mitigates the deadlock for programs running in NMI context.
-	 * rhashtable locks bucket with local_irq_save(). Only NMI programs may reenter
-	 * rhashtable code, bpf_disable_instrumentation() disables programs running in NMI, except
-	 * raw tracepoints, which we don't have in rhashtable.
-	 */
-	bpf_disable_instrumentation();
-
 	if (rhtab->map.key_size == sizeof(long))
 		err = rhashtable_remove_fast(&rhtab->ht, &elem->node, rhtab_params_long);
 	else
 		err = rhashtable_remove_fast(&rhtab->ht, &elem->node, rhtab_params);

-	bpf_enable_instrumentation();
-
 	if (err)
 		return err;

@@ -3268,16 +3260,11 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
 	memcpy(elem->data, key, map->key_size);
 	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);

-	/* Prevent deadlock for NMI programs attempting to take bucket lock */
-	bpf_disable_instrumentation();
-
 	if (map->key_size == sizeof(long))
 		tmp = rhashtable_lookup_get_insert_fast(&rhtab->ht, &elem->node, rhtab_params_long);
 	else
 		tmp = rhashtable_lookup_get_insert_fast(&rhtab->ht, &elem->node, rhtab_params);

-	bpf_enable_instrumentation();
-
 	if (tmp) {
 		bpf_mem_cache_free(&rhtab->ma, elem);
 		if (IS_ERR(tmp))
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 500c45c..2dc13f0 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -650,7 +650,8 @@ static void *rhashtable_try_insert(struct rhashtable *ht, const void *key,
 		} else {
 			bool inserted;

-			flags = rht_lock(tbl, bkt);
+			if (!rht_lock_or_busy(tbl, bkt, ht->p, &flags))
+				return ERR_PTR(-EBUSY);
 			data = rhashtable_lookup_one(ht, bkt, tbl,
 						     hash, key, obj);
 			new_tbl = rhashtable_insert_one(ht, bkt, tbl,

Pls add the reproducer as a selftest in patch 2 and cc Herbert Xu
on respin.

pw-bot: cr