netdev - Re: [PATCH bpf v2] bpf: don't leave partial mangled prog in jit

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] [day] [month] [year] [list]

Date:   Thu, 12 Jul 2018 14:05:04 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     ast@...com, netdev@...r.kernel.org
Subject: Re: [PATCH bpf v2] bpf: don't leave partial mangled prog in
 jit_subprogs error path

On Thu, Jul 12, 2018 at 09:44:28PM +0200, Daniel Borkmann wrote:
> syzkaller managed to trigger the following bug through fault injection:
> 
>   [...]
>   [  141.043668] verifier bug. No program starts at insn 3
>   [  141.044648] WARNING: CPU: 3 PID: 4072 at kernel/bpf/verifier.c:1613
>                  get_callee_stack_depth kernel/bpf/verifier.c:1612 [inline]
>   [  141.044648] WARNING: CPU: 3 PID: 4072 at kernel/bpf/verifier.c:1613
>                  fixup_call_args kernel/bpf/verifier.c:5587 [inline]
>   [  141.044648] WARNING: CPU: 3 PID: 4072 at kernel/bpf/verifier.c:1613
>                  bpf_check+0x525e/0x5e60 kernel/bpf/verifier.c:5952
>   [  141.047355] CPU: 3 PID: 4072 Comm: a.out Not tainted 4.18.0-rc4+ #51
>   [  141.048446] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),BIOS 1.10.2-1 04/01/2014
>   [  141.049877] Call Trace:
>   [  141.050324]  __dump_stack lib/dump_stack.c:77 [inline]
>   [  141.050324]  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
>   [  141.050950]  ? dump_stack_print_info.cold.2+0x52/0x52 lib/dump_stack.c:60
>   [  141.051837]  panic+0x238/0x4e7 kernel/panic.c:184
>   [  141.052386]  ? add_taint.cold.5+0x16/0x16 kernel/panic.c:385
>   [  141.053101]  ? __warn.cold.8+0x148/0x1ba kernel/panic.c:537
>   [  141.053814]  ? __warn.cold.8+0x117/0x1ba kernel/panic.c:530
>   [  141.054506]  ? get_callee_stack_depth kernel/bpf/verifier.c:1612 [inline]
>   [  141.054506]  ? fixup_call_args kernel/bpf/verifier.c:5587 [inline]
>   [  141.054506]  ? bpf_check+0x525e/0x5e60 kernel/bpf/verifier.c:5952
>   [  141.055163]  __warn.cold.8+0x163/0x1ba kernel/panic.c:538
>   [  141.055820]  ? get_callee_stack_depth kernel/bpf/verifier.c:1612 [inline]
>   [  141.055820]  ? fixup_call_args kernel/bpf/verifier.c:5587 [inline]
>   [  141.055820]  ? bpf_check+0x525e/0x5e60 kernel/bpf/verifier.c:5952
>   [...]
> 
> What happens in jit_subprogs() is that kcalloc() for the subprog func
> buffer is failing with NULL where we then bail out. Latter is a plain
> return -ENOMEM, and this is definitely not okay since earlier in the
> loop we are walking all subprogs and temporarily rewrite insn->off to
> remember the subprog id as well as insn->imm to temporarily point the
> call to __bpf_call_base + 1 for the initial JIT pass. Thus, bailing
> out in such state and handing this over to the interpreter is troublesome
> since later/subsequent e.g. find_subprog() lookups are based on wrong
> insn->imm.
> 
> Therefore, once we hit this point, we need to jump to out_free path
> where we undo all changes from earlier loop, so that interpreter can
> work on unmodified insn->{off,imm}.
> 
> Another point is that should find_subprog() fail in jit_subprogs() due
> to a verifier bug, then we also should not simply defer the program to
> the interpreter since also here we did partial modifications. Instead
> we should just bail out entirely and return an error to the user who is
> trying to load the program.
> 
> Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs")
> Reported-by: syzbot+7d427828b2ea6e592804@...kaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>

Applied, Thanks