| From 3468ba2c0b102685eb7266a0259a291f0bfe439c Mon Sep 17 00:00:00 2001 |
| From: Luke Nelson <lukenels@cs.washington.edu> |
| Date: Thu, 5 Mar 2020 15:44:12 -0800 |
| Subject: [PATCH] bpf, x32: Fix bug with JMP32 JSET BPF_X checking upper bits |
| |
| commit 80f1f85036355e5581ec0b99913410345ad3491b upstream. |
| |
| The current x32 BPF JIT is incorrect for JMP32 JSET BPF_X when the upper |
| 32 bits of operand registers are non-zero in certain situations. |
| |
| The problem is in the following code: |
| |
| case BPF_JMP | BPF_JSET | BPF_X: |
| case BPF_JMP32 | BPF_JSET | BPF_X: |
| ... |
| |
| /* and dreg_lo,sreg_lo */ |
| EMIT2(0x23, add_2reg(0xC0, sreg_lo, dreg_lo)); |
| /* and dreg_hi,sreg_hi */ |
| EMIT2(0x23, add_2reg(0xC0, sreg_hi, dreg_hi)); |
| /* or dreg_lo,dreg_hi */ |
| EMIT2(0x09, add_2reg(0xC0, dreg_lo, dreg_hi)); |
| |
| This code checks the upper bits of the operand registers regardless if |
| the BPF instruction is BPF_JMP32 or BPF_JMP64. Registers dreg_hi and |
| dreg_lo are not loaded from the stack for BPF_JMP32, however, they can |
| still be polluted with values from previous instructions. |
| |
| The following BPF program demonstrates the bug. The jset64 instruction |
| loads the temporary registers and performs the jump, since ((u64)r7 & |
| (u64)r8) is non-zero. The jset32 should _not_ be taken, as the lower |
| 32 bits are all zero, however, the current JIT will take the branch due |
| the pollution of temporary registers from the earlier jset64. |
| |
| mov64 r0, 0 |
| ld64 r7, 0x8000000000000000 |
| ld64 r8, 0x8000000000000000 |
| jset64 r7, r8, 1 |
| exit |
| jset32 r7, r8, 1 |
| mov64 r0, 2 |
| exit |
| |
| The expected return value of this program is 2; under the buggy x32 JIT |
| it returns 0. The fix is to skip using the upper 32 bits for jset32 and |
| compare the upper 32 bits for jset64 only. |
| |
| All tests in test_bpf.ko and selftests/bpf/test_verifier continue to |
| pass with this change. |
| |
| We found this bug using our automated verification tool, Serval. |
| |
| Fixes: 69f827eb6e14 ("x32: bpf: implement jitting of JMP32") |
| Co-developed-by: Xi Wang <xi.wang@gmail.com> |
| Signed-off-by: Xi Wang <xi.wang@gmail.com> |
| Signed-off-by: Luke Nelson <luke.r.nels@gmail.com> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Link: https://lore.kernel.org/bpf/20200305234416.31597-1-luke.r.nels@gmail.com |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c |
| index b29e82f190c7..8369f976483f 100644 |
| --- a/arch/x86/net/bpf_jit_comp32.c |
| +++ b/arch/x86/net/bpf_jit_comp32.c |
| @@ -2241,10 +2241,12 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, |
| } |
| /* and dreg_lo,sreg_lo */ |
| EMIT2(0x23, add_2reg(0xC0, sreg_lo, dreg_lo)); |
| - /* and dreg_hi,sreg_hi */ |
| - EMIT2(0x23, add_2reg(0xC0, sreg_hi, dreg_hi)); |
| - /* or dreg_lo,dreg_hi */ |
| - EMIT2(0x09, add_2reg(0xC0, dreg_lo, dreg_hi)); |
| + if (is_jmp64) { |
| + /* and dreg_hi,sreg_hi */ |
| + EMIT2(0x23, add_2reg(0xC0, sreg_hi, dreg_hi)); |
| + /* or dreg_lo,dreg_hi */ |
| + EMIT2(0x09, add_2reg(0xC0, dreg_lo, dreg_hi)); |
| + } |
| goto emit_cond_jmp; |
| } |
| case BPF_JMP | BPF_JSET | BPF_K: |
| -- |
| 2.7.4 |
| |