120 lines
4.1 KiB
Diff
120 lines
4.1 KiB
Diff
From: Jann Horn <jannh@google.com>
|
|
Date: Mon, 18 Dec 2017 20:11:55 -0800
|
|
Subject: [3/9] bpf: fix incorrect tracking of register size truncation
|
|
Origin: https://git.kernel.org/linus/0c17d1d2c61936401f4702e1846e2c19b200f958
|
|
|
|
Properly handle register truncation to a smaller size.
|
|
|
|
The old code first mirrors the clearing of the high 32 bits in the bitwise
|
|
tristate representation, which is correct. But then, it computes the new
|
|
arithmetic bounds as the intersection between the old arithmetic bounds and
|
|
the bounds resulting from the bitwise tristate representation. Therefore,
|
|
when coerce_reg_to_32() is called on a number with bounds
|
|
[0xffff'fff8, 0x1'0000'0007], the verifier computes
|
|
[0xffff'fff8, 0xffff'ffff] as bounds of the truncated number.
|
|
This is incorrect: The truncated number could also be in the range [0, 7],
|
|
and no meaningful arithmetic bounds can be computed in that case apart from
|
|
the obvious [0, 0xffff'ffff].
|
|
|
|
Starting with v4.14, this is exploitable by unprivileged users as long as
|
|
the unprivileged_bpf_disabled sysctl isn't set.
|
|
|
|
Debian assigned CVE-2017-16996 for this issue.
|
|
|
|
v2:
|
|
- flip the mask during arithmetic bounds calculation (Ben Hutchings)
|
|
v3:
|
|
- add CVE number (Ben Hutchings)
|
|
|
|
Fixes: b03c9f9fdc37 ("bpf/verifier: track signed and unsigned min/max values")
|
|
Signed-off-by: Jann Horn <jannh@google.com>
|
|
Acked-by: Edward Cree <ecree@solarflare.com>
|
|
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
|
|
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
|
|
[bwh: Backported to 4.14]
|
|
---
|
|
kernel/bpf/verifier.c | 44 +++++++++++++++++++++++++++-----------------
|
|
1 file changed, 27 insertions(+), 17 deletions(-)
|
|
|
|
--- a/kernel/bpf/verifier.c
|
|
+++ b/kernel/bpf/verifier.c
|
|
@@ -1079,6 +1079,29 @@ static int check_ptr_alignment(struct bp
|
|
strict);
|
|
}
|
|
|
|
+/* truncate register to smaller size (in bytes)
|
|
+ * must be called with size < BPF_REG_SIZE
|
|
+ */
|
|
+static void coerce_reg_to_size(struct bpf_reg_state *reg, int size)
|
|
+{
|
|
+ u64 mask;
|
|
+
|
|
+ /* clear high bits in bit representation */
|
|
+ reg->var_off = tnum_cast(reg->var_off, size);
|
|
+
|
|
+ /* fix arithmetic bounds */
|
|
+ mask = ((u64)1 << (size * 8)) - 1;
|
|
+ if ((reg->umin_value & ~mask) == (reg->umax_value & ~mask)) {
|
|
+ reg->umin_value &= mask;
|
|
+ reg->umax_value &= mask;
|
|
+ } else {
|
|
+ reg->umin_value = 0;
|
|
+ reg->umax_value = mask;
|
|
+ }
|
|
+ reg->smin_value = reg->umin_value;
|
|
+ reg->smax_value = reg->umax_value;
|
|
+}
|
|
+
|
|
/* check whether memory at (regno + off) is accessible for t = (read | write)
|
|
* if t==write, value_regno is a register which value is stored into memory
|
|
* if t==read, value_regno is a register which will receive the value from memory
|
|
@@ -1217,9 +1240,7 @@ static int check_mem_access(struct bpf_v
|
|
if (!err && size < BPF_REG_SIZE && value_regno >= 0 && t == BPF_READ &&
|
|
state->regs[value_regno].type == SCALAR_VALUE) {
|
|
/* b/h/w load zero-extends, mark upper bits as known 0 */
|
|
- state->regs[value_regno].var_off = tnum_cast(
|
|
- state->regs[value_regno].var_off, size);
|
|
- __update_reg_bounds(&state->regs[value_regno]);
|
|
+ coerce_reg_to_size(&state->regs[value_regno], size);
|
|
}
|
|
return err;
|
|
}
|
|
@@ -1765,14 +1786,6 @@ static int check_call(struct bpf_verifie
|
|
return 0;
|
|
}
|
|
|
|
-static void coerce_reg_to_32(struct bpf_reg_state *reg)
|
|
-{
|
|
- /* clear high 32 bits */
|
|
- reg->var_off = tnum_cast(reg->var_off, 4);
|
|
- /* Update bounds */
|
|
- __update_reg_bounds(reg);
|
|
-}
|
|
-
|
|
static bool signed_add_overflows(s64 a, s64 b)
|
|
{
|
|
/* Do the add in u64, where overflow is well-defined */
|
|
@@ -2010,8 +2023,8 @@ static int adjust_scalar_min_max_vals(st
|
|
|
|
if (BPF_CLASS(insn->code) != BPF_ALU64) {
|
|
/* 32-bit ALU ops are (32,32)->64 */
|
|
- coerce_reg_to_32(dst_reg);
|
|
- coerce_reg_to_32(&src_reg);
|
|
+ coerce_reg_to_size(dst_reg, 4);
|
|
+ coerce_reg_to_size(&src_reg, 4);
|
|
}
|
|
smin_val = src_reg.smin_value;
|
|
smax_val = src_reg.smax_value;
|
|
@@ -2391,10 +2404,7 @@ static int check_alu_op(struct bpf_verif
|
|
return -EACCES;
|
|
}
|
|
mark_reg_unknown(env, regs, insn->dst_reg);
|
|
- /* high 32 bits are known zero. */
|
|
- regs[insn->dst_reg].var_off = tnum_cast(
|
|
- regs[insn->dst_reg].var_off, 4);
|
|
- __update_reg_bounds(®s[insn->dst_reg]);
|
|
+ coerce_reg_to_size(®s[insn->dst_reg], 4);
|
|
}
|
|
} else {
|
|
/* case: R = imm
|