Since commit 31158ad02ddb ("rqspinlock: Add deadlock detection and recovery") the updated path on re-entrancy now reports deadlock via -EDEADLK instead of the previous -EBUSY.
Also, the way reentrancy was exercised (via fentry/lookup_elem_raw) has been fragile because lookup_elem_raw may be inlined (find_kernel_btf_id() will return -ESRCH).
To fix this fentry is attached to bpf_obj_free_fields() instead of lookup_elem_raw() and:
- The htab map is made to use a BTF-described struct val with a struct bpf_timer so that check_and_free_fields() reliably calls bpf_obj_free_fields() on element replacement.
- The selftest is updated to do two updates to the same key (insert + replace) in prog_test.
- The selftest is updated to align with expected errno with the kernel’s current behavior.
Signed-off-by: Saket Kumar Bhaskar skb99@linux.ibm.com --- Changes since v1: Addressed comments from Alexei: * Fixed the scenario where test may fail when lookup_elem_raw() is inlined.
v1: https://lore.kernel.org/all/20251106052628.349117-1-skb99@linux.ibm.com/
.../selftests/bpf/prog_tests/htab_update.c | 38 ++++++++++++++----- .../testing/selftests/bpf/progs/htab_update.c | 19 +++++++--- 2 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/htab_update.c b/tools/testing/selftests/bpf/prog_tests/htab_update.c index 2bc85f4814f4..96b65c1a321a 100644 --- a/tools/testing/selftests/bpf/prog_tests/htab_update.c +++ b/tools/testing/selftests/bpf/prog_tests/htab_update.c @@ -15,17 +15,17 @@ struct htab_update_ctx { static void test_reenter_update(void) { struct htab_update *skel; - unsigned int key, value; + void *value = NULL; + unsigned int key, value_size; int err;
skel = htab_update__open(); if (!ASSERT_OK_PTR(skel, "htab_update__open")) return;
- /* lookup_elem_raw() may be inlined and find_kernel_btf_id() will return -ESRCH */ - bpf_program__set_autoload(skel->progs.lookup_elem_raw, true); + bpf_program__set_autoload(skel->progs.bpf_obj_free_fields, true); err = htab_update__load(skel); - if (!ASSERT_TRUE(!err || err == -ESRCH, "htab_update__load") || err) + if (!ASSERT_TRUE(!err, "htab_update__load") || err) goto out;
skel->bss->pid = getpid(); @@ -33,14 +33,32 @@ static void test_reenter_update(void) if (!ASSERT_OK(err, "htab_update__attach")) goto out;
- /* Will trigger the reentrancy of bpf_map_update_elem() */ - key = 0; - value = 0; - err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, 0); - if (!ASSERT_OK(err, "add element")) + value_size = bpf_map__value_size(skel->maps.htab); + + value = calloc(1, value_size); + if (!ASSERT_OK_PTR(value, "calloc value")) + goto out; + /* + * First update: plain insert. This should NOT trigger the re-entrancy + * path, because there is no old element to free yet. + */ + err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, BPF_ANY); + if (!ASSERT_OK(err, "first update (insert)")) + goto out; + + /* + * Second update: replace existing element with same key and trigger + * the reentrancy of bpf_map_update_elem(). + * check_and_free_fields() calls bpf_obj_free_fields() on the old + * value, which is where fentry program runs and performs a nested + * bpf_map_update_elem(), triggering -EDEADLK. + */ + memset(&value, 0, sizeof(value)); + err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, BPF_ANY); + if (!ASSERT_OK(err, "second update (replace)")) goto out;
- ASSERT_EQ(skel->bss->update_err, -EBUSY, "no reentrancy"); + ASSERT_EQ(skel->bss->update_err, -EDEADLK, "no reentrancy"); out: htab_update__destroy(skel); } diff --git a/tools/testing/selftests/bpf/progs/htab_update.c b/tools/testing/selftests/bpf/progs/htab_update.c index 7481bb30b29b..195d3b2fba00 100644 --- a/tools/testing/selftests/bpf/progs/htab_update.c +++ b/tools/testing/selftests/bpf/progs/htab_update.c @@ -6,24 +6,31 @@
char _license[] SEC("license") = "GPL";
+/* Map value type: has BTF-managed field (bpf_timer) */ +struct val { + struct bpf_timer t; + __u64 payload; +}; + struct { __uint(type, BPF_MAP_TYPE_HASH); __uint(max_entries, 1); - __uint(key_size, sizeof(__u32)); - __uint(value_size, sizeof(__u32)); + __type(key, __u32); + __type(value, struct val); } htab SEC(".maps");
int pid = 0; int update_err = 0;
-SEC("?fentry/lookup_elem_raw") -int lookup_elem_raw(void *ctx) +SEC("?fentry/bpf_obj_free_fields") +int bpf_obj_free_fields(void *ctx) { - __u32 key = 0, value = 1; + __u32 key = 0; + struct val value = { .payload = 1 };
if ((bpf_get_current_pid_tgid() >> 32) != pid) return 0;
- update_err = bpf_map_update_elem(&htab, &key, &value, 0); + update_err = bpf_map_update_elem(&htab, &key, &value, BPF_ANY); return 0; }
diff --git a/tools/testing/selftests/bpf/prog_tests/htab_update.c b/tools/testing/selftests/bpf/prog_tests/htab_update.c index 2bc85f481..96b65c1a3 100644 --- a/tools/testing/selftests/bpf/prog_tests/htab_update.c +++ b/tools/testing/selftests/bpf/prog_tests/htab_update.c @@ -15,17 +15,17 @@ struct htab_update_ctx { static void test_reenter_update(void) { struct htab_update *skel;
- unsigned int key, value;
- void *value = NULL;
- unsigned int key, value_size;
^^^
Is key supposed to be initialized? The old code had 'unsigned int key = 0, value = 0;' but the new code drops the initialization. Later uses of key at the bpf_map_update_elem() calls will pass uninitialized stack memory.
int err;
skel = htab_update__open(); if (!ASSERT_OK_PTR(skel, "htab_update__open")) return;
[ ... ]
@@ -33,14 +33,32 @@ static void test_reenter_update(void) if (!ASSERT_OK(err, "htab_update__attach")) goto out;
- /* Will trigger the reentrancy of bpf_map_update_elem() */
- key = 0;
- value = 0;
- err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, 0);
- if (!ASSERT_OK(err, "add element"))
- value_size = bpf_map__value_size(skel->maps.htab);
- value = calloc(1, value_size);
- if (!ASSERT_OK_PTR(value, "calloc value"))
goto out;- /*
* First update: plain insert. This should NOT trigger the re-entrancy* path, because there is no old element to free yet.*/- err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, BPF_ANY);
- if (!ASSERT_OK(err, "first update (insert)"))
goto out;- /*
* Second update: replace existing element with same key and trigger* the reentrancy of bpf_map_update_elem().* check_and_free_fields() calls bpf_obj_free_fields() on the old* value, which is where fentry program runs and performs a nested* bpf_map_update_elem(), triggering -EDEADLK.*/- memset(&value, 0, sizeof(value));
^^^^^^^
Does this memset zero the allocated memory? It looks like this zeros the pointer variable itself (8 bytes) rather than the memory it points to. Should this be 'memset(value, 0, value_size)' instead?
- err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, BPF_ANY);
- if (!ASSERT_OK(err, "second update (replace)")) goto out;
- ASSERT_EQ(skel->bss->update_err, -EBUSY, "no reentrancy");
- ASSERT_EQ(skel->bss->update_err, -EDEADLK, "no reentrancy");
out: htab_update__destroy(skel); }
--- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19369517166
On Fri, Nov 14, 2025 at 03:50:44PM +0000, bot+bpf-ci@kernel.org wrote:
diff --git a/tools/testing/selftests/bpf/prog_tests/htab_update.c b/tools/testing/selftests/bpf/prog_tests/htab_update.c index 2bc85f481..96b65c1a3 100644 --- a/tools/testing/selftests/bpf/prog_tests/htab_update.c +++ b/tools/testing/selftests/bpf/prog_tests/htab_update.c @@ -15,17 +15,17 @@ struct htab_update_ctx { static void test_reenter_update(void) { struct htab_update *skel;
- unsigned int key, value;
- void *value = NULL;
- unsigned int key, value_size;
^^^Is key supposed to be initialized? The old code had 'unsigned int key = 0, value = 0;' but the new code drops the initialization. Later uses of key at the bpf_map_update_elem() calls will pass uninitialized stack memory.
int err;
skel = htab_update__open(); if (!ASSERT_OK_PTR(skel, "htab_update__open")) return;
[ ... ]
@@ -33,14 +33,32 @@ static void test_reenter_update(void) if (!ASSERT_OK(err, "htab_update__attach")) goto out;
- /* Will trigger the reentrancy of bpf_map_update_elem() */
- key = 0;
- value = 0;
- err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, 0);
- if (!ASSERT_OK(err, "add element"))
- value_size = bpf_map__value_size(skel->maps.htab);
- value = calloc(1, value_size);
- if (!ASSERT_OK_PTR(value, "calloc value"))
goto out;- /*
* First update: plain insert. This should NOT trigger the re-entrancy* path, because there is no old element to free yet.*/- err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, BPF_ANY);
- if (!ASSERT_OK(err, "first update (insert)"))
goto out;- /*
* Second update: replace existing element with same key and trigger* the reentrancy of bpf_map_update_elem().* check_and_free_fields() calls bpf_obj_free_fields() on the old* value, which is where fentry program runs and performs a nested* bpf_map_update_elem(), triggering -EDEADLK.*/- memset(&value, 0, sizeof(value));
^^^^^^^Does this memset zero the allocated memory? It looks like this zeros the pointer variable itself (8 bytes) rather than the memory it points to. Should this be 'memset(value, 0, value_size)' instead?
- err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, BPF_ANY);
- if (!ASSERT_OK(err, "second update (replace)")) goto out;
- ASSERT_EQ(skel->bss->update_err, -EBUSY, "no reentrancy");
- ASSERT_EQ(skel->bss->update_err, -EDEADLK, "no reentrancy");
out: htab_update__destroy(skel); }
AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19369517166
Will fix these.
linux-kselftest-mirror@lists.linaro.org