From cae3fe6ce7274c58e273361dc1ba59ddb5c92178 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 7 Feb 2023 12:32:26 +0100 Subject: [PATCH] 5.15-stable patches added patches: bpf-do-not-reject-when-the-stack-read-size-is-different-from-the-tracked-scalar-size.patch bpf-fix-incorrect-state-pruning-for-8b-spill-fill.patch --- ...fferent-from-the-tracked-scalar-size.patch | 88 +++++++++++++++++++ ...rect-state-pruning-for-8b-spill-fill.patch | 88 +++++++++++++++++++ queue-5.15/series | 2 + 3 files changed, 178 insertions(+) create mode 100644 queue-5.15/bpf-do-not-reject-when-the-stack-read-size-is-different-from-the-tracked-scalar-size.patch create mode 100644 queue-5.15/bpf-fix-incorrect-state-pruning-for-8b-spill-fill.patch diff --git a/queue-5.15/bpf-do-not-reject-when-the-stack-read-size-is-different-from-the-tracked-scalar-size.patch b/queue-5.15/bpf-do-not-reject-when-the-stack-read-size-is-different-from-the-tracked-scalar-size.patch new file mode 100644 index 00000000000..748055f31a5 --- /dev/null +++ b/queue-5.15/bpf-do-not-reject-when-the-stack-read-size-is-different-from-the-tracked-scalar-size.patch @@ -0,0 +1,88 @@ +From f30d4968e9aee737e174fc97942af46cfb49b484 Mon Sep 17 00:00:00 2001 +From: Martin KaFai Lau +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 + +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 +Reported-by: Yonghong Song +Signed-off-by: Martin KaFai Lau +Signed-off-by: Daniel Borkmann +Tested-by: Hengqi Chen +Acked-by: Yonghong Song +Link: https://lore.kernel.org/bpf/20211102064535.316018-1-kafai@fb.com +Signed-off-by: Greg Kroah-Hartman +--- + 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 */ diff --git a/queue-5.15/bpf-fix-incorrect-state-pruning-for-8b-spill-fill.patch b/queue-5.15/bpf-fix-incorrect-state-pruning-for-8b-spill-fill.patch new file mode 100644 index 00000000000..8f43bc4997f --- /dev/null +++ b/queue-5.15/bpf-fix-incorrect-state-pruning-for-8b-spill-fill.patch @@ -0,0 +1,88 @@ +From 345e004d023343d38088fdfea39688aa11e06ccf Mon Sep 17 00:00:00 2001 +From: Paul Chaignon +Date: Fri, 10 Dec 2021 00:46:31 +0100 +Subject: bpf: Fix incorrect state pruning for <8B spill/fill + +From: Paul Chaignon + +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 +Signed-off-by: Alexei Starovoitov +Signed-off-by: Greg Kroah-Hartman +--- + 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); diff --git a/queue-5.15/series b/queue-5.15/series index 83909ba0306..f952c4d3efa 100644 --- a/queue-5.15/series +++ b/queue-5.15/series @@ -106,3 +106,5 @@ phy-qcom-qmp-combo-fix-memleak-on-probe-deferral.patch phy-qcom-qmp-usb-fix-memleak-on-probe-deferral.patch phy-qcom-qmp-combo-fix-broken-power-on.patch phy-qcom-qmp-combo-fix-runtime-suspend.patch +bpf-fix-incorrect-state-pruning-for-8b-spill-fill.patch +bpf-do-not-reject-when-the-stack-read-size-is-different-from-the-tracked-scalar-size.patch -- 2.47.3