On 26/06/28 09:59PM, Nuoqi Gui wrote:
> During CFG construction, the verifier records the modeled gotox target set
> in insn_aux_data->jt. Later, check_indirect_jump() follows targets from
> the runtime PTR_TO_INSN register's actual INSN_ARRAY map.
>
> This lets one gotox instruction observe different INSN_ARRAY maps on
> different paths and accept a target outside the calling subprog. The
> observed x86 JIT case can then enter another subprog without a matching
> BPF call frame and crash when executed.
>
> Reject every target copied from the actual PTR_TO_INSN map if it is
> outside the calling subprog.
>
> Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps")
> Signed-off-by: Nuoqi Gui <[email protected]>
> ---
> kernel/bpf/verifier.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index eb46a81a8c51..05a996a5ecdd 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17145,9 +17145,11 @@ static int indirect_jump_min_max_index(struct
> bpf_verifier_env *env,
> static int check_indirect_jump(struct bpf_verifier_env *env, struct bpf_insn
> *insn)
> {
> struct bpf_verifier_state *other_branch;
> + struct bpf_subprog_info *subprog;
> struct bpf_reg_state *dst_reg;
> struct bpf_map *map;
> u32 min_index, max_index;
> + int subprog_start, subprog_end;
> int err = 0;
> int n;
> int i;
> @@ -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))
Can this actually happen?
> + 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) {
> + verbose(env, "gotox target %u outside subprog\n",
> target);
In the previous patch there was more info printed (at least, subprog
boundaries looked ok, not 100% sure about map id).
> + return -EINVAL;
> + }
> + }
> +
> for (i = 0; i < n - 1; i++) {
> mark_indirect_target(env, env->gotox_tmp_buf->items[i]);
> other_branch = push_stack(env, env->gotox_tmp_buf->items[i],
>
> --
> 2.34.1
>
