BACKPORT: bpf: Fix 32 bit src register truncation on div/mod
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.
Also add some other code missed when backporting.
Fixes: 68fda450a7df ("bpf: fix 32-bit divide by zero")
Co-developed-by: John Fastabend <john.fastabend@gmail.com>
Change-Id: I35a7f4f346bbcbc2f01003e607f2b00b7abe92ae
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>
This commit is contained in:
committed by
Samuel Pascua
parent
2979455ad3
commit
324bf264fc
@@ -1,8 +1,5 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
/* Copyright (c) 2016 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#include <linux/cpumask.h>
|
||||
#include <linux/spinlock.h>
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
/* SPDX-License-Identifier: GPL-2.0-only */
|
||||
/* Copyright (c) 2016 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#ifndef __BPF_LRU_LIST_H_
|
||||
#define __BPF_LRU_LIST_H_
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-or-later
|
||||
/*
|
||||
* Linux Socket Filter - Kernel level socket filtering
|
||||
*
|
||||
@@ -12,11 +13,6 @@
|
||||
* Alexei Starovoitov <ast@plumgrid.com>
|
||||
* Daniel Borkmann <dborkman@redhat.com>
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU General Public License
|
||||
* as published by the Free Software Foundation; either version
|
||||
* 2 of the License, or (at your option) any later version.
|
||||
*
|
||||
* Andi Kleen - Fix a few bad bugs and races.
|
||||
* Kris Katterjohn - Added many additional checks in bpf_check_classic()
|
||||
*/
|
||||
@@ -800,14 +796,6 @@ static void bpf_jit_uncharge_modmem(u32 pages)
|
||||
atomic_long_sub(pages, &bpf_jit_current);
|
||||
}
|
||||
|
||||
#if IS_ENABLED(CONFIG_BPF_JIT) && IS_ENABLED(CONFIG_CFI_CLANG)
|
||||
bool __weak arch_bpf_jit_check_func(const struct bpf_prog *prog)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(arch_bpf_jit_check_func);
|
||||
#endif
|
||||
|
||||
void *__weak bpf_jit_alloc_exec(unsigned long size)
|
||||
{
|
||||
return module_alloc(size);
|
||||
@@ -818,6 +806,14 @@ void __weak bpf_jit_free_exec(void *addr)
|
||||
module_memfree(addr);
|
||||
}
|
||||
|
||||
#if IS_ENABLED(CONFIG_BPF_JIT) && IS_ENABLED(CONFIG_CFI_CLANG)
|
||||
bool __weak arch_bpf_jit_check_func(const struct bpf_prog *prog)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(arch_bpf_jit_check_func);
|
||||
#endif
|
||||
|
||||
struct bpf_binary_header *
|
||||
bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
|
||||
unsigned int alignment,
|
||||
@@ -940,6 +936,9 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
|
||||
* below.
|
||||
*
|
||||
* Constant blinding is only used by JITs, not in the interpreter.
|
||||
* The interpreter uses AX in some occasions as a local temporary
|
||||
* register e.g. in DIV or MOD instructions.
|
||||
*
|
||||
* In restricted circumstances, the verifier can also use the AX
|
||||
* register for rewrites as long as they do not interfere with
|
||||
* the above cases!
|
||||
@@ -1342,7 +1341,6 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
|
||||
#undef BPF_INSN_3_LBL
|
||||
#undef BPF_INSN_2_LBL
|
||||
u32 tail_call_cnt = 0;
|
||||
u64 tmp;
|
||||
|
||||
#define CONT ({ insn++; goto select_insn; })
|
||||
#define CONT_JMP ({ insn++; goto select_insn; })
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
/*
|
||||
* Minimal file system backend for holding eBPF maps and programs,
|
||||
* used by bpf(2) object pinning.
|
||||
@@ -5,10 +6,6 @@
|
||||
* Authors:
|
||||
*
|
||||
* Daniel Borkmann <daniel@iogearbox.net>
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU General Public License
|
||||
* version 2 as published by the Free Software Foundation.
|
||||
*/
|
||||
|
||||
#include <linux/init.h>
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
/* Copyright (c) 2017 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#include <linux/slab.h>
|
||||
#include <linux/bpf.h>
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
/* SPDX-License-Identifier: GPL-2.0-only */
|
||||
/* Copyright (c) 2017 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#ifndef __MAP_IN_MAP_H__
|
||||
#define __MAP_IN_MAP_H__
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
/* Copyright (c) 2016 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#include "percpu_freelist.h"
|
||||
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
/* SPDX-License-Identifier: GPL-2.0-only */
|
||||
/* Copyright (c) 2016 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#ifndef __PERCPU_FREELIST_H__
|
||||
#define __PERCPU_FREELIST_H__
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
/* Copyright (c) 2016 Facebook
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of version 2 of the GNU General Public
|
||||
* License as published by the Free Software Foundation.
|
||||
*/
|
||||
#include <linux/bpf.h>
|
||||
#include <linux/jhash.h>
|
||||
|
||||
@@ -821,7 +821,6 @@ static int map_lookup_elem(union bpf_attr *attr)
|
||||
ptr = map->ops->map_lookup_elem_sys_only(map, key);
|
||||
else
|
||||
ptr = map->ops->map_lookup_elem(map, key);
|
||||
ptr = map->ops->map_lookup_elem(map, key);
|
||||
if (IS_ERR(ptr)) {
|
||||
err = PTR_ERR(ptr);
|
||||
} else if (!ptr) {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
/* tnum: tracked (or tristate) numbers
|
||||
*
|
||||
* A tnum tracks knowledge about the bits of a value. Each bit can be either
|
||||
|
||||
@@ -1973,6 +1973,7 @@ static int check_stack_write(struct bpf_verifier_env *env,
|
||||
} else if (reg && is_spillable_regtype(reg->type)) {
|
||||
/* register containing pointer is being spilled into stack */
|
||||
if (size != BPF_REG_SIZE) {
|
||||
verbose_linfo(env, insn_idx, "; ");
|
||||
verbose(env, "invalid size of register spill\n");
|
||||
return -EACCES;
|
||||
}
|
||||
@@ -2040,6 +2041,7 @@ static int check_stack_read(struct bpf_verifier_env *env,
|
||||
if (stype[0] == STACK_SPILL) {
|
||||
if (size != BPF_REG_SIZE) {
|
||||
if (reg->type != SCALAR_VALUE) {
|
||||
verbose_linfo(env, env->insn_idx, "; ");
|
||||
verbose(env, "invalid size of register fill\n");
|
||||
return -EACCES;
|
||||
}
|
||||
@@ -3022,7 +3024,7 @@ static int __check_stack_boundary(struct bpf_verifier_env *env, u32 regno,
|
||||
int off, int access_size,
|
||||
bool zero_size_allowed)
|
||||
{
|
||||
struct bpf_reg_state *reg = cur_regs(env) + regno;
|
||||
struct bpf_reg_state *reg = reg_state(env, regno);
|
||||
|
||||
if (off >= 0 || off < -MAX_BPF_STACK || off + access_size > 0 ||
|
||||
access_size < 0 || (access_size == 0 && !zero_size_allowed)) {
|
||||
@@ -6150,7 +6152,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
|
||||
|
||||
if (BPF_SRC(insn->code) == BPF_K)
|
||||
pred = is_branch_taken(dst_reg, insn->imm,
|
||||
opcode, is_jmp32);
|
||||
opcode, is_jmp32);
|
||||
else if (src_reg->type == SCALAR_VALUE &&
|
||||
tnum_is_const(src_reg->var_off))
|
||||
pred = is_branch_taken(dst_reg, src_reg->var_off.value,
|
||||
@@ -6477,7 +6479,7 @@ static int check_return_code(struct bpf_verifier_env *env)
|
||||
verbose(env, "has unknown scalar value");
|
||||
}
|
||||
tnum_strn(tn_buf, sizeof(tn_buf), range);
|
||||
verbose(env, " should have been %s\n", tn_buf);
|
||||
verbose(env, " should have been in %s\n", tn_buf);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
@@ -9218,31 +9220,30 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
|
||||
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_MOV_REG(BPF_CLASS(insn->code), BPF_REG_AX, insn->src_reg),
|
||||
bool isdiv = BPF_OP(insn->code) == BPF_DIV;
|
||||
struct bpf_insn *patchlet;
|
||||
struct bpf_insn chk_and_div[] = {
|
||||
/* [R,W]x div 0 -> 0 */
|
||||
BPF_JMP_IMM(BPF_JEQ, BPF_REG_AX, 0, 2),
|
||||
BPF_RAW_REG(*insn, insn->dst_reg, BPF_REG_AX),
|
||||
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),
|
||||
BPF_ALU_REG(BPF_CLASS(insn->code), BPF_XOR, insn->dst_reg, insn->dst_reg),
|
||||
*insn,
|
||||
};
|
||||
struct bpf_insn mask_and_mod[] = {
|
||||
BPF_MOV_REG(BPF_CLASS(insn->code), BPF_REG_AX, insn->src_reg),
|
||||
BPF_JMP_IMM(BPF_JEQ, BPF_REG_AX, 0, 1 + (is64 ? 0 : 1)),
|
||||
BPF_RAW_REG(*insn, insn->dst_reg, BPF_REG_AX),
|
||||
struct bpf_insn chk_and_mod[] = {
|
||||
/* [R,W]x mod 0 -> [R,W]x */
|
||||
BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) |
|
||||
BPF_JEQ | BPF_K, insn->src_reg,
|
||||
0, 1 + (is64 ? 0 : 1), 0),
|
||||
*insn,
|
||||
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
|
||||
BPF_MOV32_REG(insn->dst_reg, insn->dst_reg),
|
||||
};
|
||||
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;
|
||||
cnt = ARRAY_SIZE(mask_and_div);
|
||||
} else {
|
||||
patchlet = mask_and_mod;
|
||||
cnt = ARRAY_SIZE(mask_and_mod) - (is64 ? 2 : 0);
|
||||
}
|
||||
patchlet = isdiv ? chk_and_div : chk_and_mod;
|
||||
cnt = isdiv ? ARRAY_SIZE(chk_and_div) :
|
||||
ARRAY_SIZE(chk_and_mod) - (is64 ? 2 : 0);
|
||||
|
||||
new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt);
|
||||
if (!new_prog)
|
||||
@@ -9596,7 +9597,6 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
|
||||
env->strict_alignment = !!(attr->prog_flags & BPF_F_STRICT_ALIGNMENT);
|
||||
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
|
||||
env->strict_alignment = true;
|
||||
|
||||
if (attr->prog_flags & BPF_F_ANY_ALIGNMENT)
|
||||
env->strict_alignment = false;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user