Re: [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The negative selftest uses an uninitialized stack buffer `buf`, causing the verifier to reject the program early and bypass the intended reference leak check.
--

commit 2c90a2c3822df01ad7da448a72985505048a4374
Author: Amery Hung <ameryhung@gmail.com>

selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test

This commit updates the use_file_dynptr_slice_after_put_file test to
use bpf_dynptr_slice instead of bpf_dynptr_data, since bpf_dynptr_data
always returns NULL for read-only file dynptrs. The slice is read
rather than written to, while the test is still expected to fail.

> diff --git a/tools/testing/selftests/bpf/progs/file_reader_fail.c b/tools/testing/selftests/bpf/progs/file_reader_fail.c
> index d5fae5e4cf9ad..3bb9e2612f8f3 100644
> --- a/tools/testing/selftests/bpf/progs/file_reader_fail.c
> +++ b/tools/testing/selftests/bpf/progs/file_reader_fail.c
> @@ -87,7 +87,8 @@ int use_file_dynptr_slice_after_put_file(void *ctx)
>  	struct task_struct *task = bpf_get_current_task_btf();
>  	struct file *file = bpf_get_task_exe_file(task);
>  	struct bpf_dynptr dynptr;
> -	char *data;
> +	char buf[1];
> +	const char *data;
>  
>  	if (!file)
>  		return 0;
> @@ -95,15 +96,14 @@ int use_file_dynptr_slice_after_put_file(void *ctx)
>  	if (bpf_dynptr_from_file(file, 0, &dynptr))
>  		goto out;
>  
> -	data = bpf_dynptr_data(&dynptr, 0, 1);
> +	data = bpf_dynptr_slice(&dynptr, 0, buf, sizeof(buf));

[Severity: Medium]
Will the BPF verifier reject this program early due to the uninitialized
stack buffer, masking the actual leak test?

The verifier requires memory passed to helpers like bpf_dynptr_slice() to be
initialized. Since buf is uninitialized, the verifier will likely fail with
an invalid read from stack error here, bypassing the intended reference leak
check.

Would initializing the buffer (e.g., char buf[1] = {0};) allow the test to
fail for the expected reason?

>  	if (!data)
>  		goto out;
>  
>  	/* this should fail - file dynptr should be discarded first to prevent resource leak */
>  	bpf_put_file(file);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604220904.1032145-1-ameryhung@gmail.com?part=3