Re: [GIT PULL] Networking for v6.10

[Linus Torvalds]

On Tue, 14 May 2024 at 16:12, Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> Full disclosure I hit a KASAN OOB read warning in BPF when testing
> on Meta's production servers (which load a lot of BPF).
> BPF folks aren't super alarmed by it, and also they are partying at
> LSFMM so I don't think it's worth waiting for the fix.
> But you may feel differently...  https://pastebin.com/0fzqy3cW

Hmm. As long as people are aware of it, I don't think a known issue
needs to hold up any pull request.

Even if that whole 'struct bpf_map can be embedded in many different
structures", combined with "users just magically know which structure
it is and use container_of()" looks like a horrid pattern.

Why does it do that disgusting

        struct bpf_array *array = container_of(map, struct bpf_array, map);
        ...
                *insn++ = BPF_ALU32_IMM(BPF_AND, BPF_REG_0, array->index_mask);

thing? As far as I can tell, a bpf map can be embedded in many
different structures, not just that 'bpf_array' thing.

That spectre-v1 code generation is disgusting. But worse, it's stupid.
The way to turn the index into a data dependency isn't to just 'and'
it with some fixed mask (that is wrong anyway and requires that whole
"round up to the next power-of-two), it's to just teach the JIT to
generate the proper Spectre-v1 sequence.

So that code should be able to rely purely on map->max_entries, and
not do that disgusting "look up struct 'bpf_array'"

Anyway, I've pulled it - the bpf code looks broken, but it looks
fairly straightforward to do it right if I understood that code
correctly.

              Linus