Em Tue, Nov 14, 2017 at 02:58:24PM -0800, Yonghong Song escreveu:

On 11/14/17 12:25 PM, Daniel Borkmann wrote:

quoted

Yeah, I know, that's what I mentioned earlier in this thread to resolve it,
but do we really want to add this hack everywhere? :( Potentially any function
having ARG_CONST_SIZE would need to handle size 0 and bail out again in their
helper implementation and it ends up that progs start relying on this runtime
check where we won't be able to get rid of it later on anymore.

The compiler actually does the right thing for the below code:
         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));

Just from the above code without consulting bpf_probe_read_str internals, it
is totally possible that ret = 128, then
ret & (sizeof(filename) - 1) = 0.

The issue is that the verifier did not set the "ret" initial range as (-inf,
sizeof(filename) - 1). We could have this information associated with helper
and feed back to verifier.

If we have this range, later for ret & (sizeof(filename) - 1) with ret >= 1,
the verifier should be able to conclude
 ret & (sizeof(filename) - 1) >= 1.

To workaround the immediate problem, I tested the following hack
with bcc and it works fine.

BPF_PERF_OUTPUT(events);
int trace(struct pt_regs *ctx) {
  char filename[128];
  int ret = bpf_probe_read_str(filename, sizeof(filename), 0);
  if (ret > 0) {
    if (ret == 1)
          events.perf_submit(ctx, filename, ret);
    else if (ret < 128)
          events.perf_submit(ctx, filename, ret);
  }
  return 1;
}

The idea is to make control flow more complex to prevent llvm
do certain optimizations.

So, the hack makes it work for me, using clang 6.0:

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+/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 -

[root@jouet bpf]# perf probe -V do_sys_open
Available variables at do_sys_open
        @<do_sys_open+0>
                char*   filename
                int     dfd
                int     flags
                struct open_flags       op
                umode_t mode
[root@jouet bpf]# cat open.c 
#include "bpf.h"

SEC("prog=do_sys_open filename")
int prog(void *ctx, int err, char *filename_ptr)
{
	char filename[128];
	int len = bpf_probe_read_str(filename, sizeof(filename), filename_ptr); 
	if (len > 0) {
		if (len == 1)
       			perf_event_output(ctx, &__bpf_stdout__, BPF_F_CURRENT_CPU, filename, len);
		else if (len < 128)
       			perf_event_output(ctx, &__bpf_stdout__, BPF_F_CURRENT_CPU, filename, len);
	}
	return 1;
}
[root@jouet bpf]#
[root@jouet bpf]# perf trace -e *open,open.c touch /tmp/Thanks.Yonghong.Song\!
LLVM: dumping open.o
     0.000 ( 0.009 ms): touch/9034 open(filename: 0x5b678e37, flags: CLOEXEC                             ) ...
     0.009 (         ): __bpf_stdout__:/etc/ld.so.cache....)
     0.011 (         ): perf_bpf_probe:prog:(ffffffff8f260da0) filename=0x7f805b678e37)
     0.000 ( 0.016 ms): touch/9034  ... [continued]: open()) = 3
     0.034 ( 0.002 ms): touch/9034 open(filename: 0x5b87c640, flags: CLOEXEC                             ) ...
     0.036 (         ): __bpf_stdout__:/lib64/libc.so.6....)
     0.037 (         ): perf_bpf_probe:prog:(ffffffff8f260da0) filename=0x7f805b87c640)
     0.034 ( 0.009 ms): touch/9034  ... [continued]: open()) = 3
     0.251 ( 0.002 ms): touch/9034 open(filename: 0x5b422c70, flags: CLOEXEC                             ) ...
     0.253 (         ): __bpf_stdout__:/usr/lib/locale/locale-archive......)
     0.254 (         ): perf_bpf_probe:prog:(ffffffff8f260da0) filename=0x7f805b422c70)
     0.251 ( 0.009 ms): touch/9034  ... [continued]: open()) = 3
     0.296 ( 0.002 ms): touch/9034 open(filename: 0x1d3a00f1, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) ...
     0.298 (         ): __bpf_stdout__:/tmp/Thanks.Yonghong.Song!..)
     0.299 (         ): perf_bpf_probe:prog:(ffffffff8f260da0) filename=0x7ffd1d3a00f1)
     0.296 ( 0.009 ms): touch/9034  ... [continued]: open()) = 3
[root@jouet bpf]#