On Wed, 2024-01-24 at 11:30 +0100, Hao Sun wrote: > 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 I tried this example as a part of selftest (If put to tools/testing/selftests/bpf/progs/verifier_map_ptr.c could be executed using command: ./test_progs -vvv -a 'verifier_map_ptr/ctx_addr_leak @unpriv'): SEC("socket") __failure_unpriv __msg_unpriv("spilling pointer with var-offset is disallowed") __naked void ctx_addr_leak(void) { asm volatile ( "r0 = 0;" "*(u64 *)(r10 -8) = r0;" "*(u64 *)(r10 -16) = r0;" "*(u64 *)(r10 -24) = r0;" "r6 = r1;" "r1 = 8;" "r1 /= 1;" "r1 &= 8;" "r2 = r10;" "r2 += -16;" "r2 += r1;" "*(u64 *)(r2 +0) = r6;" "r1 = %[map_hash_16b] ll;" "r2 = r10;" "r2 += -16;" "r3 = r10;" "r3 += -8;" "r4 = 0;" "call %[bpf_map_update_elem];" "r0 = *(u64 *)(r10 -8);" "exit;" : : __imm(bpf_map_update_elem), __imm_addr(map_hash_16b) : __clobber_all); } And see the following error message: ... r1 &= 8 ; R1_w=Pscalar(smin=smin32=0,smax=umax=smax32=umax32=8,var_off=(0x0; 0x8)) r2 = r10 ; R2_w=fp0 R10=fp0 r2 += -16 ; R2_w=fp-16 r2 += r1 R2 variable stack access prohibited for !root, var_off=(0x0; 0x8) off=-16 Could you please craft a selftest that checks for expected message? Overall the change makes sense to me.