netdev - Re: [PATCH bpf] bpf: fix truncated jump targets on heavy expansions

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: <20180517212038.glmierrm6rpub5di@kafai-mbp.dhcp.thefacebook.com>
Date:   Thu, 17 May 2018 14:20:40 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Daniel Borkmann <daniel@...earbox.net>
CC:     <alexei.starovoitov@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf] bpf: fix truncated jump targets on heavy expansions

On Thu, May 17, 2018 at 01:44:11AM +0200, Daniel Borkmann wrote:
> Recently during testing, I ran into the following panic:
> 
>   [  207.892422] Internal error: Accessing user space memory outside uaccess.h routines: 96000004 [#1] SMP
>   [  207.901637] Modules linked in: binfmt_misc [...]
>   [  207.966530] CPU: 45 PID: 2256 Comm: test_verifier Tainted: G        W         4.17.0-rc3+ #7
>   [  207.974956] Hardware name: FOXCONN R2-1221R-A4/C2U4N_MB, BIOS G31FB18A 03/31/2017
>   [  207.982428] pstate: 60400005 (nZCv daif +PAN -UAO)
>   [  207.987214] pc : bpf_skb_load_helper_8_no_cache+0x34/0xc0
>   [  207.992603] lr : 0xffff000000bdb754
>   [  207.996080] sp : ffff000013703ca0
>   [  207.999384] x29: ffff000013703ca0 x28: 0000000000000001
>   [  208.004688] x27: 0000000000000001 x26: 0000000000000000
>   [  208.009992] x25: ffff000013703ce0 x24: ffff800fb4afcb00
>   [  208.015295] x23: ffff00007d2f5038 x22: ffff00007d2f5000
>   [  208.020599] x21: fffffffffeff2a6f x20: 000000000000000a
>   [  208.025903] x19: ffff000009578000 x18: 0000000000000a03
>   [  208.031206] x17: 0000000000000000 x16: 0000000000000000
>   [  208.036510] x15: 0000ffff9de83000 x14: 0000000000000000
>   [  208.041813] x13: 0000000000000000 x12: 0000000000000000
>   [  208.047116] x11: 0000000000000001 x10: ffff0000089e7f18
>   [  208.052419] x9 : fffffffffeff2a6f x8 : 0000000000000000
>   [  208.057723] x7 : 000000000000000a x6 : 00280c6160000000
>   [  208.063026] x5 : 0000000000000018 x4 : 0000000000007db6
>   [  208.068329] x3 : 000000000008647a x2 : 19868179b1484500
>   [  208.073632] x1 : 0000000000000000 x0 : ffff000009578c08
>   [  208.078938] Process test_verifier (pid: 2256, stack limit = 0x0000000049ca7974)
>   [  208.086235] Call trace:
>   [  208.088672]  bpf_skb_load_helper_8_no_cache+0x34/0xc0
>   [  208.093713]  0xffff000000bdb754
>   [  208.096845]  bpf_test_run+0x78/0xf8
>   [  208.100324]  bpf_prog_test_run_skb+0x148/0x230
>   [  208.104758]  sys_bpf+0x314/0x1198
>   [  208.108064]  el0_svc_naked+0x30/0x34
>   [  208.111632] Code: 91302260 f9400001 f9001fa1 d2800001 (29500680)
>   [  208.117717] ---[ end trace 263cb8a59b5bf29f ]---
> 
> The program itself which caused this had a long jump over the whole
> instruction sequence where all of the inner instructions required
> heavy expansions into multiple BPF instructions. Additionally, I also
> had BPF hardening enabled which requires once more rewrites of all
> constant values in order to blind them. Each time we rewrite insns,
> bpf_adj_branches() would need to potentially adjust branch targets
> which cross the patchlet boundary to accommodate for the additional
> delta. Eventually that lead to the case where the target offset could
> not fit into insn->off's upper 0x7fff limit anymore where then offset
> wraps around becoming negative (in s16 universe), or vice versa
> depending on the jump direction.
> 
> Therefore it becomes necessary to detect and reject any such occasions
> in a generic way for native eBPF and cBPF to eBPF migrations. For
> the latter we can simply check bounds in the bpf_convert_filter()'s
> BPF_EMIT_JMP helper macro and bail out once we surpass limits. The
> bpf_patch_insn_single() for native eBPF (and cBPF to eBPF in case
> of subsequent hardening) is a bit more complex in that we need to
> detect such truncations before hitting the bpf_prog_realloc(). Thus
> the latter is split into an extra pass to probe problematic offsets
> on the original program in order to fail early. With that in place
> and carefully tested I no longer hit the panic and the rewrites are
> rejected properly. The above example panic I've seen on bpf-next,
> though the issue itself is generic in that a guard against this issue
> in bpf seems more appropriate in this case.
> 
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
Acked-by: Martin KaFai Lau <kafai@...com>