public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Hao Sun <sunhao.th@gmail.com>
To: bpf@vger.kernel.org
Cc: andreimatei1@gmail.com, ast@kernel.org, andrii@kernel.org,
	daniel@iogearbox.net, eddyz87@gmail.com,
	linux-kernel@vger.kernel.org, Hao Sun <sunhao.th@gmail.com>
Subject: [PATCH bpf] bpf: Reject pointer spill with var offset
Date: Wed, 24 Jan 2024 11:30:10 +0100	[thread overview]
Message-ID: <20240124103010.51408-1-sunhao.th@gmail.com> (raw)

check_stack_write_var_off() does not reject pointer reg, this can lead
to pointer leak. When cpu_mitigation_off(), unprivileged users can add
var off to stack pointer, and loading the following prog enable them
leak kernel address:

func#0 @0
0: R1=ctx() R10=fp0
0: (7a) *(u64 *)(r10 -8) = 0          ; R10=fp0 fp-8_w=00000000
1: (7a) *(u64 *)(r10 -16) = 0         ; R10=fp0 fp-16_w=00000000
2: (7a) *(u64 *)(r10 -24) = 0         ; R10=fp0 fp-24_w=00000000
3: (bf) r6 = r1                       ; R1=ctx() R6_w=ctx()
4: (b7) r1 = 8                        ; R1_w=P8
5: (37) r1 /= 1                       ; R1_w=Pscalar()
6: (57) r1 &= 8                       ; R1_w=Pscalar(smin=smin32=0,smax=umax=smax32=umax32=8,var_off=(0x0; 0x8))
7: (bf) r2 = r10                      ; R2_w=fp0 R10=fp0
8: (07) r2 += -16                     ; R2_w=fp-16
9: (0f) r2 += r1                      ; R1_w=Pscalar(smin=smin32=0,smax=umax=smax32=umax32=8,var_off=(0x0; 0x8)) R2_w=fp(off=-16,smin=smin32=0,smax=umax=smax32=umax32=8,var_off=(0x0; 0x8))
10: (7b) *(u64 *)(r2 +0) = r6         ; R2_w=fp(off=-16,smin=smin32=0,smax=umax=smax32=umax32=8,var_off=(0x0; 0x8)) R6_w=ctx() fp-8_w=mmmmmmmm fp-16_w=mmmmmmmm
11: (18) r1 = 0x0                     ; R1_w=map_ptr(ks=4,vs=8)
13: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
14: (07) r2 += -16                    ; R2_w=fp-16
15: (bf) r3 = r10                     ; R3_w=fp0 R10=fp0
16: (07) r3 += -8                     ; R3_w=fp-8
17: (b7) r4 = 0                       ; R4_w=P0
18: (85) call bpf_map_update_elem#2   ; R0_w=Pscalar()
19: (79) r0 = *(u64 *)(r10 -8)        ; R0_w=Pscalar() R10=fp0 fp-8_w=mmmmmmmm
20: (95) exit
processed 20 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

The prog first inits several slots, so it later can access, and then
adds var-off to fp, where it knows the off is -8. Finally, the prog
spills the ctx ptr and leaks it to a map, and unprivileged users can
read the pointer through a map lookup:

	Leaked Map Address: 0xffff98d3828f5700

Fix this by rejecting pointer reg in check_stack_write_var_off().
Applying the patch makes the prog rejected with "spilling pointer
with var-offset is disallowed".

Also add missed newline to error messages in this check.

Signed-off-by: Hao Sun <sunhao.th@gmail.com>
---

Note that it's hard to add this test to test_progs or test_verifier, as
this requires cpu_mitigation_off() setup, currently tested on my local.

 kernel/bpf/verifier.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f31868ba0c2d..c34b938fa06f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4627,6 +4627,11 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
 	    (!value_reg && is_bpf_st_mem(insn) && insn->imm == 0))
 		writing_zero = true;
 
+	if (value_reg && __is_pointer_value(env->allow_ptr_leaks, value_reg)) {
+		verbose(env, "spilling pointer with var-offset is disallowed\n");
+		return -EINVAL;
+	}
+
 	for (i = min_off; i < max_off; i++) {
 		int spi;
 
@@ -4658,7 +4663,7 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
 			 * later for CAP_PERFMON, as the write may not happen to
 			 * that slot.
 			 */
-			verbose(env, "spilled ptr in range of var-offset stack write; insn %d, ptr off: %d",
+			verbose(env, "spilled ptr in range of var-offset stack write; insn %d, ptr off: %d\n",
 				insn_idx, i);
 			return -EINVAL;
 		}
@@ -4694,7 +4699,7 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
 		 * them, the error would be too confusing.
 		 */
 		if (*stype == STACK_INVALID && !env->allow_uninit_stack) {
-			verbose(env, "uninit stack in range of var-offset write prohibited for !root; insn %d, off: %d",
+			verbose(env, "uninit stack in range of var-offset write prohibited for !root; insn %d, off: %d\n",
 					insn_idx, i);
 			return -EINVAL;
 		}
-- 
2.34.1


             reply	other threads:[~2024-01-24 10:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-24 10:30 Hao Sun [this message]
2024-01-25  1:35 ` [PATCH bpf] bpf: Reject pointer spill with var offset Eduard Zingerman
2024-01-25  8:34   ` Hao Sun
2024-01-25 13:05     ` Eduard Zingerman

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=20240124103010.51408-1-sunhao.th@gmail.com \
    --to=sunhao.th@gmail.com \
    --cc=andreimatei1@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=linux-kernel@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