On Mon, Jul 7, 2025 at 4:45 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> 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.
How would it associate gotox with map (set of IPs) at check_cfg() stage?
llvm needs to help.
