Re: [PATCH bpf-next v3 01/10] bpf: Implement resizable hashmap basic functions

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Critical] Using rhashtable for BPF map updates/deletes is unsafe and will lead to hard deadlocks from IRQ/NMI/softirq contexts.
--

commit 47c235933705172d3a36c4339c754282500161f1
Author: Mykyta Yatsenko <yatsenko@meta.com>

bpf: Implement resizable hashmap basic functions

This commit adds a new BPF map type BPF_MAP_TYPE_RHASH backed by the kernel
rhashtable implementation. It provides basic map operations like lookup,
update, and delete using rhashtable APIs.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 3dd9b4924ae4fb..a37bd2a7b30f4b 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> +static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem)
> +{
> +	int err;
> +
> +	err = rhashtable_remove_fast(&rhtab->ht, &elem->node, rhtab->params);

Can this lead to a deadlock when used by non-sleepable BPF programs in
interrupt or softirq contexts?

The verifier permits BPF_MAP_TYPE_RHASH to be used by non-sleepable BPF
programs, such as those attached to XDP or TC, which execute in softirq
context and do not check bpf_prog_active.

Map operations like rhashtable_remove_fast() and
rhashtable_lookup_get_insert_fast() internally use bit_spin_lock() for
bucket-level synchronization. This locking mechanism disables preemption
but leaves hardware and software interrupts enabled, unlike the
raw_spin_lock_irqsave() approach used by the standard htab.

If a syscall updates the map from process context and acquires the bucket
lock, and is then preempted by a hardware interrupt that schedules a softirq
on return, the softirq will execute the XDP or TC BPF program.

If this BPF program attempts to access the same hash bucket, wouldn't it spin
indefinitely waiting for the bit_spin_lock held by the interrupted process,
causing a system deadlock?

[ ... ]

> +static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u64 map_flags)
> +{
> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
> +	struct rhtab_elem *elem, *tmp;

[ ... ]

> +	tmp = rhashtable_lookup_get_insert_fast(&rhtab->ht, &elem->node, rhtab->params);

Does the same deadlock mechanism apply here as well, given that
rhashtable_lookup_get_insert_fast() also relies on bucket-level locks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260424-rhash-v3-0-d0fa0ce4379b@meta.com?part=1