linux-kernel - Re: [PATCH bpf] bpf: Reject pointer spill with var offset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 25 Jan 2024 09:34:44 +0100
From: Hao Sun <sunhao.th@...il.com>
To: Eduard Zingerman <eddyz87@...il.com>
Cc: bpf@...r.kernel.org, andreimatei1@...il.com, ast@...nel.org, 
	andrii@...nel.org, daniel@...earbox.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf] bpf: Reject pointer spill with var offset

>
> 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.

Testing this case with test_progs/test_verifier is hard because it happens
when cpu_mitigations_off() is true, but we do not have this setup yet.
So the mentioned prog is rejected by sanitize_check_bounds() due to ptr
alu with var_off when adding it to test_progs, and loading as unpriv.

My local test was conducted: (1) booting the kernel with "mitigations=off"
so that bypass_spec_v1 is true and sanitize_check_bounds() is skipped;
(2) running the prog without the patch leaks the pointer; (3) loading the
prog with the patch applied resulting in the expected message.