* [PATCH bpf-next v2] selftests/bpf: Fix htab_update/reenter_update selftest failure
@ 2025-11-14 15:26 Saket Kumar Bhaskar
2025-11-14 15:50 ` bot+bpf-ci
0 siblings, 1 reply; 3+ messages in thread
From: Saket Kumar Bhaskar @ 2025-11-14 15:26 UTC (permalink / raw)
To: bpf, linux-kselftest, linux-kernel
Cc: hbathini, sachinpb, venkat88, andrii, eddyz87, ast, daniel,
martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, shuah
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;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v2] selftests/bpf: Fix htab_update/reenter_update selftest failure
2025-11-14 15:26 [PATCH bpf-next v2] selftests/bpf: Fix htab_update/reenter_update selftest failure Saket Kumar Bhaskar
@ 2025-11-14 15:50 ` bot+bpf-ci
2025-11-14 17:19 ` Saket Kumar Bhaskar
0 siblings, 1 reply; 3+ messages in thread
From: bot+bpf-ci @ 2025-11-14 15:50 UTC (permalink / raw)
To: skb99, bpf, linux-kselftest, linux-kernel
Cc: hbathini, sachinpb, venkat88, andrii, eddyz87, ast, daniel,
martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, shuah, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v2] selftests/bpf: Fix htab_update/reenter_update selftest failure
2025-11-14 15:50 ` bot+bpf-ci
@ 2025-11-14 17:19 ` Saket Kumar Bhaskar
0 siblings, 0 replies; 3+ messages in thread
From: Saket Kumar Bhaskar @ 2025-11-14 17:19 UTC (permalink / raw)
To: bot+bpf-ci
Cc: bpf, linux-kselftest, linux-kernel, hbathini, sachinpb, venkat88,
andrii, eddyz87, ast, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, martin.lau,
clm, ihor.solodrai
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-14 17:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 15:26 [PATCH bpf-next v2] selftests/bpf: Fix htab_update/reenter_update selftest failure Saket Kumar Bhaskar
2025-11-14 15:50 ` bot+bpf-ci
2025-11-14 17:19 ` Saket Kumar Bhaskar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox