On Wed, 2025-06-18 at 15:08 +0000, Anton Protopopov wrote:
> On 25/06/17 08:22PM, Alexei Starovoitov wrote:
> > On Sun, Jun 15, 2025 at 1:55 AM Anton Protopopov
> > <a.s.protopopov@xxxxxxxxx> wrote:
> > >
> > > The final line generates an indirect jump. The
> > > format of the indirect jump instruction supported by BPF is
> > >
> > > BPF_JMP|BPF_X|BPF_JA, SRC=0, DST=Rx, off=0, imm=fd(M)
> > >
> > > and, obviously, the map M must be the same map which was used to
> > > init the register rX. This patch implements this in the following,
> > > hacky, but so far suitable for all existing use-cases, way. On
> > > encountering a `gotox` instruction libbpf tracks back to the
> > > previous direct load from map and stores this map file descriptor
> > > in the gotox instruction.
> >
> > ...
> >
> > > +/*
> > > + * This one is too dumb, of course. TBD to make it smarter.
> > > + */
> > > +static int find_jt_map_fd(struct bpf_program *prog, int insn_idx)
> > > +{
> > > + struct bpf_insn *insn = &prog->insns[insn_idx];
> > > + __u8 dst_reg = insn->dst_reg;
> > > +
> > > + /* TBD: this function is such smart for now that it even ignores this
> > > + * register. Instead, it should backtrack the load more carefully.
> > > + * (So far even this dumb version works with all selftests.)
> > > + */
> > > + pr_debug("searching for a load instruction which populated dst_reg=r%u\n", dst_reg);
> > > +
> > > + while (--insn >= prog->insns) {
> > > + if (insn->code == (BPF_LD|BPF_DW|BPF_IMM))
> > > + return insn[0].imm;
> > > + }
> > > +
> > > + return -ENOENT;
> > > +}
> > > +
> > > +static int bpf_object__patch_gotox(struct bpf_object *obj, struct bpf_program *prog)
> > > +{
> > > + struct bpf_insn *insn = prog->insns;
> > > + int map_fd;
> > > + int i;
> > > +
> > > + for (i = 0; i < prog->insns_cnt; i++, insn++) {
> > > + if (!insn_is_gotox(insn))
> > > + continue;
> > > +
> > > + if (obj->gen_loader)
> > > + return -EFAULT;
> > > +
> > > + map_fd = find_jt_map_fd(prog, i);
> > > + if (map_fd < 0)
> > > + return map_fd;
> > > +
> > > + insn->imm = map_fd;
> > > + }
> >
> > This is obviously broken and cannot be made smarter in libbpf.
> > It won't be doing data flow analysis.
> >
> > The only option I see is to teach llvm to tag jmp_table in gotox.
> > Probably the simplest way is to add the same relo to gotox insn
> > as for ld_imm64. Then libbpf has a direct way to assign
> > the same map_fd into both ld_imm64 and gotox.
>
> This would be nice.
I did not implement this is a change for jt section + jt symbols.
It can be added, but thinking about it again, are you sure it is
necessary to have map fd in the gotox?
Verifier should be smart enough already to track what map the rX in
the `gotox rX` is a derivative of. It can make use of
bpf_insn_aux_data->map_index to enforce that only one map is used with
a particular gotox instruction.
[...]
