Re: [PATCH bpf v3 2/2] selftests/bpf: Cover partial copy of non-linear test_run output

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized stack buffer `out` is read if the BPF test run fails.
--

commit f204128d45018a80c7b9b142c6d9a253204b22bd
Author: Sun Jian <sun.jian.kdev@gmail.com>

selftests/bpf: Cover partial copy of non-linear test_run output
    
This commit adds selftest coverage for partial copies of non-linear test_run
output for TC and XDP programs. It verifies that test_run returns -ENOSPC,
reports the full packet length, and copies the packet prefix into the
provided buffer.

> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c b/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> index 01f1d1b6715a6..9cc898e6a9f71 100644
> --- a/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
[ ... ]
> @@ -20,6 +24,69 @@ static void check_run_cnt(int prog_fd, __u64 run_cnt)
>  	      "incorrect number of repetitions, want %llu have %llu\n", run_cnt, info.run_cnt);
>  }
>  
> +static void init_pkt(__u8 *pkt, size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < len; i++)
> +		pkt[i] = i & 0xff;
> +}
> +
> +static void test_skb_nonlinear_data_out_partial(struct test_pkt_access *skel)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	__u8 pkt[NONLINEAR_PKT_LEN];
> +	__u8 out[SHORT_OUT_LEN];

[Severity: Medium]
Should out[] be explicitly zero-initialized?

If bpf_prog_test_run_opts() fails with an unexpected error and doesn't write
to the userspace buffer, the ASSERT_EQ macros below won't abort the test.
This means the subsequent ASSERT_OK(memcmp(...)) call will read uninitialized
stack memory, which can trigger undefined behavior and Memory Sanitizer (MSAN)
warnings.

> +	struct __sk_buff skb = {};
> +	int prog_fd, err;
> +
[ ... ]
> +	prog_fd = bpf_program__fd(skel->progs.tc_pass_prog);
> +	err = bpf_prog_test_run_opts(prog_fd, &topts);
> +
> +	ASSERT_EQ(err, -ENOSPC, "skb_nonlinear_partial_err");
> +	ASSERT_EQ(topts.data_size_out, sizeof(pkt), "skb_nonlinear_partial_data_size_out");
> +	ASSERT_OK(memcmp(out, pkt, sizeof(out)), "skb_nonlinear_partial_data_out");
> +}
> +
> +static void test_xdp_nonlinear_data_out_partial(struct test_pkt_access *skel)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	__u8 pkt[NONLINEAR_PKT_LEN];
> +	__u8 out[SHORT_OUT_LEN];

[Severity: Medium]
Could we also zero-initialize this buffer for the same reason?

> +	struct xdp_md ctx = {};
> +	int prog_fd, err;
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617093557.63880-1-sun.jian.kdev@gmail.com?part=2