Re: [PATCH bpf-next] bpf: arm64: Fix panic due to missing BTI at indirect jump targets

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/12/04 02:47PM, Xu Kuohai wrote:
> On 11/28/2025 12:46 AM, Anton Protopopov wrote:
> 
> [...]
> 
> > > > diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
> > > > index 61ce52882632..ed20b186a1f5 100644
> > > > --- a/kernel/bpf/bpf_insn_array.c
> > > > +++ b/kernel/bpf/bpf_insn_array.c
> > > > @@ -299,3 +299,46 @@ void bpf_prog_update_insn_ptrs(struct bpf_prog *prog, u32 *offsets, void *image)
> > > >   		}
> > > >   	}
> > > >   }
> > > > +
> > > > +bool bpf_prog_has_insn_array(const struct bpf_prog *prog)
> > > > +{
> > > > +	int i;
> > > > +
> > > > +	for (i = 0; i < prog->aux->used_map_cnt; i++) {
> > > > +		if (is_insn_array(prog->aux->used_maps[i]))
> > > > +			return true;
> > > > +	}
> > > > +	return false;
> > > > +}
> > > 
> > > I think a different check is needed here (and a different function
> > > name, smth like "bpf_prog_has_indirect_jumps"), and a different
> > > algorithm to collect jump targets in the chunk below. A program can
> > > have instruction arrays not related to indirect jumps (see, e.g.,
> > > bpf_insn_array selftests + in future insns arrays will be used to
> > > also support other functionality). As an extreme case, an insn array
> > > can point to every instruction in a prog, thus a BTI will be
> > > generated for every instruction.
> > > 
> > > In verifier it is used a bit differently, namely, all insn arrays for
> > > a given subprog are collected when an indirect jump is encountered
> > > (and non-deterministic only in check_cfg). Later in verification, an
> > > exact map is used, so this is not a problem.
> > > 
> > > Initially I wanted to have a map flag (in map_extra) to distingiush between
> > > different types of instruction arrays ("plane ones", "jump targets",
> > > "call targets", "static keys"), but Andrii wasn't happy with it,
> > > so eventually I've dropped it. Maybe it is worth adding it until
> > > the code is merged to upstream? Eduard, Alexei, wdyt?
> > 
> > Actually, this is even better to mark a map as containing indirect
> > jump targets in check_indirect_jump(). This will be the most precise
> > set of targets, and won't require any userspace-visible changes/flags.
> > 
> > Something like this:
> > 
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 6498be4c44f8..c2d708213330 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -292,6 +292,10 @@ struct bpf_map_owner {
> >   	enum bpf_attach_type expected_attach_type;
> >   };
> > +/* map_subtype values for map_type BPF_MAP_TYPE_INSN_ARRAY */
> > +#define BPF_INSN_ARRAY_VOID		0
> > +#define BPF_INSN_ARRAY_JUMP_TABLE	1
> > +
> >   struct bpf_map {
> >   	u8 sha[SHA256_DIGEST_SIZE];
> >   	const struct bpf_map_ops *ops;
> > @@ -331,6 +335,7 @@ struct bpf_map {
> >   	bool frozen; /* write-once; write-protected by freeze_mutex */
> >   	bool free_after_mult_rcu_gp;
> >   	bool free_after_rcu_gp;
> > +	u32 map_subtype; /* defined per map type */
> >   	atomic64_t sleepable_refcnt;
> >   	s64 __percpu *elem_count;
> >   	u64 cookie; /* write-once */
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 766695491bc5..60bbd32e793a 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -20293,6 +20293,12 @@ static int check_indirect_jump(struct bpf_verifier_env *env, struct bpf_insn *in
> >   		return -EINVAL;
> >   	}
> > +	/*
> > +	 * Explicitly mark this map as a jump table such that it can be
> > +	 * distinguished later from other instruction arrays
> > +	 */
> > +	map->map_subtype = BPF_INSN_ARRAY_JUMP_TABLE;
> > +
> >   	for (i = 0; i < n - 1; i++) {
> >   		other_branch = push_stack(env, env->gotox_tmp_buf->items[i],
> >   					  env->insn_idx, env->cur_state->speculative);
> > 
> 
> Looks good to me. Would you like to submit it as a separate patch, or
> shall I include it in my patch with your SOB?

Ok if you just include it, makes sense as part of your changes.