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@xxxxxxxxx> --- 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