Re: [PATCH v8 bpf-next 01/11] bpf, x86: add new map type: instructions array

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

On 25/10/31 08:04AM, Alexei Starovoitov wrote:
> On Fri, Oct 31, 2025 at 12:16 AM Anton Protopopov
> <a.s.protopopov@gmail.com> wrote:
> >
> > On 25/10/30 03:50PM, Alexei Starovoitov wrote:
> > > On Tue, Oct 28, 2025 at 7:15 AM Anton Protopopov
> > > <a.s.protopopov@gmail.com> wrote:
> > > >                 }
> > > > +
> > > > +               bpf_prog_update_insn_ptrs(prog, addrs, image);
> > > > +
> > >
> > > I suspect there is off-by-1 bug somewhere in bpf_prog_update_insn_ptrs() math,
> > > since addrs[0] points to function prologue.
> > > addrs[1] is the offset of the first bpf insn.
> > > See how it's called in other place:
> > >    bpf_prog_fill_jited_linfo(prog, addrs + 1);
> >
> > So all the maps I have in all selftests tests point to a wrong ip for
> > every goto, and still work?
> >
> > In fact, addrs[0] points to right after the prologue, see how it is
> > initialized in do_jit:
> >
> >   addrs[0] = proglen;
> >
> > here proglen is the length of prologue. The loop in do_jit startrs
> > with i=1, but all addrs are referenced with i-1:
> >
> >     case BPF_JMP | BPF_CALL: {
> >             u8 *ip = image + addrs[i - 1];
> >
> > The bpf_prog_fill_jited_linfo internally also does the -1 thingy:
> >
> >     insn_to_jit_off[linfo[i].insn_off - insn_start - 1];
> 
> I added that '+ 1' in
> commit 7c2e988f400e ("bpf: fix x64 JIT code generation for jmp to 1st insn")
> 
> Please add a comment to explain what addrs[] are and
> that bpf_prog_update_insn_ptrs() treats this array as
> offsets to 1st byte, while bpf_prog_fill_jited_linfo() is offset to last
> for historical reasons.

Ok, now this makes sense. Thanks!