lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 23 Jan 2022 01:03:25 +0000
From:   Song Liu <>
To:     Alexei Starovoitov <>
CC:     Ilya Leoshkevich <>, Song Liu <>,
        bpf <>,
        Network Development <>,
        LKML <>,
        Alexei Starovoitov <>,
        "Daniel Borkmann" <>,
        Andrii Nakryiko <>,
        "Kernel Team" <>,
        Peter Zijlstra <>, X86 ML <>
Subject: Re: [PATCH v6 bpf-next 6/7] bpf: introduce bpf_prog_pack allocator

> On Jan 21, 2022, at 6:12 PM, Alexei Starovoitov <> wrote:
> On Fri, Jan 21, 2022 at 5:30 PM Song Liu <> wrote:
>>> On Jan 21, 2022, at 5:12 PM, Alexei Starovoitov <> wrote:
>>> On Fri, Jan 21, 2022 at 5:01 PM Song Liu <> wrote:
>>>> In this way, we need to allocate rw_image here, and free it in
>>>> bpf_jit_comp.c. This feels a little weird to me, but I guess that
>>>> is still the cleanest solution for now.
>>> You mean inside bpf_jit_binary_alloc?
>>> That won't be arch independent.
>>> It needs to be split into generic piece that stays in core.c
>>> and callbacks like bpf_jit_fill_hole_t
>>> or into multiple helpers with prep in-between.
>>> Don't worry if all archs need to be touched.
>> How about we introduce callback bpf_jit_set_header_size_t? Then we
>> can split x86's jit_fill_hole() into two functions, one to fill the
>> hole, the other to set size. The rest of the logic gonna stay the same.
>> Archs that do not use bpf_prog_pack won't need bpf_jit_set_header_size_t.
> That's not any better.
> Currently the choice of bpf_jit_binary_alloc_pack vs bpf_jit_binary_alloc
> leaks into arch bits and bpf_prog_pack_max_size() doesn't
> really make it generic.
> Ideally all archs continue to use bpf_jit_binary_alloc()
> and magic happens in a generic code.
> If not then please remove bpf_prog_pack_max_size(),
> since it doesn't provide much value and pick
> bpf_jit_binary_alloc_pack() signature to fit x86 jit better.
> It wouldn't need bpf_jit_fill_hole_t callback at all.
> Please think it through so we don't need to redesign it
> when another arch will decide to use huge pages for bpf progs.
> cc-ing Ilya for ideas on how that would fit s390.

I guess we have a few different questions here:

1. Can we use bpf_jit_binary_alloc() for both regular page and shared 
huge page? 

I think the answer is no, as bpf_jit_binary_alloc() allocates a rw 
buffer, and arch calls bpf_jit_binary_lock_ro after JITing. The new 
allocator will return a slice of a shared huge page, which is locked
RO before JITing. 

2. The problem with bpf_prog_pack_max_size() limitation. 

I think this is the worst part of current version of bpf_prog_pack, 
but it shouldn't be too hard to fix. I will remove this limitation 
in the next version. 

3. How to set proper header->size? 

I guess we can introduce something similar to bpf_arch_text_poke() 
for this? 

My proposal for the next version is:
1. No changes to archs that do not use huge page, just keep using 

2. For x86_64 (and other arch that would support bpf program on huge
   2.1 arch/bpf_jit_comp calls bpf_jit_binary_alloc_pack() to allocate
       an RO bpf_binary_header;
   2.2 arch allocates a temporary buffer for JIT. Once JIT is done, 
       use text_poke_copy to copy the code to the RO bpf_binary_header. 

3. Remove bpf_prog_pack_max_size limitation. 

Does this sound reasonable?


Powered by blists - more mailing lists