Re: [PATCH bpf-next v6 6/7] selftests/bpf: Cover BPF program manual loading

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: flaky test due to missing PID filtering and RCU detach delay
--

commit 5554ecac607fedce9b924955d0f276b639ab57c5
Author: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
Subject: selftests/bpf: Cover BPF program manual loading

This commit adds a manual load test for BPF programs, covering declarative
and imperative manual load strategies, attach/detach/reload cycles, and a
deferred module BTF attach. It also adds a signed loader test to ensure
that manual programs are rejected when a gen loader object is used.

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

[ ... ]

> +	/*
> +	 * detach prog3 explicitly: its link is kept in a local variable, not in
> +	 * skel->links, so test_manual_load__destroy() has nothing to detach
> +	 */
> +	err = bpf_link__destroy(link);
> +	link = NULL;
> +	if (!ASSERT_OK(err, "link_destroy"))
> +		goto cleanup;
> +
> +	/* reset the call flag after detach */
> +	skel->bss->prog3_called = false;
> +
> +	usleep(1);
> +
> +	ASSERT_FALSE(skel->bss->prog3_called, "prog3_called");

[Severity: Low]
Since tracepoint unregistration uses RCU, could a concurrent syscall on
another CPU trigger the program during the grace period and set
prog3_called to true?

This might cause the test to sporadically fail when run in parallel.

[ ... ]

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

[ ... ]

> +/*
> + * disabled at parse time; its attach target is resolved against module
> + * BTF via bpf_program__set_attach_target() before it is switched to
> + * MANUAL and loaded
> + */
> +SEC("?fentry")
> +int BPF_PROG(prog5, struct file *file, struct kobject *kobj,
> +	     const struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
> +{
> +	prog5_sz = len;
> +	return 0;
> +}

[Severity: Low]
Does this tracepoint need PID filtering?

When tests are run in parallel, concurrent tests triggering the testmod read
with different lengths could overwrite prog5_sz, which would fail the length
assertion in the test.

Filtering on the test process's PID (e.g., bpf_get_current_pid_tgid() >> 32
== test_pid) might prevent this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010143156.379864-1-andrey.grodzovsky@crowdstrike.com?part=6