Re: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf_mark_live_subregs_zext() fails to mark pending zero-extensions for registers of type PTR_TO_ARENA on pruned paths.
--

--- Patch [1]: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state ---
commit 59834505da01b398ada03dbde42eaf69952c910a
Author: Daniel Borkmann <[email protected]>

bpf: Mark pending sub-register zero extension before pruning a state

This commit ensures that 32-bit sub-register definitions maintain their
zero-extension marks even on pruned verification paths. It achieves this
by marking live sub-registers for zero extension when the verifier stops
its walk at an equivalent state.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d925197c2e5f7..addcfd1319c5c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -3161,6 +3161,44 @@ static void mark_insn_zext(struct bpf_verifier_env *env,
[ ... ]
> +void bpf_mark_live_subregs_zext(struct bpf_verifier_env *env,
> +				struct bpf_verifier_state *vstate)
> +{
> +	struct bpf_insn_aux_data *aux = env->insn_aux_data;
> +	struct bpf_func_state *func;
> +	u16 live_regs;
> +	int i, j;
> +
> +	for (i = vstate->curframe; i >= 0; i--) {
> +		live_regs = aux[bpf_frame_insn_idx(vstate, i)].live_regs_before;
> +		func = vstate->frame[i];
> +		for (j = 0; j < BPF_REG_FP; j++) {
> +			if (!(live_regs & BIT(j)))
> +				continue;
> +			if (func->regs[j].type != SCALAR_VALUE)
> +				continue;

[Severity: Critical]
Does this check inadvertently skip PTR_TO_ARENA registers?

PTR_TO_ARENA is also a 32-bit register type that relies on subreg_def for
zero extension. If a BPF program casts a 64-bit value to a 32-bit
PTR_TO_ARENA, branches, and gets pruned, skipping PTR_TO_ARENA here
would leave its subreg_def unmarked.

Could this lead to missing zero-extensions in the JIT, leaving garbage in
the upper 32 bits and potentially causing out-of-bounds memory accesses
outside the arena boundaries?

> +			mark_insn_zext(env, &func->regs[j]);
> +		}
> +	}
> +}

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