Re: [PATCH bpf-next 1/9] bpf: static subprogram arguments liveness should use BTF type info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: state explosion for non-BTF static subprograms
--

commit aaf0f2ac344fe68dd84ff5e9d26151d93e265a96
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: static subprogram arguments liveness should use BTF type info

When computing liveness marks for arguments of static subprograms, this
commit modifies the verifier to mark all arguments in the subprogram's
BTF type as live to avoid gaps in unreliable BTF detection.

> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index e657e6d95408f..024790097f960 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c

[ ... ]

> @@ -2623,14 +2623,15 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
>  			if (bpf_get_call_summary(env, insn, &cs)) {
>  				use = GENMASK(min_t(u8, cs.arg_slot_cnt, MAX_BPF_FUNC_REG_ARGS), 1);
>  			} else if (bpf_pseudo_call(insn)) {
> -				/* a static callee's reads are added in bpf_compute_live_registers() */
> +				/*
> +				 * Use BTF signatures as a starting point.
> +				 * See special case for static subprograms
> +				 * in the bpf_compute_live_registers()'s main loop.
> +				 */
>  				subprog = callee_subprog(env, insn);
> -				if (bpf_subprog_is_global(env, subprog)) {
> -					reg_args = min_t(u8, env->subprog_info[subprog].arg_slot_cnt, MAX_BPF_FUNC_REG_ARGS);
> -					use = GENMASK(reg_args, 1);
> -				} else {
> -					use = 0;
> -				}
> +				reg_args = min_t(u8, env->subprog_info[subprog].arg_slot_cnt,
> +						 MAX_BPF_FUNC_REG_ARGS);
> +				use = GENMASK(reg_args, 1);

[Severity: Medium]
Will this unconditionally force registers R1-R5 to be tracked as live for
static subprograms when BTF is missing?

Before this patch, `use` was initialized to 0 for static subprograms, which
correctly limited liveness to registers actually read by the callee. By
removing the `bpf_subprog_is_global()` check, could this unnecessarily widen
the liveness mask?

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2f190abe34489..fc1937b37dcff 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -14834,15 +14834,27 @@ static bool try_btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  	return !err;
>  }
>  
> -/* Prepare argument metadata for liveness; the main program is handled separately. */
> +/* Initialize env->subprog_info[].arg_slot_cnt */
>  static void prepare_subprog_args(struct bpf_verifier_env *env)
>  {
> +	struct bpf_prog_aux *aux = env->prog->aux;
> +	const struct btf_type *t;
>  	int i;
>  
> -	for (i = 1; i < env->subprog_cnt; i++) {
> -		if (!try_btf_prepare_func_args(env, i) && bpf_subprog_is_global(env, i))
> -			/* Keep all argument registers live if the prototype is unavailable. */
> -			env->subprog_info[i].arg_slot_cnt = MAX_BPF_FUNC_REG_ARGS;
> +	for (i = 0; i < env->subprog_cnt; i++) {
> +		/*
> +		 * Keep all argument registers live if the prototype is unavailable.
> +		 * No need to include stack args, as those are not supported for
> +		 * subprograms w/o BTF.
> +		 */
> +		env->subprog_info[i].arg_slot_cnt = MAX_BPF_FUNC_REG_ARGS;
> +		if (!aux->func_info)
> +			continue;

[Severity: Medium]
Does initializing `arg_slot_cnt` to 5 for all subprograms without BTF severely
degrade state pruning?

If a BPF program without BTF makes a pseudo-call to a static subprogram, the
verifier now requires exact state matches for R1-R5 during pruning, even if
the callee never reads them. Could this cause previously valid programs to be
rejected by the 1M instruction complexity limit due to state explosion?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-scev-minimal-v3-followups-v1-0-6000e95ef993@gmail.com?part=1