Re: [PATCH bpf] bpf: Propagate precision for outgoing stack arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: mark_all_scalars_imprecise() omits stack arguments, causing state explosion
--

commit 6af07cea664bce39c3788fcfdd7a5a9e4adbc1e1
Author: Jan-Gerd Tenberge <janten@gmail.com>
Subject: bpf: Propagate precision for outgoing stack arguments

BPF subprogram arguments beyond r1-r5 are kept separately in stack_arg_regs.
This commit includes precise outgoing stack arguments in the backtracking seed
used by propagate_precision(), and marks them precise in the fallback path
as well. This prevents the verifier from missing value-dependent safety checks.

> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 18bf7b660c2f6..c62bc9fe2e325 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
> @@ -1063,6 +1063,20 @@ static int propagate_precision(struct bpf_verifier_env *env,
>  			bpf_bt_set_frame_slot(&env->bt, fr, i);
>  			first = false;
>  		}
> +
> +		bpf_for_each_spilled_stack_arg(i, state, state_reg) {
> +			if (!state_reg || state_reg->type != SCALAR_VALUE ||
> +			    !state_reg->precise)
> +				continue;
> +			if (env->log.level & BPF_LOG_LEVEL2) {
> +				if (first)
> +					verbose(env, "frame %d: propagating sa%d", fr, i);
> +				else
> +					verbose(env, ",sa%d", i);
> +			}
> +			bt_set_frame_stack_arg_slot(&env->bt, fr, i);
> +			first = false;
> +		}
>  		if (!first && (env->log.level & BPF_LOG_LEVEL2))
>  			verbose(env, "\n");
>  	}

[Severity: Medium]
Does this patch also need to update mark_all_scalars_imprecise() in this
same file?

When a state is added to the explored states cache, mark_all_scalars_imprecise()
clears precision marks to ensure maximum pruning efficiency. It currently
iterates over standard registers and stack slots, but misses stack_arg_regs:

kernel/bpf/states.c:mark_all_scalars_imprecise() {
    ...
		for (j = 0; j < bpf_stack_nr_slots(func); j++) {
			struct bpf_stack_state *ss = bpf_stack_slot(func, j);

			if (!bpf_is_spilled_reg(ss))
				continue;
			reg = &ss->spilled_ptr;
			if (reg->type != SCALAR_VALUE)
				continue;
			reg->precise = false;
		}
    ...
}

Because this patch allows stack arguments to be marked precise, wouldn't any
precise stack argument enter the cache with precise = true? 

When a later verification path compares against this cached state via
regsafe(), the old stack argument having precise = true would enforce tight
bounds checks or an exact memory match.

Could this cause state explosion by preventing pruning for any state with
different scalar values?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009072752.21037-1-janten@gmail.com?part=1