All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Protopopov <a.s.protopopov@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Jiyong Yang <ksur673@gmail.com>,
	Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
Subject: Re: [PATCH v2 bpf-next 1/2] bpf: Do not ignore offsets for loads from insn_arrays
Date: Fri, 3 Apr 2026 07:56:30 +0000	[thread overview]
Message-ID: <ac9yriPJX9RXa5tr@mail.gmail.com> (raw)
In-Reply-To: <CAADnVQJ_e2fyODknJyMPRw4Z7ztu3-0ZrVM8jXpoO1g7bH52jA@mail.gmail.com>

On 26/04/02 02:32PM, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 1:44 PM Anton Protopopov
> <a.s.protopopov@gmail.com> wrote:
> >
> > On 26/04/02 12:00PM, Alexei Starovoitov wrote:
> > > On Thu, Apr 2, 2026 at 11:38 AM Anton Protopopov
> > > <a.s.protopopov@gmail.com> wrote:
> > > >
> > > > When a pointer to PTR_TO_INSN is dereferenced it is possible to
> > > > specify an offset inside the load instruction. This is a bug,
> > > > because while the verifier ignores the field, JITs are not.
> > > > So, patch the verifier to not ignore this field.
> > >
> > > This is too terse to understand.
> > > In 2nd patch you're saying:
> > >
> > >    r1 = &map + offset1
> > >    r1 += offset2
> > >    r1 = *(r1 + offset3)
> > >    gotox r1
> > >
> > > Here offset3 is, normally, equal to zero; but this is not guaranteed.
> > >
> > > What is this 'offset3'? Where did it come from?
> >
> > The offset3 is the .off field of the BPF_LDX_MEM instruction.
> > The BPF assembler will correctly work with non-zero offsets:
> >
> >     r1 = &map;
> >     r1 = *(r1 + offset)
> >
> > > What do you mean JIT handles it?
> >
> > JIT will issue a memory load with this offset,
> > but verifier will ignore it (before this patch).
> >
> > > can llvm ever generate such code?
> > > if not, reject it early ?
> >
> > LLVM can generate code with non-zero offset in BPF_LDX_MEM,
> > say, if one jumps to a structure field, like in `goto *p->j`
> 
> sorry, I still don't get it. What kind of syntax is that?
> Could you share the godbolt link where llvm actually generates
> such code? If the verifier will reject it anyway it's fine.
> I just want to make sure we're fixing real problem.

I am not aware of any real-life code, only can construct
some artificial examples:

        SEC("syscall")
        int test(unsigned int n)
        {
                struct {
                        int i;
                        void *j[3];
                } x = {
                        .i = n,
                        .j = { &&l1, &&l2, &&l3 },
                };

                if (n < 3)
                        goto *x.j[n];

                return 0;
        l1:
                return 1;
        l2:
                return 3;
        l3:
                return 5;
        }

It will compile into

        <test>:
        ;               .j = { &&l1, &&l2, &&l3 },
             160:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0x0 ll
                        0000000000000500:  R_BPF_64_64  BPF.JT.4.0
             162:       79 23 10 00 00 00 00 00 r3 = *(u64 *)(r2 + 0x10)
                                                                   ^ offet != 0
             163:       7b 3a f8 ff 00 00 00 00 *(u64 *)(r10 - 0x8) = r3
             164:       79 23 08 00 00 00 00 00 r3 = *(u64 *)(r2 + 0x8)
             165:       7b 3a f0 ff 00 00 00 00 *(u64 *)(r10 - 0x10) = r3
             ...

  reply	other threads:[~2026-04-03  7:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-02 18:46 [PATCH v2 bpf-next 0/2] Properly load values from insn_arays with non-zero offsets Anton Protopopov
2026-04-02 18:46 ` [PATCH v2 bpf-next 1/2] bpf: Do not ignore offsets for loads from insn_arrays Anton Protopopov
2026-04-02 19:00   ` Alexei Starovoitov
2026-04-02 20:53     ` Anton Protopopov
2026-04-02 21:32       ` Alexei Starovoitov
2026-04-03  7:56         ` Anton Protopopov [this message]
2026-04-03 15:10           ` Alexei Starovoitov
2026-04-03 18:10             ` Anton Protopopov
2026-04-03 18:22               ` Alexei Starovoitov
2026-04-05 18:24                 ` Anton Protopopov
2026-04-02 18:46 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Add more tests for loading insn arrays with offsets Anton Protopopov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac9yriPJX9RXa5tr@mail.gmail.com \
    --to=a.s.protopopov@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ksur673@gmail.com \
    --cc=memxor@gmail.com \
    --cc=mykyta.yatsenko5@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.