Em Tue, Nov 14, 2017 at 02:09:34PM +0100, Daniel Borkmann escreveu:

On 11/14/2017 01:58 PM, Arnaldo Carvalho de Melo wrote:

quoted

Em Tue, Nov 14, 2017 at 01:09:39AM +0100, Daniel Borkmann escreveu:

quoted

On 11/13/2017 04:08 PM, Arnaldo Carvalho de Melo wrote:

quoted

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

I changed to int, didn't help
 

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

[root@jouet bpf]# llc --version
LLVM (http://llvm.org/):
  LLVM version 4.0.0svn

Old stuff! ;-) Will change, but improving these messages should be on
the radar, I think :-)

- Arnaldo
 

  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

quoted

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