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  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:   Tue, 14 Nov 2017 01:09:39 +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/13/2017 04:08 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 13, 2017 at 03:56:14PM +0100, Daniel Borkmann escreveu:
>> On 11/13/2017 03:30 PM, Arnaldo Carvalho de Melo wrote:
>>> Hi,
>>>
>>> 	In a5e8c07059d0 ("bpf: add bpf_probe_read_str helper") you
>>> state:
>>>
>>>    "This is suboptimal because the size of the string needs to be estimated
>>>     at compile time, causing more memory to be copied than often necessary,
>>>     and can become more problematic if further processing on buf is done,
>>>     for example by pushing it to userspace via bpf_perf_event_output(),
>>>     since the real length of the string is unknown and the entire buffer
>>>     must be copied (and defining an unrolled strnlen() inside the bpf
>>>     program is a very inefficient and unfeasible approach)."
>>>
>>> So I went on to try this with 'perf trace' but it isn't working if I use
>>> the return from bpf_probe_read_str(), I must be missing something
>>> here... 
>>>
>>> I.e. this works:
>>>
>>> [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);
>>> 	perf_event_output(ctx, &__bpf_stdout__, get_smp_processor_id(), filename, 32);
>>
>> By the way, you can just use BPF_F_CURRENT_CPU flag instead of the helper
>> call get_smp_processor_id() to get current CPU.
> 
> Thanks, switched to it.
> 
>>> But then if I use the return value to push just the string lenght, it
>>> doesn't work:
>>>
>>> [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);
>>> 	perf_event_output(ctx, &__bpf_stdout__, get_smp_processor_id(), filename, len);
>>
>> The below issue 'invalid stack type R4 off=-128 access_size=0' is basically that
>> unsigned len is unknown at verification time, thus unbounded. Can you try the
>> following to see if that passes?
>>
>> if (len > 0 && len <= sizeof(filename))
>>     perf_event_output(ctx, &__bpf_stdout__, get_smp_processor_id(), filename, len);
> 
> I had it like:
> 
> 	if (len > 0 && len < 32)
> 
> And it didn't helped, now I did exactly as you suggested:
> 
> [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);
> 	if (len > 0 && len <= sizeof(filename))
> 		perf_event_output(ctx, &__bpf_stdout__, BPF_F_CURRENT_CPU, filename, len);
> 	return 1;
> }
> [root@...et bpf]# trace -e open,open.c touch /etc/passwd
> bpf: builtin compilation failed: -95, try external compiler
> event syntax error: 'open.c'
>                      \___ Kernel verifier blocks program loading
> <SNIP>
> [root@...et bpf]# 
> 
> The -v output looks the same:
> 
> .[root@...et bpf]# trace -v -e open,open.c touch /etc/passwd
> bpf: builtin compilation failed: -95, try external compiler
> Kernel build dir is set to /lib/modules/4.14.0-rc6+/build
> set env: KBUILD_DIR=/lib/modules/4.14.0-rc6+/build
> unset env: KBUILD_OPTS
> include option is set to  -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/7/include -I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated  -I/home/acme/git/linux/include -I./include -I/home/acme/git/linux/arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi -I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h 
> set env: NR_CPUS=4
> set env: LINUX_VERSION_CODE=0x40e00
> set env: CLANG_EXEC=/usr/local/bin/clang
> unset env: CLANG_OPTIONS
> set env: KERNEL_INC_OPTIONS= -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/7/include -I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated  -I/home/acme/git/linux/include -I./include -I/home/acme/git/linux/arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi -I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h 
> set env: WORKING_DIR=/lib/modules/4.14.0-rc6+/build
> set env: CLANG_SOURCE=/home/acme/bpf/open.c
> llvm compiling command template: $CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS -DLINUX_VERSION_CODE=$LINUX_VERSION_CODE $CLANG_OPTIONS $KERNEL_INC_OPTIONS -Wno-unused-value -Wno-pointer-sign -working-directory $WORKING_DIR -c "$CLANG_SOURCE" -target bpf -O2 -o -
> libbpf: loading object 'open.c' from buffer
> libbpf: section .strtab, size 103, link 0, flags 0, type=3
> libbpf: section .text, size 0, link 0, flags 6, type=1
> libbpf: section prog=do_sys_open filename, size 184, link 0, flags 6, type=1
> libbpf: found program prog=do_sys_open filename
> libbpf: section .relprog=do_sys_open filename, size 16, link 8, flags 0, type=9
> libbpf: section maps, size 16, link 0, flags 3, type=1
> libbpf: section license, size 4, link 0, flags 3, type=1
> libbpf: license of open.c is GPL
> libbpf: section version, size 4, link 0, flags 3, type=1
> libbpf: kernel version of open.c is 40e00
> libbpf: section .symtab, size 144, link 1, flags 0, type=2
> libbpf: maps in open.c: 1 maps in 16 bytes
> libbpf: map 0 is "__bpf_stdout__"
> libbpf: collecting relocating info for: 'prog=do_sys_open filename'
> libbpf: relocation: insn_idx=15
> libbpf: relocation: find map 0 (__bpf_stdout__) for insn 15
> bpf: config program 'prog=do_sys_open filename'
> symbol:do_sys_open file:(null) line:0 offset:0 return:0 lazy:(null)
> parsing arg: filename into filename
> bpf: config 'prog=do_sys_open filename' is ok
> Looking at the vmlinux_path (8 entries long)
> Using /lib/modules/4.14.0-rc6+/build/vmlinux for symbols
> Open Debuginfo file: /lib/modules/4.14.0-rc6+/build/vmlinux
> Try to find probe point from debuginfo.
> Matched function: do_sys_open [2a2bbbe]
> Probe point found: do_sys_open+0
> Searching 'filename' variable in context.
> Converting variable filename into trace event.
> filename type is (null).
> Opening /sys/kernel/debug/tracing//README write=0
> Found 1 probe_trace_events.
> Opening /sys/kernel/debug/tracing//kprobe_events write=1
> Writing event: p:perf_bpf_probe/prog _text+2493152 filename=%si:x64
> In map_prologue, ntevs=1
> mapping[0]=0
> libbpf: create map __bpf_stdout__: fd=3
> prologue: pass validation
> prologue: fast path
> libbpf: load bpf program failed: Permission denied
> 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

