Re: [PATCH bpf-next v4 2/4] selftests/bpf: Add tests for bpf_call_rcu()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: data race in test_call_rcu_chain between callbacks increment and chain_err
--

commit 4178e68b1467222b91e572aaee89ee9d04f051af
Author: Puranjay Mohan <puranjay@kernel.org>

selftests/bpf: Add tests for bpf_call_rcu()

This commit adds tests for bpf_call_rcu() to cover callback execution
after a grace period, testing error handling on double arming, reuse of 
the head, and verifying proper verifier rejections for invalid usages.

> diff --git a/tools/testing/selftests/bpf/prog_tests/call_rcu.c b/tools/testing/selftests/bpf/prog_tests/call_rcu.c
> new file mode 100644
> index 0000000000000..c2d266aee8eac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/call_rcu.c

[ ... ]

> +static void test_call_rcu_chain(void)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	struct call_rcu *skel;
> +
> +	skel = call_rcu__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
> +		return;
> +
> +	skel->bss->chain = 1;
> +	if (!ASSERT_OK(bpf_prog_test_run_opts(bpf_program__fd(skel->progs.arm), &opts), "test_run"))
> +		goto out;
> +
> +	if (!wait_for_callbacks(skel, 1))
> +		goto out;
> +	ASSERT_EQ(skel->bss->chain_err, 0, "chain_err");

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/call_rcu.c b/tools/testing/selftests/bpf/progs/call_rcu.c
> new file mode 100644
> index 0000000000000..aa5d2b0fa36e4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/call_rcu.c

[ ... ]

> +static int reclaim(struct bpf_map *map, void *key, void *value)
> +{
> +	struct elem *e = value;
> +
> +	cb_key = *(__u32 *)key;
> +	__sync_fetch_and_or(&cb_keys, 1ULL << cb_key);
> +	cb_val = e->val;
> +	cb_max_entries = map->max_entries;
> +	e->val = 0;
> +	__sync_fetch_and_add(&callbacks, 1);
> +
> +	if (chain > 0) {
> +		chain--;
> +		chain_err = bpf_call_rcu(&e->rh, &arr, reclaim);
> +	}
> +	return 0;
> +}

[Severity: Low]
Could there be a data race here between the callback execution and userspace?

In test_call_rcu_chain(), userspace calls wait_for_callbacks(skel, 1), which
returns as soon as it observes the callbacks counter reach 1. It then
immediately asserts that chain_err == 0.

Since __sync_fetch_and_add(&callbacks, 1) happens before chain_err is updated
in reclaim(), userspace could potentially read a stale chain_err value (0)
before bpf_call_rcu() is actually invoked and writes its return value.

Would it be safer to increment the callbacks counter after updating chain_err?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915154248.3612028-1-puranjay@kernel.org?part=2