From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnaldo Carvalho de Melo Subject: Re: len = bpf_probe_read_str(); bpf_perf_event_output(... len) == FAIL Date: Mon, 22 Jan 2018 17:52:48 -0300 Message-ID: <20180122205248.GA25043@kernel.org> References: <20171114134229.GO8836@kernel.org> <32dd93e5-d119-dc9d-1222-b53c31a13b35@fb.com> <0f68d96b-c19d-d154-411b-313acf07d62c@iogearbox.net> <0839587a-9520-c844-61a3-01a7a30f0015@fb.com> <20171121142905.GJ7918@kernel.org> <20171121223157.v3nodaugpuzl5dwg@ast-mbp.dhcp.thefacebook.com> <20180122150644.GJ18383@kernel.org> <871019a0-71f8-c26d-0ae8-c7fd8c8867fc@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Gianluca Borello , Daniel Borkmann , Alexei Starovoitov , David Miller , Linux Networking Development Mailing List To: Yonghong Song Return-path: Received: from mail.kernel.org ([198.145.29.99]:38452 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750973AbeAVUww (ORCPT ); Mon, 22 Jan 2018 15:52:52 -0500 Content-Disposition: inline In-Reply-To: <871019a0-71f8-c26d-0ae8-c7fd8c8867fc@fb.com> Sender: netdev-owner@vger.kernel.org List-ID: Em Mon, Jan 22, 2018 at 10:28:11AM -0800, Yonghong Song escreveu: > The compiler did "40: (bf) r1 = r0" and then uses "r1" for branch > comparison, the original "r0" is left with complete unknown integer value > and later used to calculate the buffer size "55: (bf) r5 = r0" > where "r5" could be negative value and the verifier rightfully > complains. > There is no easy way to fix this in verifier unless verifier starts to track > correlations between registers which is a big task. So your below workaround > is okay. The below workaround should also work: > int len = bpf_probe_read_str(filename.path, sizeof(filename.path), > filename.ptr); > if (len > 0 && len < 256) > bpf_perf_event_output(ctx, &my_map, BPF_F_CURRENT_CPU, > &filename, (len & 0xff) + sizeof(filename.ptr)); > return 0; Ok, thanks for one more time doing the analysis of the optimizations emitted and suggesting something more compact, that I can confirm works: [root@jouet bpf]# perf trace -a -e open,sys_enter_open.c sleep 0.1 LLVM: dumping sys_enter_open.o 1.212 ( ): __bpf_stdout__:......../usr/lib/locale/locale-archive......) 1.218 ( 0.021 ms): sleep/9872 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC) = 3 2.905 ( ): __bpf_stdout__:..:.F.../usr/lib/locale/locale-archive......) 2.910 ( 0.013 ms): rm/9873 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC) = 3 7.562 ( ): __bpf_stdout__:..ul..../usr/lib/locale/locale-archive......) 7.564 ( 0.013 ms): mv/9874 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC) = 3 11.275 ( ): __bpf_stdout__:...d..../usr/lib/locale/locale-archive......) 11.278 ( 0.012 ms): sh/9875 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC) = 3 11.945 ( ): __bpf_stdout__:...d..../usr/lib64/gconv/gconv-modules.cache........) 11.953 ( 0.018 ms): sh/9875 open(filename: /usr/lib64/gconv/gconv-modules.cache) = 3 17.906 ( ): __bpf_stdout__:..T.p.../usr/lib/locale/locale-archive......) 17.913 ( 0.319 ms): gcc/9877 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC) = 4 18.389 ( ): __bpf_stdout__:...l..../usr/share/locale/locale.alias......) 18.394 ( 0.266 ms): gcc/9877 open(filename: /usr/share/locale/locale.alias, flags: CLOEXEC) = 4 18.777 ( ): __bpf_stdout__:@......./usr/share/locale/en_US.UTF-8/LC_MESSAGES/gcc.mo....) 18.782 ( 0.318 ms): gcc/9877 open(filename: /usr/share/locale/en_US.UTF-8/LC_MESSAGES/gcc.mo, mode: IFBLK|IFIFO|ISGID|ISVTX|IRUSR|IXUSR|0xb5cc0000) = -1 ENOENT No such file or directory [root@jouet bpf]# cat sys_enter_open.c #include "bpf.h" SEC("syscalls:sys_enter_open") int func(void *ctx) { struct { char *ptr; char path[256]; } filename = { .ptr = *((char **)(ctx + 16)), }; int len = bpf_probe_read_str(filename.path, sizeof(filename.path), filename.ptr); if (len > 0 && len < 256) perf_event_output(ctx, &__bpf_stdout__, BPF_F_CURRENT_CPU, &filename, (len & 0xff) + sizeof(filename.ptr)); return 0; } [root@jouet bpf]# - Arnaldo