From: Ovidiu Panait <ovidiu.panait@windriver.com>
To: stable@vger.kernel.org
Cc: bpf@vger.kernel.org, daniel@iogearbox.net
Subject: [PATCH 4.19 02/13] bpf: correct slot_type marking logic to allow more stack slot sharing
Date: Mon, 13 Sep 2021 18:35:26 +0300 [thread overview]
Message-ID: <20210913153537.2162465-3-ovidiu.panait@windriver.com> (raw)
In-Reply-To: <20210913153537.2162465-1-ovidiu.panait@windriver.com>
From: Jiong Wang <jiong.wang@netronome.com>
commit 0bae2d4d62d523f06ff1a8e88ce38b45400acd28 upstream.
Verifier is supposed to support sharing stack slot allocated to ptr with
SCALAR_VALUE for privileged program. However this doesn't happen for some
cases.
The reason is verifier is not clearing slot_type STACK_SPILL for all bytes,
it only clears part of them, while verifier is using:
slot_type[0] == STACK_SPILL
as a convention to check one slot is ptr type.
So, the consequence of partial clearing slot_type is verifier could treat a
partially overridden ptr slot, which should now be a SCALAR_VALUE slot,
still as ptr slot, and rejects some valid programs.
Before this patch, test_xdp_noinline.o under bpf selftests, bpf_lxc.o and
bpf_netdev.o under Cilium bpf repo, when built with -mattr=+alu32 are
rejected due to this issue. After this patch, they all accepted.
There is no processed insn number change before and after this patch on
Cilium bpf programs.
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
Reviewed-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
[OP: adjusted context for 4.19]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
kernel/bpf/verifier.c | 5 +++
tools/testing/selftests/bpf/test_verifier.c | 34 +++++++++++++++++++--
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a5259ff30073..b6f008dcb30c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1039,6 +1039,10 @@ static int check_stack_write(struct bpf_verifier_env *env,
/* regular write of data into stack destroys any spilled ptr */
state->stack[spi].spilled_ptr.type = NOT_INIT;
+ /* Mark slots as STACK_MISC if they belonged to spilled ptr. */
+ if (state->stack[spi].slot_type[0] == STACK_SPILL)
+ for (i = 0; i < BPF_REG_SIZE; i++)
+ state->stack[spi].slot_type[i] = STACK_MISC;
/* only mark the slot as written if all 8 bytes were written
* otherwise read propagation may incorrectly stop too soon
@@ -1056,6 +1060,7 @@ static int check_stack_write(struct bpf_verifier_env *env,
register_is_null(&cur->regs[value_regno]))
type = STACK_ZERO;
+ /* Mark slots affected by this stack write. */
for (i = 0; i < size; i++)
state->stack[spi].slot_type[(slot - i) % BPF_REG_SIZE] =
type;
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c7d17781dbfe..6b9ed915c6b0 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -956,15 +956,45 @@ static struct bpf_test tests[] = {
BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8),
/* mess up with R1 pointer on stack */
BPF_ST_MEM(BPF_B, BPF_REG_10, -7, 0x23),
- /* fill back into R0 should fail */
+ /* fill back into R0 is fine for priv.
+ * R0 now becomes SCALAR_VALUE.
+ */
BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
+ /* Load from R0 should fail. */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 8),
BPF_EXIT_INSN(),
},
.errstr_unpriv = "attempt to corrupt spilled",
- .errstr = "corrupted spill",
+ .errstr = "R0 invalid mem access 'inv",
.result = REJECT,
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
+ {
+ "check corrupted spill/fill, LSB",
+ .insns = {
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_H, BPF_REG_10, -8, 0xcafe),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
+ BPF_EXIT_INSN(),
+ },
+ .errstr_unpriv = "attempt to corrupt spilled",
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+ .retval = POINTER_VALUE,
+ },
+ {
+ "check corrupted spill/fill, MSB",
+ .insns = {
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 0x12345678),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
+ BPF_EXIT_INSN(),
+ },
+ .errstr_unpriv = "attempt to corrupt spilled",
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+ .retval = POINTER_VALUE,
+ },
{
"invalid src register in STX",
.insns = {
--
2.25.1
next prev parent reply other threads:[~2021-09-13 15:36 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-13 15:35 [PATCH 4.19 00/13] bpf: backport fixes for CVE-2021-34556/CVE-2021-35477 Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 01/13] bpf/verifier: per-register parent pointers Ovidiu Panait
2021-09-13 15:35 ` Ovidiu Panait [this message]
2021-09-13 15:35 ` [PATCH 4.19 03/13] bpf: Support variable offset stack access from helpers Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 04/13] bpf: Reject indirect var_off stack access in raw mode Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 05/13] bpf: Reject indirect var_off stack access in unpriv mode Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 06/13] bpf: Sanity check max value for var_off stack access Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 07/13] selftests/bpf: Test variable offset " Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 08/13] bpf: track spill/fill of constants Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 09/13] selftests/bpf: fix tests due to const spill/fill Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 10/13] bpf: Introduce BPF nospec instruction for mitigating Spectre v4 Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 11/13] bpf: Fix leakage due to insufficient speculative store bypass mitigation Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 12/13] bpf: verifier: Allocate idmap scratch in verifier env Ovidiu Panait
2021-09-13 15:35 ` [PATCH 4.19 13/13] bpf: Fix pointer arithmetic mask tightening under state pruning Ovidiu Panait
2021-09-15 12:39 ` [PATCH 4.19 00/13] bpf: backport fixes for CVE-2021-34556/CVE-2021-35477 Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210913153537.2162465-3-ovidiu.panait@windriver.com \
--to=ovidiu.panait@windriver.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).