Re: [PATCH bpf-next v3 1/2] bpf: Enforce gotox targets against subprog bounds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch adds a runtime bounds check to prevent `gotox` targets from jumping outside the current subprogram, but fails to ensure that dynamically observed targets were actually modeled during Control Flow Graph (CFG) construction. This allows an attacker to follow unmodeled control-flow edges and bypass static liveness-based state pruning.
--

commit 5e0bdd5b4a7e2ce073913482349669d9387d6a32
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

bpf: Enforce gotox targets against subprog bounds

During CFG construction, the verifier records the modeled gotox target set
in insn_aux_data->jt. This patch adds a check to reject targets outside the
calling subprogram bounds when check_indirect_jump() follows targets from
the runtime PTR_TO_INSN register's actual INSN_ARRAY map.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -17188,6 +17190,23 @@ static int check_indirect_jump(struct bpf_verifier_env *env, struct bpf_insn *in
>  		return -EINVAL;
>  	}
>  
> +	subprog = bpf_find_containing_subprog(env, env->insn_idx);
> +	if (verifier_bug_if(!subprog, env,
> +			    "gotox insn %d is outside subprog bounds\n",
> +			    env->insn_idx))
> +		return -EFAULT;
> +	subprog_start = subprog->start;
> +	subprog_end = (subprog + 1)->start;
> +
> +	for (i = 0; i < n; i++) {
> +		u32 target = env->gotox_tmp_buf->items[i];
> +
> +		if (target < subprog_start || target >= subprog_end) {

[Severity: Critical]
This is a pre-existing issue, but does this bounds check alone fully protect
against unmodeled control-flow edges?

During CFG construction in kernel/bpf/cfg.c:jt_from_subprog(), if an
INSN_ARRAY map's minimum offset falls outside the subprogram, the heuristic
skips the map entirely:

    if (jt_cur->items[0] >= subprog_start && jt_cur->items[0] < subprog_end) {

If an attacker creates an INSN_ARRAY map containing both an out-of-bounds
offset (e.g., 0) and a valid in-bounds target Y, the CFG construction drops
Y completely because items[0] is less than subprog_start.

Later at runtime, if the attacker restricts the bounds to index exactly Y,
this check passes because Y is within subprog_start and subprog_end. The
verifier then explores the gotox to Y edge that was never modeled in the
CFG jump tables.

Because liveness analysis relies strictly on the statically modeled CFG,
could traversing this unmodeled edge cause liveness to incorrectly assume
registers are dead, allowing an unsound state pruning bypass?

Should this logic verify that the dynamic target is actually present in
env->insn_aux_data[env->insn_idx].jt, rather than only verifying the target
is within subprogram boundaries?

> +			verbose(env, "gotox target %u outside subprog\n", target);
> +			return -EINVAL;
> +		}
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-f01-03-gotox-bpf-next-v3-0-b744432e1361@mails.tsinghua.edu.cn?part=1