netdev - Re: len = bpf_probe_read_str(); bpf_perf_event

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 for Android: free password hash cracker in your pocket

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

Message-ID: <b1c2575f-7663-4adf-dbb7-aaa9df15b283@iogearbox.net>
Date:   Tue, 14 Nov 2017 14:09:34 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Gianluca Borello <g.borello@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        David Miller <davem@...emloft.net>,
        Linux Networking Development Mailing List 
        <netdev@...r.kernel.org>, yhs@...com
Subject: Re: len = bpf_probe_read_str(); bpf_perf_event_output(... len) ==
 FAIL

On 11/14/2017 01:58 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 14, 2017 at 01:09:39AM +0100, Daniel Borkmann escreveu:
>> On 11/13/2017 04:08 PM, Arnaldo Carvalho de Melo wrote:
>>> libbpf: -- BEGIN DUMP LOG ---
>>> libbpf: 
>>> 0: (79) r3 = *(u64 *)(r1 +104)
>>> 1: (b7) r2 = 0
>>> 2: (bf) r6 = r1
>>> 3: (bf) r1 = r10
>>> 4: (07) r1 += -128
>>> 5: (b7) r2 = 128
>>> 6: (85) call bpf_probe_read_str#45
>>> 7: (bf) r1 = r0
>>> 8: (07) r1 += -1
>>> 9: (67) r1 <<= 32
>>> 10: (77) r1 >>= 32
>>> 11: (25) if r1 > 0x7f goto pc+11
>>
>> Right, so the compiler is optimizing the two tests into a single one above,
>> which means lower bound cannot properly be derived again by the verifier due
>> to this and thus you'll get the error. Similar issue was seen recently [1].
>>
>> Does the below hack work for you?
>>
>> int prog([...])
>> {
>>         char filename[128];
>>         int ret = bpf_probe_read_str(filename, sizeof(filename), filename_ptr);
>>         if (ret > 0)
>>                 bpf_perf_event_output(ctx, &__bpf_stdout__, BPF_F_CURRENT_CPU, filename,
>>                                       ret & (sizeof(filename) - 1));
>>         return 1;
>> }
>>
>> r0 should keep on tracking bounds here at least:
>>
>> prog:
>>        0:	bf 16 00 00 00 00 00 00 	r6 = r1
>>        1:	bf a1 00 00 00 00 00 00 	r1 = r10
>>        2:	07 01 00 00 80 ff ff ff 	r1 += -128
>>        3:	b7 02 00 00 80 00 00 00 	r2 = 128
>>        4:	85 00 00 00 2d 00 00 00 	call 45
>>        5:	67 00 00 00 20 00 00 00 	r0 <<= 32
>>        6:	c7 00 00 00 20 00 00 00 	r0 s>>= 32
>>        7:	b7 01 00 00 01 00 00 00 	r1 = 1
>>        8:	6d 01 0a 00 00 00 00 00 	if r1 s> r0 goto 10
>>        9:	57 00 00 00 7f 00 00 00 	r0 &= 127
>>       10:	bf a4 00 00 00 00 00 00 	r4 = r10
>>       11:	07 04 00 00 80 ff ff ff 	r4 += -128
>>       12:	bf 61 00 00 00 00 00 00 	r1 = r6
>>       13:	18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 	r2 = 0ll
>>       15:	18 03 00 00 ff ff ff ff 00 00 00 00 00 00 00 00 	r3 = 4294967295ll
>>       17:	bf 05 00 00 00 00 00 00 	r5 = r0
>>       18:	85 00 00 00 19 00 00 00 	call 25
>>
>>   [1] http://patchwork.ozlabs.org/project/netdev/list/?series=13211
> 
> Not yet:
> 
> 6: (85) call bpf_probe_read_str#45
> 7: (bf) r1 = r0
> 8: (67) r1 <<= 32
> 9: (77) r1 >>= 32
> 10: (15) if r1 == 0x0 goto pc+10
>  R0=inv(id=0) R1=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R6=ctx(id=0,off=0,imm=0) R10=fp0
> 11: (57) r0 &= 127
> 12: (bf) r4 = r10
> 13: (07) r4 += -128
> 14: (bf) r1 = r6
> 15: (18) r2 = 0xffff92bfc2aba840
> 17: (18) r3 = 0xffffffff
> 19: (bf) r5 = r0
> 20: (85) call bpf_perf_event_output#25
> invalid stack type R4 off=-128 access_size=0
> 
> I'll try updating clang/llvm...
> 
> Full details:
> 
> [root@...et bpf]# cat open.c 
> #include "bpf.h"
> 
> SEC("prog=do_sys_open filename")
> int prog(void *ctx, int err, const char __user *filename_ptr)
> {
> 	char filename[128];
> 	const unsigned len = bpf_probe_read_str(filename, sizeof(filename), filename_ptr);

Btw, I was using 'int' here above instead of 'unsigned' as strncpy_from_unsafe()
could potentially return errors like -EFAULT.

Currently having a version compiled from the git tree:

# llc --version
LLVM (http://llvm.org/):
  LLVM version 6.0.0git-2d810c2
  Optimized build.
  Default target: x86_64-unknown-linux-gnu
  Host CPU: skylake

  Registered Targets:
    bpf    - BPF (host endian)
    bpfeb  - BPF (big endian)
    bpfel  - BPF (little endian)
    x86    - 32-bit X86: Pentium-Pro and above
    x86-64 - 64-bit X86: EM64T and AMD64

> 	if (len > 0)
>        		perf_event_output(ctx, &__bpf_stdout__, BPF_F_CURRENT_CPU, filename,
> 				  len & (sizeof(filename) - 1));
> 	return 1;
> }