From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Yonghong Song <yhs@fb.com>
Cc: Gianluca Borello <g.borello@gmail.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Alexei Starovoitov <ast@kernel.org>,
David Miller <davem@davemloft.net>,
Linux Networking Development Mailing List
<netdev@vger.kernel.org>
Subject: Re: len = bpf_probe_read_str(); bpf_perf_event_output(... len) == FAIL
Date: Mon, 22 Jan 2018 17:52:48 -0300 [thread overview]
Message-ID: <20180122205248.GA25043@kernel.org> (raw)
In-Reply-To: <871019a0-71f8-c26d-0ae8-c7fd8c8867fc@fb.com>
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
next prev parent reply other threads:[~2018-01-22 20:52 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-13 14:30 len = bpf_probe_read_str(); bpf_perf_event_output(... len) == FAIL Arnaldo Carvalho de Melo
2017-11-13 14:56 ` Daniel Borkmann
2017-11-13 15:08 ` Arnaldo Carvalho de Melo
2017-11-14 0:09 ` Daniel Borkmann
2017-11-14 12:58 ` Arnaldo Carvalho de Melo
2017-11-14 13:09 ` Daniel Borkmann
2017-11-14 13:42 ` Arnaldo Carvalho de Melo
2017-11-14 14:19 ` Daniel Borkmann
2017-11-14 14:58 ` Arnaldo Carvalho de Melo
2017-11-14 18:15 ` Yonghong Song
2017-11-14 20:25 ` Daniel Borkmann
2017-11-14 22:58 ` Yonghong Song
2017-11-21 14:29 ` Arnaldo Carvalho de Melo
2017-11-21 22:31 ` Alexei Starovoitov
2017-11-22 18:42 ` Gianluca Borello
2018-01-22 15:06 ` Arnaldo Carvalho de Melo
2018-01-22 18:28 ` Yonghong Song
2018-01-22 20:52 ` Arnaldo Carvalho de Melo [this message]
2017-11-20 13:31 ` Arnaldo Carvalho de Melo
2017-11-20 16:47 ` Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180122205248.GA25043@kernel.org \
--to=acme@kernel.org \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=g.borello@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).