Re: [PATCH bpf-next v3 2/3] selftests/bpf: Test freeing a struct_ops map under a running prog

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: Infinite loop in selftest on bpf_prog_test_run_opts failure
--

commit baed672ead379b019d7064743fc22c2ee377b163
Author: Qiliang Yuan <odys.yuan@gmail.com>

    selftests/bpf: Test freeing a struct_ops map under a running prog
    
    st_ops_assoc_in_timer_no_uref drops every reference to the struct_ops
    map while a timer callback of its prog calls .test_1 through the
    association, and checks that the callback ends up with no struct_ops.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
> index 461ded7223515..987117091c676 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
> @@ -157,10 +158,13 @@ static void test_st_ops_assoc_in_timer_no_uref(void)
>  	/*
>  	 * Run .test_1 by calling kfunc bpf_kfunc_multi_st_ops_test_1_prog_arg() and checks
>  	 * the return value. .test_1 will also schedule timer_cb that runs .test_1 again.
> -	 * timer_cb will run 500ms after syscall_prog runs, when the user space no longer
> -	 * holds a reference to st_ops_map.
> +	 * timer_cb will run timer_ns after syscall_prog runs and, with wait_map_free, every
> +	 * timer_ns after that until the map is freed. With 500ms, it first runs when the
> +	 * user space no longer holds a reference to st_ops_map.
>  	 */
> -	skel->bss->timer_ns = 500000000;
> +	skel->bss->timer_ns = timer_ns;
> +	skel->bss->wait_map_free = wait_map_free;
> +	skel->bss->spin_loops = spin_loops;
>  	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog), NULL);
>  	ASSERT_OK(err, "bpf_prog_test_run_opts");

[Severity: Medium]
Will this lead to an infinite loop if bpf_prog_test_run_opts() fails?

The ASSERT_OK macro merely logs errors without aborting execution.
If bpf_prog_test_run_opts() fails to execute the test program, the code will
continue to the loop further down in run_st_ops_assoc_in_timer_no_uref():

	/* Check the return of the kfunc after timer_cb runs */
	while (!READ_ONCE(skel->bss->timer_cb_run))
		sched_yield();

Since the program never executed, timer_cb_run will remain 0, and the test
suite will hang indefinitely instead of failing cleanly.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-selftests-bpf-struct-ops-assoc-timer-v3-0-efe643d4b2a3@gmail.com?part=2