Re: bpf_jit_limit close shave

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, Sep 21, 2021 at 4:50 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Hi,
>
> We just had a close shave with bpf_jit_limit. Something on our edge
> caused us to cross the default limit, which made seccomp and xt_bpf
> filters fail to load. Looking at the source made me realise that we
> narrowly avoided taking out our load balancer, which would've been
> pretty bad. We still run the LB with CAP_SYS_ADMIN instead of narrower
> CAP_BPF, CAP_NET_ADMIN. If we had migrated to the lesser capability
> set we would've been prevented from loading new eBPF:
>
> int bpf_jit_charge_modmem(u32 pages)
> {
>     if (atomic_long_add_return(pages, &bpf_jit_current) >
>         (bpf_jit_limit >> PAGE_SHIFT)) {
>         if (!capable(CAP_SYS_ADMIN)) {
>             atomic_long_sub(pages, &bpf_jit_current);
>             return -EPERM;
>         }
>     }
>
>     return 0;
> }
>
> Does it make sense to include !capable(CAP_BPF) in the check?

Good point. Makes sense to add CAP_BPF there.
Taking down critical networking infrastructure because of this limit
that supposed to apply to unpriv users only is scary indeed.

> This limit reminds me a bit of the memlock issue, where a global limit
> causes coupling between independent systems / processes. Can we remove
> the limit in favour of something more fine grained?

Right. Unfortunately memcg doesn't distinguish kernel module
memory vs any other memory. All types of memory are memory.
Regardless of whether its type is per-cpu, bpf map memory, bpf jit memory, etc.
That's the main reason for the independent knob for JITed memory.
Since it's a bit special. It's a crude knob. Certainly not perfect.