Re: [PATCH bpf-next v8 3/6] bpf: Show more useful info in stack depth stats

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch breaks `veristat`'s ability to parse and track the BPF verifier's stack depth statistics (both STACK and MAX_STACK metrics).
--

--- Patch [3]: [PATCH bpf-next v8 3/6] bpf: Show more useful info in stack depth stats ---
commit de454cf9ad70eafec58298c609cb8cd8aea82368
Author: Kumar Kartikeya Dwivedi <[email protected]>

bpf: Show more useful info in stack depth stats

This commit updates the BPF verifier statistics to report stack depth and
instruction counts separately per subprogram. It changes the log output format
to display subprogram-specific metrics more clearly, rather than appending
depths in a single line.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index fced21ec23040..73d6cd563cdf8 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18693,15 +18693,20 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>  	if (env->log.level & BPF_LOG_STATS) {
>  		verbose(env, "verification time %lld usec\n",
>  			div_u64(env->verification_time, 1000));
> -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
> -		for (i = 1; i < subprog_cnt; i++)
> -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
> -		verbose(env, " max %d\n", env->max_stack_depth);
> -		verbose(env, "insns processed %d", env->subprog_info[0].insns_total);
> -		for (i = 1; i < subprog_cnt; i++)
> -			if (bpf_subprog_is_global(env, i))
> -				verbose(env, "+%d", env->subprog_info[i].insns_total);
> -		verbose(env, "\n");
> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);

[Severity: Medium]
Does this change in output format break the veristat tool's parsing logic?

Looking at tools/testing/selftests/bpf/veristat.c:parse_verif_log():

	if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK])) {
		...
	}

Because the output format changed from "stack depth %d+%d... max %d\n" to
"stack depth max %d\n", sscanf() will match "%511s" to the literal string
"max". The expected literal " max " then fails to match the remainder of the
string.

This causes sscanf to return 1, leaving s->stats[MAX_STACK] untouched and
breaking the stack calculator which tries to parse "max" for integers. Should
veristat be updated alongside this change to parse the new output format?

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