This is a note to let you know that I've just added the patch titled
[PATCH stable/4.14 08/14] bpf: fix incorrect tracking of register size truncation
to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
bpf-fix-incorrect-tracking-of-register-size-truncation.patch
and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From foo@baz Fri Dec 22 16:47:02 CET 2017
From: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Date: Fri, 22 Dec 2017 16:23:06 +0100
Subject: [PATCH stable/4.14 08/14] bpf: fix incorrect tracking of register size truncation
To: gregkh@xxxxxxxxxxxxxxxxxxx
Cc: ast@xxxxxxxxxx, daniel@xxxxxxxxxxxxx, jannh@xxxxxxxxxx, stable@xxxxxxxxxxxxxxx
Message-ID: <20171222152312.2945-9-daniel@xxxxxxxxxxxxx>
From: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
From: Jann Horn <jannh@xxxxxxxxxx>
[ Upstream commit 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@xxxxxxxxxx>
Acked-by: Edward Cree <ecree@xxxxxxxxxxxxxx>
Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
kernel/bpf/verifier.c | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1068,6 +1068,29 @@ static int check_ptr_alignment(struct bp
return check_generic_ptr_alignment(reg, pointer_desc, off, size, 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
@@ -1200,9 +1223,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;
}
@@ -1742,14 +1763,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 */
@@ -1984,8 +1997,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;
@@ -2364,10 +2377,7 @@ static int check_alu_op(struct bpf_verif
return -EACCES;
}
mark_reg_unknown(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
Patches currently in stable-queue which might be from daniel@xxxxxxxxxxxxx are
queue-4.14/bpf-fix-integer-overflows.patch
queue-4.14/bpf-fix-branch-pruning-logic.patch
queue-4.14/bpf-s390x-do-not-reload-skb-pointers-in-non-skb-context.patch
queue-4.14/bpf-sparc-fix-usage-of-wrong-reg-for-load_skb_regs-after-call.patch
queue-4.14/bpf-fix-incorrect-tracking-of-register-size-truncation.patch
queue-4.14/bpf-don-t-prune-branches-when-a-scalar-is-replaced-with-a-pointer.patch
queue-4.14/bpf-verifier-fix-bounds-calculation-on-bpf_rsh.patch
queue-4.14/selftests-bpf-add-tests-for-recent-bugfixes.patch
queue-4.14/bpf-fix-corruption-on-concurrent-perf_event_output-calls.patch
queue-4.14/bpf-fix-incorrect-sign-extension-in-check_alu_op.patch
queue-4.14/bpf-ppc64-do-not-reload-skb-pointers-in-non-skb-context.patch
queue-4.14/bpf-fix-missing-error-return-in-check_stack_boundary.patch
queue-4.14/bpf-force-strict-alignment-checks-for-stack-pointers.patch
queue-4.14/bpf-fix-32-bit-alu-op-verification.patch
queue-4.14/bpf-fix-build-issues-on-um-due-to-mising-bpf_perf_event.h.patch