From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, John Fastabend <john.fastabend@gmail.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Alexei Starovoitov <ast@kernel.org>
Subject: [PATCH 5.4 20/24] bpf: Fix 32 bit src register truncation on div/mod
Date: Thu, 11 Feb 2021 16:02:43 +0100 [thread overview]
Message-ID: <20210211150149.404852871@linuxfoundation.org> (raw)
In-Reply-To: <20210211150148.516371325@linuxfoundation.org>
From: Daniel Borkmann <daniel@iogearbox.net>
commit e88b2c6e5a4d9ce30d75391e4d950da74bb2bd90 upstream.
While reviewing a different fix, John and I noticed an oddity in one of the
BPF program dumps that stood out, for example:
# bpftool p d x i 13
0: (b7) r0 = 808464450
1: (b4) w4 = 808464432
2: (bc) w0 = w0
3: (15) if r0 == 0x0 goto pc+1
4: (9c) w4 %= w0
[...]
In line 2 we noticed that the mov32 would 32 bit truncate the original src
register for the div/mod operation. While for the two operations the dst
register is typically marked unknown e.g. from adjust_scalar_min_max_vals()
the src register is not, and thus verifier keeps tracking original bounds,
simplified:
0: R1=ctx(id=0,off=0,imm=0) R10=fp0
0: (b7) r0 = -1
1: R0_w=invP-1 R1=ctx(id=0,off=0,imm=0) R10=fp0
1: (b7) r1 = -1
2: R0_w=invP-1 R1_w=invP-1 R10=fp0
2: (3c) w0 /= w1
3: R0_w=invP(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1_w=invP-1 R10=fp0
3: (77) r1 >>= 32
4: R0_w=invP(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1_w=invP4294967295 R10=fp0
4: (bf) r0 = r1
5: R0_w=invP4294967295 R1_w=invP4294967295 R10=fp0
5: (95) exit
processed 6 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
Runtime result of r0 at exit is 0 instead of expected -1. Remove the
verifier mov32 src rewrite in div/mod and replace it with a jmp32 test
instead. After the fix, we result in the following code generation when
having dividend r1 and divisor r6:
div, 64 bit: div, 32 bit:
0: (b7) r6 = 8 0: (b7) r6 = 8
1: (b7) r1 = 8 1: (b7) r1 = 8
2: (55) if r6 != 0x0 goto pc+2 2: (56) if w6 != 0x0 goto pc+2
3: (ac) w1 ^= w1 3: (ac) w1 ^= w1
4: (05) goto pc+1 4: (05) goto pc+1
5: (3f) r1 /= r6 5: (3c) w1 /= w6
6: (b7) r0 = 0 6: (b7) r0 = 0
7: (95) exit 7: (95) exit
mod, 64 bit: mod, 32 bit:
0: (b7) r6 = 8 0: (b7) r6 = 8
1: (b7) r1 = 8 1: (b7) r1 = 8
2: (15) if r6 == 0x0 goto pc+1 2: (16) if w6 == 0x0 goto pc+1
3: (9f) r1 %= r6 3: (9c) w1 %= w6
4: (b7) r0 = 0 4: (b7) r0 = 0
5: (95) exit 5: (95) exit
x86 in particular can throw a 'divide error' exception for div
instruction not only for divisor being zero, but also for the case
when the quotient is too large for the designated register. For the
edx:eax and rdx:rax dividend pair it is not an issue in x86 BPF JIT
since we always zero edx (rdx). Hence really the only protection
needed is against divisor being zero.
Fixes: 68fda450a7df ("bpf: fix 32-bit divide by zero")
Co-developed-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
kernel/bpf/verifier.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9002,30 +9002,28 @@ static int fixup_bpf_calls(struct bpf_ve
insn->code == (BPF_ALU | BPF_MOD | BPF_X) ||
insn->code == (BPF_ALU | BPF_DIV | BPF_X)) {
bool is64 = BPF_CLASS(insn->code) == BPF_ALU64;
- struct bpf_insn mask_and_div[] = {
- BPF_MOV32_REG(insn->src_reg, insn->src_reg),
+ bool isdiv = BPF_OP(insn->code) == BPF_DIV;
+ struct bpf_insn *patchlet;
+ struct bpf_insn chk_and_div[] = {
/* Rx div 0 -> 0 */
- BPF_JMP_IMM(BPF_JNE, insn->src_reg, 0, 2),
+ BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
+ BPF_JNE | BPF_K, insn->src_reg,
+ 0, 2, 0),
BPF_ALU32_REG(BPF_XOR, insn->dst_reg, insn->dst_reg),
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
*insn,
};
- struct bpf_insn mask_and_mod[] = {
- BPF_MOV32_REG(insn->src_reg, insn->src_reg),
+ struct bpf_insn chk_and_mod[] = {
/* Rx mod 0 -> Rx */
- BPF_JMP_IMM(BPF_JEQ, insn->src_reg, 0, 1),
+ BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
+ BPF_JEQ | BPF_K, insn->src_reg,
+ 0, 1, 0),
*insn,
};
- struct bpf_insn *patchlet;
- if (insn->code == (BPF_ALU64 | BPF_DIV | BPF_X) ||
- insn->code == (BPF_ALU | BPF_DIV | BPF_X)) {
- patchlet = mask_and_div + (is64 ? 1 : 0);
- cnt = ARRAY_SIZE(mask_and_div) - (is64 ? 1 : 0);
- } else {
- patchlet = mask_and_mod + (is64 ? 1 : 0);
- cnt = ARRAY_SIZE(mask_and_mod) - (is64 ? 1 : 0);
- }
+ patchlet = isdiv ? chk_and_div : chk_and_mod;
+ cnt = isdiv ? ARRAY_SIZE(chk_and_div) :
+ ARRAY_SIZE(chk_and_mod);
new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt);
if (!new_prog)
next prev parent reply other threads:[~2021-02-11 15:54 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-11 15:02 [PATCH 5.4 00/24] 5.4.98-rc1 review Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 01/24] tracing/kprobe: Fix to support kretprobe events on unloaded modules Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 02/24] af_key: relax availability checks for skb size calculation Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 03/24] regulator: core: avoid regulator_resolve_supply() race condition Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 04/24] mac80211: 160MHz with extended NSS BW in CSA Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 05/24] ASoC: Intel: Skylake: Zero snd_ctl_elem_value Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 06/24] chtls: Fix potential resource leak Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 07/24] pNFS/NFSv4: Try to return invalid layout in pnfs_layout_process() Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 08/24] ASoC: ak4458: correct reset polarity Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 09/24] iwlwifi: mvm: skip power command when unbinding vif during CSA Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 10/24] iwlwifi: mvm: take mutex for calling iwl_mvm_get_sync_time() Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 11/24] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 12/24] iwlwifi: pcie: fix context info memory leak Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 13/24] iwlwifi: mvm: invalidate IDs of internal stations at mvm start Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 14/24] iwlwifi: mvm: guard against device removal in reprobe Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 15/24] SUNRPC: Move simple_get_bytes and simple_get_netobj into private header Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 16/24] SUNRPC: Handle 0 length opaque XDR object data properly Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 17/24] i2c: mediatek: Move suspend and resume handling to NOIRQ phase Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 18/24] blk-cgroup: Use cond_resched() when destroy blkgs Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 19/24] regulator: Fix lockdep warning resolving supplies Greg Kroah-Hartman
2021-02-11 15:02 ` Greg Kroah-Hartman [this message]
2021-02-11 15:02 ` [PATCH 5.4 21/24] Fix unsynchronized access to sev members through svm_register_enc_region Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 22/24] squashfs: add more sanity checks in id lookup Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 23/24] squashfs: add more sanity checks in inode lookup Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 24/24] squashfs: add more sanity checks in xattr id lookup Greg Kroah-Hartman
2021-02-12 4:24 ` [PATCH 5.4 00/24] 5.4.98-rc1 review Naresh Kamboju
2021-02-12 15:20 ` Naresh Kamboju
2021-02-12 16:17 ` Shuah Khan
2021-02-12 18:08 ` Guenter Roeck
2021-02-12 19:02 ` Florian Fainelli
2021-02-13 3:17 ` Ross Schmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210211150149.404852871@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.