--- /dev/null
+From f30d4968e9aee737e174fc97942af46cfb49b484 Mon Sep 17 00:00:00 2001
+From: Martin KaFai Lau <kafai@fb.com>
+Date: Mon, 1 Nov 2021 23:45:35 -0700
+Subject: bpf: Do not reject when the stack read size is different from the tracked scalar size
+
+From: Martin KaFai Lau <kafai@fb.com>
+
+commit f30d4968e9aee737e174fc97942af46cfb49b484 upstream.
+
+Below is a simplified case from a report in bcc [0]:
+
+ r4 = 20
+ *(u32 *)(r10 -4) = r4
+ *(u32 *)(r10 -8) = r4 /* r4 state is tracked */
+ r4 = *(u64 *)(r10 -8) /* Read more than the tracked 32bit scalar.
+ * verifier rejects as 'corrupted spill memory'.
+ */
+
+After commit 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill"),
+the 8-byte aligned 32bit spill is also tracked by the verifier and the
+register state is stored.
+
+However, if 8 bytes are read from the stack instead of the tracked 4 byte
+scalar, then verifier currently rejects the program as "corrupted spill
+memory". This patch fixes this case by allowing it to read but marks the
+register as unknown.
+
+Also note that, if the prog is trying to corrupt/leak an earlier spilled
+pointer by spilling another <8 bytes register on top, this has already
+been rejected in the check_stack_write_fixed_off().
+
+ [0] https://github.com/iovisor/bcc/pull/3683
+
+Fixes: 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
+Reported-by: Hengqi Chen <hengqi.chen@gmail.com>
+Reported-by: Yonghong Song <yhs@gmail.com>
+Signed-off-by: Martin KaFai Lau <kafai@fb.com>
+Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
+Tested-by: Hengqi Chen <hengqi.chen@gmail.com>
+Acked-by: Yonghong Song <yhs@fb.com>
+Link: https://lore.kernel.org/bpf/20211102064535.316018-1-kafai@fb.com
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ kernel/bpf/verifier.c | 18 ++++++------------
+ 1 file changed, 6 insertions(+), 12 deletions(-)
+
+--- a/kernel/bpf/verifier.c
++++ b/kernel/bpf/verifier.c
+@@ -2930,9 +2930,12 @@ static int check_stack_read_fixed_off(st
+ reg = ®_state->stack[spi].spilled_ptr;
+
+ if (is_spilled_reg(®_state->stack[spi])) {
+- if (size != BPF_REG_SIZE) {
+- u8 scalar_size = 0;
++ u8 spill_size = 1;
+
++ for (i = BPF_REG_SIZE - 1; i > 0 && stype[i - 1] == STACK_SPILL; i--)
++ spill_size++;
++
++ if (size != BPF_REG_SIZE || spill_size != BPF_REG_SIZE) {
+ if (reg->type != SCALAR_VALUE) {
+ verbose_linfo(env, env->insn_idx, "; ");
+ verbose(env, "invalid size of register fill\n");
+@@ -2943,10 +2946,7 @@ static int check_stack_read_fixed_off(st
+ if (dst_regno < 0)
+ return 0;
+
+- for (i = BPF_REG_SIZE; i > 0 && stype[i - 1] == STACK_SPILL; i--)
+- scalar_size++;
+-
+- if (!(off % BPF_REG_SIZE) && size == scalar_size) {
++ if (!(off % BPF_REG_SIZE) && size == spill_size) {
+ /* The earlier check_reg_arg() has decided the
+ * subreg_def for this insn. Save it first.
+ */
+@@ -2970,12 +2970,6 @@ static int check_stack_read_fixed_off(st
+ state->regs[dst_regno].live |= REG_LIVE_WRITTEN;
+ return 0;
+ }
+- for (i = 1; i < BPF_REG_SIZE; i++) {
+- if (stype[(slot - i) % BPF_REG_SIZE] != STACK_SPILL) {
+- verbose(env, "corrupted spill memory\n");
+- return -EACCES;
+- }
+- }
+
+ if (dst_regno >= 0) {
+ /* restore register state from stack */
--- /dev/null
+From 345e004d023343d38088fdfea39688aa11e06ccf Mon Sep 17 00:00:00 2001
+From: Paul Chaignon <paul@isovalent.com>
+Date: Fri, 10 Dec 2021 00:46:31 +0100
+Subject: bpf: Fix incorrect state pruning for <8B spill/fill
+
+From: Paul Chaignon <paul@isovalent.com>
+
+commit 345e004d023343d38088fdfea39688aa11e06ccf upstream.
+
+Commit 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
+introduced support in the verifier to track <8B spill/fills of scalars.
+The backtracking logic for the precision bit was however skipping
+spill/fills of less than 8B. That could cause state pruning to consider
+two states equivalent when they shouldn't be.
+
+As an example, consider the following bytecode snippet:
+
+ 0: r7 = r1
+ 1: call bpf_get_prandom_u32
+ 2: r6 = 2
+ 3: if r0 == 0 goto pc+1
+ 4: r6 = 3
+ ...
+ 8: [state pruning point]
+ ...
+ /* u32 spill/fill */
+ 10: *(u32 *)(r10 - 8) = r6
+ 11: r8 = *(u32 *)(r10 - 8)
+ 12: r0 = 0
+ 13: if r8 == 3 goto pc+1
+ 14: r0 = 1
+ 15: exit
+
+The verifier first walks the path with R6=3. Given the support for <8B
+spill/fills, at instruction 13, it knows the condition is true and skips
+instruction 14. At that point, the backtracking logic kicks in but stops
+at the fill instruction since it only propagates the precision bit for
+8B spill/fill. When the verifier then walks the path with R6=2, it will
+consider it safe at instruction 8 because R6 is not marked as needing
+precision. Instruction 14 is thus never walked and is then incorrectly
+removed as 'dead code'.
+
+It's also possible to lead the verifier to accept e.g. an out-of-bound
+memory access instead of causing an incorrect dead code elimination.
+
+This regression was found via Cilium's bpf-next CI where it was causing
+a conntrack map update to be silently skipped because the code had been
+removed by the verifier.
+
+This commit fixes it by enabling support for <8B spill/fills in the
+bactracking logic. In case of a <8B spill/fill, the full 8B stack slot
+will be marked as needing precision. Then, in __mark_chain_precision,
+any tracked register spilled in a marked slot will itself be marked as
+needing precision, regardless of the spill size. This logic makes two
+assumptions: (1) only 8B-aligned spill/fill are tracked and (2) spilled
+registers are only tracked if the spill and fill sizes are equal. Commit
+ef979017b837 ("bpf: selftest: Add verifier tests for <8-byte scalar
+spill and refill") covers the first assumption and the next commit in
+this patchset covers the second.
+
+Fixes: 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
+Signed-off-by: Paul Chaignon <paul@isovalent.com>
+Signed-off-by: Alexei Starovoitov <ast@kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ kernel/bpf/verifier.c | 4 ----
+ 1 file changed, 4 deletions(-)
+
+--- a/kernel/bpf/verifier.c
++++ b/kernel/bpf/verifier.c
+@@ -2224,8 +2224,6 @@ static int backtrack_insn(struct bpf_ver
+ */
+ if (insn->src_reg != BPF_REG_FP)
+ return 0;
+- if (BPF_SIZE(insn->code) != BPF_DW)
+- return 0;
+
+ /* dreg = *(u64 *)[fp - off] was a fill from the stack.
+ * that [fp - off] slot contains scalar that needs to be
+@@ -2248,8 +2246,6 @@ static int backtrack_insn(struct bpf_ver
+ /* scalars can only be spilled into stack */
+ if (insn->dst_reg != BPF_REG_FP)
+ return 0;
+- if (BPF_SIZE(insn->code) != BPF_DW)
+- return 0;
+ spi = (-insn->off - 1) / BPF_REG_SIZE;
+ if (spi >= 64) {
+ verbose(env, "BUG spi %d\n", spi);
projects / thirdparty / kernel / stable-queue.git / commitdiff
