Re: [PATCH bpf-next v3 3/3] selftests/bpf: Cover commuted pointer state propagation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Multi-line comment uses the deprecated style where the text starts on the same line as the opening `/*`.
--

--- Patch [3]: [PATCH bpf-next v3 3/3] selftests/bpf: Cover commuted pointer state propagation ---
commit 39ba76cb345769c48c9359e58e1fcdae124fef19
Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

selftests/bpf: Cover commuted pointer state propagation

This commit adds verifier coverage for three cases affected by preserving
the full pointer state across scalar += pointer operations. These cases
include stack frame number inheritance, readonly-untrusted memory access,
and dynptr data-slice invalidation.

> diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> index 94489ac64da8e..7da4266bae09b 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
> @@ -1635,6 +1635,37 @@ static int callback(__u32 index, void *data)
>          return 0;
>  }
>  
> +/* A commuted add should preserve the parent id of a dynptr data slice.
> + */

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the preferred
BPF subsystem style? It looks like the text starts on the same line as the
opening /*, whereas the modern kernel style guidelines typically prefer the
opening /* to be on its own line for multi-line comments.

> +SEC("?raw_tp")
> +__failure __msg("invalid mem access 'scalar'")
> +int dynptr_slice_commuted_invalidate(void *ctx)
> +{
> +	struct bpf_dynptr ptr;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784696371.git.chenyy23@mails.tsinghua.edu.cn?part=3