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+7d427828b2ea6e592...@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <dan...@iogearbox.net>
Applied, Thanks
