Re: [PATCH bpf-next v6 6/6] selftests/bpf: Test subprogram instruction statistics

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Inline assembly in non-naked C functions implicitly reads argument registers, risking register corruption and test failures due to unstable instruction counts.
--

--- Patch [6]: [PATCH bpf-next v6 6/6] selftests/bpf: Test subprogram instruction statistics ---
commit 567cc6b8313bad87d8db2a9e9b28df5026767381
Author: Kumar Kartikeya Dwivedi <[email protected]>

selftests/bpf: Test subprogram instruction statistics

Adds small verifier programs with deterministic instruction streams to
exercise per-subprogram own and inclusive instruction accounting.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_insn_stats.c b/tools/testing/selftests/bpf/progs/verifier_subprog_insn_stats.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_subprog_insn_stats.c

[ ... ]

> +__noinline __used
> +static __u64 stats_async_schedule(struct bpf_map *map, __u32 *key,
> +				  struct timer_value *value, void *ctx)
> +{
> +	asm volatile (
> +		"r1 = r3;"

[Severity: Medium]
Because stats_async_schedule() is not marked __naked, could the compiler
emit prologue or epilogue instructions that change the exact instruction
count, causing the strict test assertions (e.g., insns_own 6) to fail?

Also, since r3 is implicitly read here without an input constraint, is
there a risk that the register allocator could clobber r3 before the
assembly block executes?

[ ... ]

> +__noinline __used
> +static int stats_async_outer(void *map, __u32 *key, struct bpf_timer *timer)
> +{
> +	asm volatile (
> +		"r1 = r3;"

[Severity: Medium]
Similarly, does the lack of __naked on stats_async_outer() risk unexpected
prologue instructions breaking the exact instruction counts?

Could the implicit use of r3 without an input constraint result in register
corruption?

[ ... ]

> +__noinline __used
> +static __u64 stats_async_nested_schedule(struct bpf_map *map, __u32 *key,
> +					 struct timer_value *value, void *ctx)
> +{
> +	asm volatile (
> +		"r1 = r3;"

[Severity: Medium]
Could the compiler emit extra instructions here for
stats_async_nested_schedule() since it is not marked __naked, breaking
the expected instruction counts?

Does reading r3 without constraints here also risk register corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6