netdev - Re: len = bpf_probe_read_str(); bpf_perf_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180122205248.GA25043@kernel.org>
Date:   Mon, 22 Jan 2018 17:52:48 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Yonghong Song <yhs@...com>
Cc:     Gianluca Borello <g.borello@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        David Miller <davem@...emloft.net>,
        Linux Networking Development Mailing List 
        <netdev@...r.kernel.org>
Subject: Re: len = bpf_probe_read_str(); bpf_perf_event_output(... len) ==
 FAIL

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@...et 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@...et 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@...et bpf]# 

- Arnaldo