netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).