netdev - Re: [PATCH] bpf, x64: bump the number of passes to 64

lists.openwall.net		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 linux-cve-announce 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]

Message-ID: <8fbcb23d-d140-48fb-819d-61f49672d9bd@gmail.com>
Date:   Thu, 3 Dec 2020 12:20:38 +0100
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Gary Lin <glin@...e.com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        andreas.taschner@...e.com
Subject: Re: [PATCH] bpf, x64: bump the number of passes to 64



On 12/3/20 10:12 AM, Gary Lin wrote:
> The x64 bpf jit expects bpf images converge within the given passes, but
> it could fail to do so with some corner cases. For example:
> 
>       l0:     ldh [4]
>       l1:     jeq #0x537d, l2, l40
>       l2:     ld [0]
>       l3:     jeq #0xfa163e0d, l4, l40
>       l4:     ldh [12]
>       l5:     ldx #0xe
>       l6:     jeq #0x86dd, l41, l7
>       l8:     ld [x+16]
>       l9:     ja 41
> 
>         [... repeated ja 41 ]
> 
>       l40:    ja 41
>       l41:    ret #0
>       l42:    ld #len
>       l43:    ret a
> 
> This bpf program contains 32 "ja 41" instructions which are effectively
> NOPs and designed to be replaced with valid code dynamically. Ideally,
> bpf jit should optimize those "ja 41" instructions out when translating
> the bpf instructions into x86_64 machine code. However, do_jit() can
> only remove one "ja 41" for offset==0 on each pass, so it requires at
> least 32 runs to eliminate those JMPs and exceeds the current limit of
> passes (20). In the end, the program got rejected when BPF_JIT_ALWAYS_ON
> is set even though it's legit as a classic socket filter.
> 
> Since this kind of programs are usually handcrafted rather than
> generated by LLVM, those programs tend to be small. To avoid increasing
> the complexity of BPF JIT, this commit just bumps the number of passes
> to 64 as suggested by Daniel to make it less likely to fail on such cases.
> 

Another idea would be to stop trying to reduce size of generated
code after a given number of passes have been attempted.

Because even a limit of 64 wont ensure all 'valid' programs can be JITed.




> Signed-off-by: Gary Lin <glin@...e.com>
> ---
>  arch/x86/net/bpf_jit_comp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 796506dcfc42..43cc80387548 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2042,7 +2042,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>  	 * may converge on the last pass. In such case do one more
>  	 * pass to emit the final image.
>  	 */
> -	for (pass = 0; pass < 20 || image; pass++) {
> +	for (pass = 0; pass < 64 || image; pass++) {
>  		proglen = do_jit(prog, addrs, image, oldproglen, &ctx);
>  		if (proglen <= 0) {
>  out_image:
>