diff mbox series

bpf speculative execution fixes for 4.14.y

Message ID CAFzhf4qk9aFhhEtraUo0b9Si2y5taYDgdGwVZoSJ9Yj-59RGrw@mail.gmail.com
State New
Headers show
Series bpf speculative execution fixes for 4.14.y | expand

Commit Message

Piotr Krysiuk March 19, 2021, 11:56 p.m. UTC
I noticed that bpf speculative execution fixes are already queued for
4.14.y except for f232326f6966 ("bpf: Prohibit alu ops for pointer
types not defining ptr_limit").

It is important that for all patches from this series to be applied
together, so we avoid introducing a new vulnerability.

For the missing patch, I see conflicting lines in the context diffs
due to API change that apparently caused import to fail.

I'm attaching a copy of the patch that is backported to 4.14.y. The
only change comparing with version queued for newer version is that
"verbose" API does not take "env" parameter.

Please queue or let me know how to proceed.

Thanks,

Piotr

Comments

gregkh@linuxfoundation.org March 20, 2021, 7:42 a.m. UTC | #1
On Fri, Mar 19, 2021 at 11:56:18PM +0000, Piotr Krysiuk wrote:
> I noticed that bpf speculative execution fixes are already queued for

> 4.14.y except for f232326f6966 ("bpf: Prohibit alu ops for pointer

> types not defining ptr_limit").

> 

> It is important that for all patches from this series to be applied

> together, so we avoid introducing a new vulnerability.

> 

> For the missing patch, I see conflicting lines in the context diffs

> due to API change that apparently caused import to fail.

> 

> I'm attaching a copy of the patch that is backported to 4.14.y. The

> only change comparing with version queued for newer version is that

> "verbose" API does not take "env" parameter.

> 

> Please queue or let me know how to proceed.


Thank you for the updates.  I had not looked into them further as I was
not going to do a 4.14.y release this week, but was waiting to next week
and would dig into it then, but you have saved me that effort, thanks!

I'll queue these up in a day or so, and if I've missed anything else
here, please let me know.

greg k-h
gregkh@linuxfoundation.org March 20, 2021, 10:43 a.m. UTC | #2
On Fri, Mar 19, 2021 at 11:56:18PM +0000, Piotr Krysiuk wrote:
> I noticed that bpf speculative execution fixes are already queued for

> 4.14.y except for f232326f6966 ("bpf: Prohibit alu ops for pointer

> types not defining ptr_limit").

> 

> It is important that for all patches from this series to be applied

> together, so we avoid introducing a new vulnerability.

> 

> For the missing patch, I see conflicting lines in the context diffs

> due to API change that apparently caused import to fail.

> 

> I'm attaching a copy of the patch that is backported to 4.14.y. The

> only change comparing with version queued for newer version is that

> "verbose" API does not take "env" parameter.

> 

> Please queue or let me know how to proceed.


Now queued up, thanks!

greg k-h
Piotr Krysiuk March 20, 2021, 10:50 a.m. UTC | #3
Thanks for the quick turn-around,

Piotr

On Sat, Mar 20, 2021 at 10:43 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>

> On Fri, Mar 19, 2021 at 11:56:18PM +0000, Piotr Krysiuk wrote:

> > I noticed that bpf speculative execution fixes are already queued for

> > 4.14.y except for f232326f6966 ("bpf: Prohibit alu ops for pointer

> > types not defining ptr_limit").

> >

> > It is important that for all patches from this series to be applied

> > together, so we avoid introducing a new vulnerability.

> >

> > For the missing patch, I see conflicting lines in the context diffs

> > due to API change that apparently caused import to fail.

> >

> > I'm attaching a copy of the patch that is backported to 4.14.y. The

> > only change comparing with version queued for newer version is that

> > "verbose" API does not take "env" parameter.

> >

> > Please queue or let me know how to proceed.

>

> Now queued up, thanks!

>

> greg k-h
diff mbox series

Patch

From f232326f6966cf2a1d1db7bc917a4ce5f9f55f76 Mon Sep 17 00:00:00 2001
From: Piotr Krysiuk <piotras@gmail.com>
Date: Tue, 16 Mar 2021 09:47:02 +0100
Subject: bpf: Prohibit alu ops for pointer types not defining ptr_limit

From: Piotr Krysiuk <piotras@gmail.com>

commit f232326f6966cf2a1d1db7bc917a4ce5f9f55f76 upstream.

The purpose of this patch is to streamline error propagation and in particular
to propagate retrieve_ptr_limit() errors for pointer types that are not defining
a ptr_limit such that register-based alu ops against these types can be rejected.

The main rationale is that a gap has been identified by Piotr in the existing
protection against speculatively out-of-bounds loads, for example, in case of
ctx pointers, unprivileged programs can still perform pointer arithmetic. This
can be abused to execute speculatively out-of-bounds loads without restrictions
and thus extract contents of kernel memory.

Fix this by rejecting unprivileged programs that attempt any pointer arithmetic
on unprotected pointer types. The two affected ones are pointer to ctx as well
as pointer to map. Field access to a modified ctx' pointer is rejected at a
later point in time in the verifier, and 7c6967326267 ("bpf: Permit map_ptr
arithmetic with opcode add and offset 0") only relevant for root-only use cases.
Risk of unprivileged program breakage is considered very low.

Fixes: 7c6967326267 ("bpf: Permit map_ptr arithmetic with opcode add and offset 0")
Fixes: b2157399cc98 ("bpf: prevent out-of-bounds speculation")
Signed-off-by: Piotr Krysiuk <piotras@gmail.com>
Co-developed-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/verifier.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4192a9e56654..1c8cbef7cc14 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5934,6 +5934,7 @@  static int sanitize_ptr_alu(struct bpf_verifier_env *env,
 	u32 alu_state, alu_limit;
 	struct bpf_reg_state tmp;
 	bool ret;
+	int err;
 
 	if (can_skip_alu_sanitation(env, insn))
 		return 0;
@@ -5949,10 +5950,13 @@  static int sanitize_ptr_alu(struct bpf_verifier_env *env,
 	alu_state |= ptr_is_dst_reg ?
 		     BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST;
 
-	if (retrieve_ptr_limit(ptr_reg, &alu_limit, opcode, off_is_neg))
-		return 0;
-	if (update_alu_sanitation_state(aux, alu_state, alu_limit))
-		return -EACCES;
+	err = retrieve_ptr_limit(ptr_reg, &alu_limit, opcode, off_is_neg);
+	if (err < 0)
+		return err;
+
+	err = update_alu_sanitation_state(aux, alu_state, alu_limit);
+	if (err < 0)
+		return err;
 do_sim:
 	/* Simulate and find potential out-of-bounds access under
 	 * speculative execution from truncation as a result of
@@ -6103,7 +6107,7 @@  static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 	case BPF_ADD:
 		ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0);
 		if (ret < 0) {
-			verbose("R%d tried to add from different maps or paths\n", dst);
+			verbose("R%d tried to add from different maps, paths, or prohibited types\n", dst);
 			return ret;
 		}
 		/* We can take a fixed offset as long as it doesn't overflow
@@ -6158,7 +6162,7 @@  static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 	case BPF_SUB:
 		ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0);
 		if (ret < 0) {
-			verbose("R%d tried to sub from different maps or paths\n", dst);
+			verbose("R%d tried to sub from different maps, paths, or prohibited types\n", dst);
 			return ret;
 		}
 		if (dst_reg == off_reg) {
-- 
2.25.1