Re: [PATCH bpf-next v3 5/7] selftests/bpf: Cover trusted-or-null BTF pointer reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Tests for atomic RMW operations and BPF_LOAD_ACQ on trusted-or-null pointers are missing, despite the commit message claiming they are verified.
- [Low] Multi-line comment opening '/*' is not on its own line, violating BPF subsystem guidelines.
--

commit a8459550255e4dfc12584cd94a1820a3e538fba1
Author: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>

selftests/bpf: Cover trusted-or-null BTF pointer reads

This commit updates verifier tests for unchecked trusted-or-null BTF pointer
dereferences and adds tests covering scalar reads and chained reads. It also
adds runtime tests to verify that non-NULL reads return the field value while
NULL reads return zero.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_maybe_null.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_maybe_null.c
> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_maybe_null.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_maybe_null.c
> @@ -19,15 +19,15 @@ static void maybe_null(void)
>  	struct_ops_maybe_null__destroy(skel);
>  }
>  
> -/* Test that the verifier rejects a program that access a nullable pointer
> - * without a check beforehand.
> +/* Test that the verifier accepts a fault-protected read through a nullable
> + * trusted pointer without an explicit NULL check.
>   */

[Severity: Low]
This is not a bug, but does this follow the BPF subsystem comment guidelines?
The guidelines require multi-line comments to have the opening /* on its own
line, rather than keeping the text on the same line as the opening characters.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
> --- a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
> +++ b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
[ ... ]
> +SEC("tp_btf/bpf_testmod_test_nullable_bare_tp")
> +__failure __msg("pointer arithmetic on trusted_ptr_or_null_ prohibited")
> +int BPF_PROG(handle_tp_btf_nullable_arith, struct bpf_testmod_test_read_ctx *nullable_ctx)
> +{
> +	asm volatile("%[ctx] += 1" : [ctx] "+r"(nullable_ctx));
> +	return nullable_ctx->len;
> +}

[Severity: Medium]
The commit message states that the tests verify that atomic RMW operations
and BPF_LOAD_ACQ accesses remain prohibited. Does this patch actually include
those tests? The test for pointer arithmetic is present here in
handle_tp_btf_nullable_arith(), but there do not seem to be any corresponding
tests for atomic RMW operations or BPF_LOAD_ACQ accesses in this file or
elsewhere in the patch.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831092305.42062-1-tasos.papagiannnis@gmail.com?part=5