> -----Original Messages-----
> From: "Anton Protopopov" <[email protected]>
> Send time:Tuesday, 09/06/2026 23:56:23
> To: "Nuoqi Gui" <[email protected]>
> Cc: [email protected], "Alexei Starovoitov" <[email protected]>, "Daniel
> Borkmann" <[email protected]>, "Andrii Nakryiko" <[email protected]>,
> "Eduard Zingerman" <[email protected]>, "Shuah Khan" <[email protected]>,
> [email protected], [email protected]
> Subject: Re: [PATCH bpf-next 1/2] bpf: Fix gotox target validation against CFG
>
> On 26/06/09 11:03PM, Nuoqi Gui wrote:
> > CFG construction records the modeled gotox target set in
> > insn_aux_data->jt. It includes INSN_ARRAY maps based on whether the map
> > target is in the current subprog. check_indirect_jump() later validates and
> > follows the current PTR_TO_INSN register's actual INSN_ARRAY map. The
> > verifier does not check that targets copied from that map match the targets
> > that CFG construction modeled for this gotox instruction.
> >
> > This lets one gotox instruction observe two different INSN_ARRAY maps. CFG
> > can select a map whose target is in the current subprog. Another path to
> > the same gotox can carry a PTR_TO_INSN value from a map whose target points
> > at a different subprog. The verifier then accepts an edge absent from the
> > CFG.
> >
> > On x86, gotox becomes a raw indirect jump in the JIT image. Accepting a
> > target not modeled by CFG can enter another subprog without a matching BPF
> > call frame and crash when executed. Validation observed a GPF in
> > bpf_test_run().
> >
> > Fix this by requiring every target copied from the actual PTR_TO_INSN map
> > to be present in the CFG jump table built for the current gotox
> > instruction.
> > Reject the program before pushing verifier states for any unmodeled target.
> >
> > Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps")
> > Signed-off-by: Nuoqi Gui <[email protected]>
> > ---
> > kernel/bpf/verifier.c | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index ed7ba0e6a9ce..25fa90e731e3 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -17124,6 +17124,23 @@ static int indirect_jump_min_max_index(struct
> > bpf_verifier_env *env,
> > return 0;
> > }
> >
> > +static bool is_cfg_indirect_jump_target(struct bpf_verifier_env *env,
> > + u32 target)
> > +{
> > + struct bpf_iarray *jt = env->insn_aux_data[env->insn_idx].jt;
> > + int i;
> > +
> > + if (!jt)
> > + return false;
> > +
> > + for (i = 0; i < jt->cnt; i++) {
> > + if (jt->items[i] == target)
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > /* gotox *dst_reg */
> > static int check_indirect_jump(struct bpf_verifier_env *env, struct
> > bpf_insn *insn)
> > {
> > @@ -17171,6 +17188,15 @@ static int check_indirect_jump(struct
> > bpf_verifier_env *env, struct bpf_insn *in
> > return -EINVAL;
> > }
> >
> > + for (i = 0; i < n; i++) {
> > + if (!is_cfg_indirect_jump_target(env,
> > env->gotox_tmp_buf->items[i])) {
> > + verbose(env,
> > + "gotox target %u from map id=%d is not in the
> > CFG jump table\n",
> > + env->gotox_tmp_buf->items[i], map->id);
> > + return -EINVAL;
> > + }
> > + }
>
> Thanks for reporting the bug.
>
> As for the fix, would it make more sense to either record maps in
> check_cfg or to re-check subfunc boundaries here?
>
> > 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
> >
Hi Anton, Eduard,
Thanks for the review.
I agree that checking the current subprog bounds in check_indirect_jump()
is the better fix. I will rework v2 to validate the runtime INSN_ARRAY
targets against the subprog containing the gotox instruction, instead of
doing CFG jump-table membership checks.
I will also fix the selftest expected errno.