>  R0=inv(id=0) R1=inv(id=0,umax_value=127,var_off=(0x0; 0x7f)) R6=ctx(id=0,off=0,imm=0) R10=fp0
> 12: (67) r0 <<= 32
> 13: (77) r0 >>= 32
> 14: (bf) r4 = r10
> 15: (07) r4 += -128
> 16: (bf) r1 = r6
> 17: (18) r2 = 0xffffa0b74ba91000
> 19: (18) r3 = 0xffffffff
> 21: (bf) r5 = r0
> 22: (85) call bpf_perf_event_output#25
> invalid stack type R4 off=-128 access_size=0
> 
> libbpf: -- END LOG --
> libbpf: Loading the 0th instance of program 'prog=do_sys_open filename' failed
> libbpf: failed to load program 'prog=do_sys_open filename'
> libbpf: failed to load object 'open.c'
> bpf: load objects failed
> event syntax error: 'open.c'
>                      \___ Kernel verifier blocks program loading
> 
> Also:
> 
> [root@...et bpf]# clang -v
> clang version 4.0.0 (http://llvm.org/git/clang.git f5be8ba13adc4ba1011a7ccd60c844bd60427c1c) (http://llvm.org/git/llvm.git efca1a37676f4cd276d947658cf90b0fb625abfd)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /usr/local/bin
> Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/7
> Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/7
> Candidate multilib: .;@m64
> Candidate multilib: 32;@m32
> Selected multilib: .;@m64
> [root@...et bpf]#
> 
> And now I've really attached that bpf.h header I use.
> 
> - Arnaldo
> 

Powered by blists - more mailing lists