Re: [PATCH bpf-next v2 6/7] selftests/bpf: Adjust verifier_map_ptr for the map's excl field

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete and degrading adjustment of verifier_map_ptr tests after struct bpf_map layout change.
--

commit 5b67581177de15e6a8e536ecacc43112b6c44726
Author: KP Singh <kpsingh@kernel.org>

selftests/bpf: Adjust verifier_map_ptr for the map's excl field

Adding the u32 excl field at offset 32 of struct bpf_map right after the
sha[SHA256_DIGEST_SIZE] hash shifts the ops pointer from offset 32 to 40.
This commit attempts to fix up the test case by updating the expected error
messages.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
> index e2767d27d8aaf..d8e822d1a8bab 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
> @@ -70,13 +70,15 @@ __naked void bpf_map_ptr_write_rejected(void)
>  	: __clobber_all);
>  }
>  
> -/* The first element of struct bpf_map is a SHA256 hash of 32 bytes, accessing
> - * into this array is valid. The opts field is now at offset 33.
> +/*
> + * struct bpf_map starts with the SHA256 hash sha[32] at offset 0 (a readable
> + * byte array), followed by the u32 excl field at offset 32. Reading a u32 at
> + * offset 33 runs past the end of excl and is rejected.
>   */
>  SEC("socket")
>  __description("bpf_map_ptr: read non-existent field rejected")
>  __failure
> -__msg("cannot access ptr member ops with moff 32 in struct bpf_map with off 33 size 4")
> +__msg("access beyond the end of member excl (mend:36) in struct bpf_map with off 33 size 4")
>  __failure_unpriv
>  __msg_unpriv("access is allowed only to CAP_PERFMON and CAP_SYS_ADMIN")
>  __flag(BPF_F_ANY_ALIGNMENT)

[Severity: Medium]
Does this adjustment reduce the test suite's coverage?

By only changing the expected error message here,
read_non_existent_field_rejected() now tests for a scalar out-of-bounds
access rather than testing the verifier's rejection of partial pointer
accesses.

Should the offset inside read_non_existent_field_rejected() be updated to 41
instead, to continue testing partial access to the ops pointer (which shifted
to offset 40)?

tools/testing/selftests/bpf/progs/verifier_map_ptr.c:read_non_existent_field_rejected() {
    ...
    r6 = *(u32*)(r1 + 33);
    ...
}

Additionally, ptr_read_ops_field_accepted() explicitly claims to read the
ops field, but still reads from the sha array at offset 0:

tools/testing/selftests/bpf/progs/verifier_map_ptr.c:ptr_read_ops_field_accepted() {
    ...
    r6 = *(u64*)(r1 + 0);
    ...
}

Should this test also be updated to read from offset 40 to properly verify
valid access to the ops pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601150248.394863-1-daniel@iogearbox.net?part=6