* Re: [PATCH bpf] bpf: reject passing modified ctx to helper functions
From: Alexei Starovoitov @ 2018-06-07 19:43 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev, ecree
In-Reply-To: <20180607154003.5368-1-daniel@iogearbox.net>
On Thu, Jun 07, 2018 at 05:40:03PM +0200, Daniel Borkmann wrote:
> As commit 28e33f9d78ee ("bpf: disallow arithmetic operations on
> context pointer") already describes, f1174f77b50c ("bpf/verifier:
> rework value tracking") removed the specific white-listed cases
> we had previously where we would allow for pointer arithmetic in
> order to further generalize it, and allow e.g. context access via
> modified registers. While the dereferencing of modified context
> pointers had been forbidden through 28e33f9d78ee, syzkaller did
> recently manage to trigger several KASAN splats for slab out of
> bounds access and use after frees by simply passing a modified
> context pointer to a helper function which would then do the bad
> access since verifier allowed it in adjust_ptr_min_max_vals().
>
> Rejecting arithmetic on ctx pointer in adjust_ptr_min_max_vals()
> generally could break existing programs as there's a valid use
> case in tracing in combination with passing the ctx to helpers as
> bpf_probe_read(), where the register then becomes unknown at
> verification time due to adding a non-constant offset to it. An
> access sequence may look like the following:
>
> offset = args->filename; /* field __data_loc filename */
> bpf_probe_read(&dst, len, (char *)args + offset); // args is ctx
>
> There are two options: i) we could special case the ctx and as
> soon as we add a constant or bounded offset to it (hence ctx type
> wouldn't change) we could turn the ctx into an unknown scalar, or
> ii) we generalize the sanity test for ctx member access into a
> small helper and assert it on the ctx register that was passed
> as a function argument. Fwiw, latter is more obvious and less
> complex at the same time, and one case that may potentially be
> legitimate in future for ctx member access at least would be for
> ctx to carry a const offset. Therefore, fix follows approach
> from ii) and adds test cases to BPF kselftests.
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Reported-by: syzbot+3d0b2441dbb71751615e@syzkaller.appspotmail.com
> Reported-by: syzbot+c8504affd4fdd0c1b626@syzkaller.appspotmail.com
> Reported-by: syzbot+e5190cb881d8660fb1a3@syzkaller.appspotmail.com
> Reported-by: syzbot+efae31b384d5badbd620@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Applied, Thanks
^ permalink raw reply
* [PATCH] netfilter: remove include/net/netfilter/nft_dup.h
From: Corentin Labbe @ 2018-06-07 19:38 UTC (permalink / raw)
To: davem, fw, kadlec, pablo
Cc: coreteam, linux-kernel, netdev, netfilter-devel, Corentin Labbe
include/net/netfilter/nft_dup.h was introduced in d877f07112f1 ("netfilter: nf_tables: add nft_dup expression")
but was never user since this date.
Furthermore, the only struct in this file is unused elsewhere.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
include/net/netfilter/nft_dup.h | 10 ----------
1 file changed, 10 deletions(-)
delete mode 100644 include/net/netfilter/nft_dup.h
diff --git a/include/net/netfilter/nft_dup.h b/include/net/netfilter/nft_dup.h
deleted file mode 100644
index 4d9d512984b2..000000000000
--- a/include/net/netfilter/nft_dup.h
+++ /dev/null
@@ -1,10 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _NFT_DUP_H_
-#define _NFT_DUP_H_
-
-struct nft_dup_inet {
- enum nft_registers sreg_addr:8;
- enum nft_registers sreg_dev:8;
-};
-
-#endif /* _NFT_DUP_H_ */
--
2.16.4
^ permalink raw reply related
* Re: [PATCH bpf-next v5 00/10] BTF: BPF Type Format
From: Arnaldo Carvalho de Melo @ 2018-06-07 19:30 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: netdev, Alexei Starovoitov, Daniel Borkmann, kernel-team,
Wang Nan, Jiri Olsa, Namhyung Kim, Ingo Molnar
In-Reply-To: <20180607190510.qtj7gq4wbfewcn4g@kafai-mbp>
Em Thu, Jun 07, 2018 at 12:05:10PM -0700, Martin KaFai Lau escreveu:
> On Thu, Jun 07, 2018 at 11:03:37AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jun 07, 2018 at 10:54:01AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Tue, Jun 05, 2018 at 02:25:48PM -0700, Martin KaFai Lau escreveu:
> > > > [ btw, the latest commit (1 commit) should be 94a11b59e592 ].
> > So, the commit log message for the pahole patch is non-existent:
> > https://github.com/iamkafai/pahole/commit/94a11b59e5920908085bfc8d24c92f95c8ffceaf
> > we should do better in describing what is done and how, I'm staring
> > with a message you sent to the kernel part:
> > --
> > This patch introduces BPF Type Format (BTF).
> > BTF (BPF Type Format) is the meta data format which describes
> > the data types of BPF program/map. Hence, it basically focus
> > on the C programming language which the modern BPF is primary
> > using. The first use case is to provide a generic pretty print
> > capability for a BPF map.
> I will add details in the next github respin/push.
Ok, but I can do that if there is nothing else to do in the code at this
stage :-)
> > Now I'm going to do the step-by-step guide on testing the feature just
> > introduced, and will try to convert from dwarf to BTF and back, compare
> > the pahole output for types encoded in DWARF and BTF, etc.
> > If you have something ressembling this already, please share.
> The pahole only has the encoder part. I tested with the verbose output
> from the "pahole -V -J". Loading the btf to the kernel is also tested.
Ok, so here it goes my first stab at testing, using perf's BPF
integration:
[root@jouet bpf]# cat hello.c
#include "stdio.h"
int syscall_enter(openat)(void *ctx)
{
puts("Hello, world\n");
return 0;
}
[root@jouet bpf]# cat ~/.perfconfig
[llvm]
dump-obj = true
[root@jouet bpf]# perf trace -e open*,hello.c touch /tmp/hello.BTF
LLVM: dumping hello.o
0.017 ( ): __bpf_stdout__:Hello, world
0.019 ( 0.011 ms): touch/28147 openat(dfd: CWD, filename: /etc/ld.so.cache, flags: CLOEXEC ) = 3
0.053 ( ): __bpf_stdout__:Hello, world
0.055 ( 0.011 ms): touch/28147 openat(dfd: CWD, filename: /lib64/libc.so.6, flags: CLOEXEC ) = 3
0.354 ( 0.012 ms): touch/28147 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC ) = 3
0.411 ( ): __bpf_stdout__:Hello, world
0.412 ( 0.198 ms): touch/28147 openat(dfd: CWD, filename: /tmp/hello.BTF, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
[root@jouet bpf]#
[root@jouet bpf]# file hello.o
hello.o: ELF 64-bit LSB relocatable, *unknown arch 0xf7* version 1 (SYSV), not stripped
[root@jouet bpf]# pahole --btf_encode hello.o
pahole: hello.o: No debugging information found
[root@jouet bpf]#
[root@jouet bpf]# readelf -s hello.o
Symbol table '.symtab' contains 5 entries:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 7 __bpf_stdout__
2: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 5 _license
3: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 6 _version
4: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 3 syscall_enter_openat
[root@jouet bpf]#
[root@jouet bpf]# readelf -SW hello.o
There are 10 section headers, starting at offset 0x1f8:
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 0] NULL 0000000000000000 000000 000000 00 0 0 0
[ 1] .strtab STRTAB 0000000000000000 000178 00007f 00 0 0 1
[ 2] .text PROGBITS 0000000000000000 000040 000000 00 AX 0 0 4
[ 3] syscalls:sys_enter_openat PROGBITS 0000000000000000 000040 000088 00 AX 0 0 8
[ 4] .relsyscalls:sys_enter_openat REL 0000000000000000 000168 000010 10 9 3 8
[ 5] license PROGBITS 0000000000000000 0000c8 000004 00 WA 0 0 1
[ 6] version PROGBITS 0000000000000000 0000cc 000004 00 WA 0 0 4
[ 7] maps PROGBITS 0000000000000000 0000d0 000010 00 WA 0 0 4
[ 8] .rodata.str1.1 PROGBITS 0000000000000000 0000e0 00000e 01 AMS 0 0 1
[ 9] .symtab SYMTAB 0000000000000000 0000f0 000078 18 1 1 8
Key to Flags:
W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
L (link order), O (extra OS processing required), G (group), T (TLS),
C (compressed), x (unknown), o (OS specific), E (exclude),
p (processor specific)
[root@jouet bpf]#
Humm, lemme try something, add -g to clang-opt:
[root@jouet bpf]# cat ~/.perfconfig
[llvm]
dump-obj = true
clang-opt = -g
[root@jouet bpf]# perf trace -e open*,hello.c touch /tmp/hello.BTF
LLVM: dumping hello.o
0.185 ( ): __bpf_stdout__:Hello, world
0.188 ( 0.009 ms): touch/19670 openat(dfd: CWD, filename: /etc/ld.so.cache, flags: CLOEXEC ) = 3
0.219 ( ): __bpf_stdout__:Hello, world
0.220 ( 0.011 ms): touch/19670 openat(dfd: CWD, filename: /lib64/libc.so.6, flags: CLOEXEC ) = 3
0.480 ( 0.095 ms): touch/19670 open(filename: /usr/lib/locale/locale-archive, flags: CLOEXEC ) = 3
0.624 ( ): __bpf_stdout__:Hello, world
0.626 ( 0.011 ms): touch/19670 openat(dfd: CWD, filename: /tmp/hello.BTF, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
[root@jouet bpf]# file hello.o
hello.o: ELF 64-bit LSB relocatable, *unknown arch 0xf7* version 1 (SYSV), with debug_info, not stripped
[root@jouet bpf]#
Much better:
[root@jouet bpf]# readelf -SW hello.o
There are 25 section headers, starting at offset 0xc70:
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 0] NULL 0000000000000000 000000 000000 00 0 0 0
[ 1] .strtab STRTAB 0000000000000000 000b50 000119 00 0 0 1
[ 2] .text PROGBITS 0000000000000000 000040 000000 00 AX 0 0 4
[ 3] syscalls:sys_enter_openat PROGBITS 0000000000000000 000040 000088 00 AX 0 0 8
[ 4] .relsyscalls:sys_enter_openat REL 0000000000000000 000910 000010 10 24 3 8
[ 5] license PROGBITS 0000000000000000 0000c8 000004 00 WA 0 0 1
[ 6] version PROGBITS 0000000000000000 0000cc 000004 00 WA 0 0 4
[ 7] maps PROGBITS 0000000000000000 0000d0 000010 00 WA 0 0 4
[ 8] .rodata.str1.1 PROGBITS 0000000000000000 0000e0 00000e 01 AMS 0 0 1
[ 9] .debug_str PROGBITS 0000000000000000 0000ee 00010e 01 MS 0 0 1
[10] .debug_loc PROGBITS 0000000000000000 0001fc 000023 00 0 0 1
[11] .debug_abbrev PROGBITS 0000000000000000 00021f 0000e3 00 0 0 1
[12] .debug_info PROGBITS 0000000000000000 000302 00015e 00 0 0 1
[13] .rel.debug_info REL 0000000000000000 000920 0001e0 10 24 12 8
[14] .debug_ranges PROGBITS 0000000000000000 000460 000030 00 0 0 1
[15] .debug_macinfo PROGBITS 0000000000000000 000490 000001 00 0 0 1
[16] .debug_pubnames PROGBITS 0000000000000000 000491 00006e 00 0 0 1
[17] .rel.debug_pubnames REL 0000000000000000 000b00 000010 10 24 16 8
[18] .debug_pubtypes PROGBITS 0000000000000000 0004ff 00005a 00 0 0 1
[19] .rel.debug_pubtypes REL 0000000000000000 000b10 000010 10 24 18 8
[20] .debug_frame PROGBITS 0000000000000000 000560 000028 00 0 0 8
[21] .rel.debug_frame REL 0000000000000000 000b20 000020 10 24 20 8
[22] .debug_line PROGBITS 0000000000000000 000588 00006e 00 0 0 1
[23] .rel.debug_line REL 0000000000000000 000b40 000010 10 24 22 8
[24] .symtab SYMTAB 0000000000000000 0005f8 000318 18 1 29 8
Key to Flags:
W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
L (link order), O (extra OS processing required), G (group), T (TLS),
C (compressed), x (unknown), o (OS specific), E (exclude),
p (processor specific)
[root@jouet bpf]#
[root@jouet bpf]# readelf -s hello.o
Symbol table '.symtab' contains 33 entries:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000000000 0 NOTYPE LOCAL DEFAULT 9
2: 000000000000002d 0 NOTYPE LOCAL DEFAULT 9
3: 0000000000000044 0 NOTYPE LOCAL DEFAULT 9
4: 0000000000000053 0 NOTYPE LOCAL DEFAULT 9
5: 000000000000005c 0 NOTYPE LOCAL DEFAULT 9
6: 0000000000000061 0 NOTYPE LOCAL DEFAULT 9
7: 000000000000006a 0 NOTYPE LOCAL DEFAULT 9
8: 0000000000000073 0 NOTYPE LOCAL DEFAULT 9
9: 0000000000000077 0 NOTYPE LOCAL DEFAULT 9
10: 0000000000000086 0 NOTYPE LOCAL DEFAULT 9
11: 000000000000008b 0 NOTYPE LOCAL DEFAULT 9
12: 0000000000000098 0 NOTYPE LOCAL DEFAULT 9
13: 00000000000000a1 0 NOTYPE LOCAL DEFAULT 9
14: 00000000000000ac 0 NOTYPE LOCAL DEFAULT 9
15: 00000000000000b8 0 NOTYPE LOCAL DEFAULT 9
16: 00000000000000c4 0 NOTYPE LOCAL DEFAULT 9
17: 00000000000000d6 0 NOTYPE LOCAL DEFAULT 9
18: 00000000000000e8 0 NOTYPE LOCAL DEFAULT 9
19: 00000000000000fd 0 NOTYPE LOCAL DEFAULT 9
20: 0000000000000101 0 NOTYPE LOCAL DEFAULT 9
21: 0000000000000107 0 NOTYPE LOCAL DEFAULT 9
22: 0000000000000000 0 SECTION LOCAL DEFAULT 3
23: 0000000000000000 0 SECTION LOCAL DEFAULT 10
24: 0000000000000000 0 SECTION LOCAL DEFAULT 11
25: 0000000000000000 0 SECTION LOCAL DEFAULT 12
26: 0000000000000000 0 SECTION LOCAL DEFAULT 14
27: 0000000000000000 0 SECTION LOCAL DEFAULT 20
28: 0000000000000000 0 SECTION LOCAL DEFAULT 22
29: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 7 __bpf_stdout__
30: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 5 _license
31: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 6 _version
32: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 3 syscall_enter_openat
[root@jouet bpf]#
Progress:
[root@jouet bpf]# pahole --btf_encode hello.o
sh: llvm-objcopy: command not found
Failed to encode BTF
[root@jouet bpf]#
[acme@jouet perf]$ llvm-
llvm-ar llvm-cov llvm-dis llvm-lib llvm-modextract llvm-profdata llvm-split
llvm-as llvm-c-test llvm-dlltool llvm-link llvm-mt llvm-ranlib llvm-stress
llvm-bcanalyzer llvm-cvtres llvm-dsymutil llvm-lto llvm-nm llvm-readelf llvm-strings
llvm-cat llvm-cxxdump llvm-dwarfdump llvm-lto2 llvm-objdump llvm-readobj llvm-symbolizer
llvm-config llvm-cxxfilt llvm-dwp llvm-mc llvm-opt-report llvm-rtdyld llvm-tblgen
llvm-config-64 llvm-diff llvm-extract llvm-mcmarkup llvm-pdbutil llvm-size llvm-xray
[acme@jouet perf]$ llvm-
So this must be available in a newer llvm version? Which one?
[root@jouet bpf]# clang -v
clang version 5.0.1 (tags/RELEASE_501/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/7
Selected GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
[root@jouet bpf]#
[acme@jouet perf]$ rpm -q clang llvm
clang-5.0.1-5.fc27.x86_64
llvm-5.0.1-6.fc27.x86_64
[acme@jouet perf]$
[root@jouet bpf]# clang -v
clang version 5.0.1 (tags/RELEASE_501/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/7
Selected GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
[root@jouet bpf]#
[acme@jouet perf]$ rpm -q clang llvm
clang-5.0.1-5.fc27.x86_64
llvm-5.0.1-6.fc27.x86_64
[acme@jouet perf]$
- Arnaldo
^ permalink raw reply
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: Michael S. Tsirkin @ 2018-06-07 19:29 UTC (permalink / raw)
To: Al Viro
Cc: syzbot, avagin, davem, dingtianhong, edumazet, elena.reshetova,
jasowang, kvm, linux-kernel, matthew, mingo, netdev, pabeni,
syzkaller-bugs, virtualization, willemb
In-Reply-To: <20180607180449.GS30522@ZenIV.linux.org.uk>
On Thu, Jun 07, 2018 at 07:04:49PM +0100, Al Viro wrote:
> On Thu, Jun 07, 2018 at 08:59:06PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote:
> > > On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote:
> > > > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617
> > > >
> > > > Subject: vhost: fix info leak
> > > >
> > > > Fixes: CVE-2018-1118
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > index f0be5f35ab28..9beefa6ed1ce 100644
> > > > --- a/drivers/vhost/vhost.c
> > > > +++ b/drivers/vhost/vhost.c
> > > > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> > > > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> > > > if (!node)
> > > > return NULL;
> > > > +
> > > > + /* Make sure all padding within the structure is initialized. */
> > > > + memset(&node->msg, 0, sizeof node->msg);
> > >
> > > Umm... Maybe kzalloc(), then? You have
> > >
> > > struct vhost_msg_node {
> > > struct vhost_msg msg;
> > > struct vhost_virtqueue *vq;
> > > struct list_head node;
> > > };
> > >
> > > and that's what, 68 bytes in msg, then either 4 bytes pointer or
> > > 4 bytes padding + 8 bytes pointer, then two pointers? How much
> > > does explicit partial memset() save you here?
> >
> > Yes but 0 isn't a nop here so if this struct is used without
> > a sensible initialization, it will crash elsewhere.
> > I prefer KASAN to catch such uses.
> >
> >
> > > > node->vq = vq;
> > > > node->msg.type = type;
>
> IDGI - what would your variant catch that kzalloc + 2 assignments won't?
> Accesses to uninitialized ->node? Because that's the only difference in
> what is and is not initialized between those variants...
For now yes but we'll likely add more fields in this structure
down the road, which is where I'd expect new bugs to come from.
--
MST
^ permalink raw reply
* Re: nfp: bpf: perf event output helpers support
From: Kees Cook @ 2018-06-07 19:13 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Quentin Monnet, Daniel Borkmann, Network Development, LKML
In-Reply-To: <20180607093340.1742a5d1@cakuba.netronome.com>
On Thu, Jun 7, 2018 at 9:33 AM, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
> On Wed, 6 Jun 2018 22:15:04 -0700, Kees Cook wrote:
>> > + rcu_read_lock();
>> > + if (!rhashtable_lookup_fast(&bpf->maps_neutral, &map,
>> > + nfp_bpf_maps_neutral_params)) {
>> > + rcu_read_unlock();
>> > + pr_warn("perf event: dest map pointer %px not recognized, dropping event\n",
>> > + map);
>>
>> Please don't use %px on kernel pointers unless you absolutely have
>> to[1]. It seems like this value wouldn't be actionable here, so likely
>> it's best to just remove its use entirely.
>
> We're using kernel pointer as an opaque handle when communicating with
> the device. We need the actual value to correlate things. Maybe I used
> the %px slightly out of spite there, because I forgot %p is now useless
> and wasted half an hour on debugging an endian issue :S
>
> This message can only really trigger when root loads a specific BPF map
> onto the device and FW is buggy. Can I fix it in -next? I'm making
> changes to this part of the code anyway.
That'd be fine by me, thanks!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH] selftests: bpf: fix urandom_read build issue
From: Anders Roxell @ 2018-06-07 19:07 UTC (permalink / raw)
To: Y Song
Cc: Alexei Starovoitov, Daniel Borkmann, Shuah Khan, netdev, LKML,
linux-kselftest
In-Reply-To: <CAH3MdRWo0kUYngyAwXP1sOT_XqbxyfTvDd6sNz+qd7LL1T8LUA@mail.gmail.com>
On 7 June 2018 at 19:52, Y Song <ys114321@gmail.com> wrote:
> On Thu, Jun 7, 2018 at 3:57 AM, Anders Roxell <anders.roxell@linaro.org> wrote:
>> gcc complains that urandom_read gets built twice.
>>
>> gcc -o tools/testing/selftests/bpf/urandom_read
>> -static urandom_read.c -Wl,--build-id
>> gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../lib/bpf
>> -I../../../../include/generated -I../../../include urandom_read.c
>> urandom_read -lcap -lelf -lrt -lpthread -o
>> tools/testing/selftests/bpf/urandom_read
>> gcc: fatal error: input file
>> ‘tools/testing/selftests/bpf/urandom_read’ is the
>> same as output file
>> compilation terminated.
>> ../lib.mk:110: recipe for target
>> 'tools/testing/selftests/bpf/urandom_read' failed
>
> What is the build/make command to reproduce the above failure?
make -C tools/testing/selftests
Cheers,
Anders
>
>> To fix this issue remove the urandom_read target and so target
>> TEST_CUSTOM_PROGS gets used.
>>
>> Fixes: 81f77fd0deeb ("bpf: add selftest for stackmap with BPF_F_STACK_BUILD_ID")
>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>> ---
>> tools/testing/selftests/bpf/Makefile | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 607ed8729c06..67285591ffd7 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -16,10 +16,8 @@ LDLIBS += -lcap -lelf -lrt -lpthread
>> TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
>> all: $(TEST_CUSTOM_PROGS)
>>
>> -$(TEST_CUSTOM_PROGS): urandom_read
>> -
>> -urandom_read: urandom_read.c
>> - $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
>> +$(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
>> + $(CC) -o $@ -static $< -Wl,--build-id
>>
>> # Order correspond to 'make run_tests' order
>> TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH bpf-next v5 00/10] BTF: BPF Type Format
From: Martin KaFai Lau @ 2018-06-07 19:05 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: netdev, Alexei Starovoitov, Daniel Borkmann, kernel-team
In-Reply-To: <20180607140337.GF30317@kernel.org>
On Thu, Jun 07, 2018 at 11:03:37AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 07, 2018 at 10:54:01AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Jun 05, 2018 at 02:25:48PM -0700, Martin KaFai Lau escreveu:
> > > [ btw, the latest commit (1 commit) should be 94a11b59e592 ].
>
> So, the commit log message for the pahole patch is non-existent:
>
> https://github.com/iamkafai/pahole/commit/94a11b59e5920908085bfc8d24c92f95c8ffceaf
>
> we should do better in describing what is done and how, I'm staring
> with a message you sent to the kernel part:
>
> --
> This patch introduces BPF Type Format (BTF).
>
> BTF (BPF Type Format) is the meta data format which describes
> the data types of BPF program/map. Hence, it basically focus
> on the C programming language which the modern BPF is primary
> using. The first use case is to provide a generic pretty print
> capability for a BPF map.
> --
I will add details in the next github respin/push.
>
> Now I'm going to do the step-by-step guide on testing the feature just
> introduced, and will try to convert from dwarf to BTF and back, compare
> the pahole output for types encoded in DWARF and BTF, etc.
>
> If you have something ressembling this already, please share.
The pahole only has the encoder part. I tested with the verbose output
from the "pahole -V -J". Loading the btf to the kernel is also tested.
^ permalink raw reply
* Re: INFO: task hung in ip6gre_exit_batch_net
From: Dmitry Vyukov @ 2018-06-07 19:03 UTC (permalink / raw)
To: Kirill Tkhai
Cc: syzbot, Christian Brauner, David Miller, David Ahern,
Florian Westphal, Jiri Benc, LKML, Xin Long, mschiffer, netdev,
syzkaller-bugs, Vladislav Yasevich
In-Reply-To: <04b6ee08-7919-bf2d-5b77-bd346a0bff48@virtuozzo.com>
On Thu, Jun 7, 2018 at 8:54 PM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>>>>> Hi, Dmirty!
>>>>>
>>>>> On 04.06.2018 18:22, Dmitry Vyukov wrote:
>>>>>> On Mon, Jun 4, 2018 at 5:03 PM, syzbot
>>>>>> <syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com> wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> syzbot found the following crash on:
>>>>>>>
>>>>>>> HEAD commit: bc2dbc5420e8 Merge branch 'akpm' (patches from Andrew)
>>>>>>> git tree: upstream
>>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=164e42b7800000
>>>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=982e2df1b9e60b02
>>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=bf78a74f82c1cf19069e
>>>>>>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>>>>>>
>>>>>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>>>>>
>>>>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>>>>> Reported-by: syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com
>>>>>>
>>>>>> Another hang on rtnl lock:
>>>>>>
>>>>>> #syz dup: INFO: task hung in netdev_run_todo
>>>>>>
>>>>>> May be related to "unregister_netdevice: waiting for DEV to become free":
>>>>>> https://syzkaller.appspot.com/bug?id=1a97a5bd119fd97995f752819fd87840ab9479a9
>>>>
>>>> netdev_wait_allrefs does not hold rtnl lock during waiting, so it must
>>>> be something different.
>>>>
>>>>
>>>>>> Any other explanations for massive hangs on rtnl lock for minutes?
>>>>>
>>>>> To exclude the situation, when a task exists with rtnl_mutex held:
>>>>>
>>>>> would the pr_warn() from print_held_locks_bug() be included in the console output
>>>>> if they appear?
>>>>
>>>> Yes, everything containing "WARNING:" is detected as bug.
>>>
>>> OK, then dead task not releasing the lock is excluded.
>>>
>>> One more assumption: someone corrupted memory around rtnl_mutex and it looks like locked.
>>> (I track lockdep "(rtnl_mutex){+.+.}" prints in initial message as "nobody owns rtnl_mutex").
>>> There may help a crash dump of the VM.
>>
>> I can't find any legend for these +'s and .'s, but {+.+.} is present
>> in large amounts in just any task hung report for different mutexes,
>> so I would not expect that it means corruption.
>>
>> Are dozens of known corruptions that syzkaller can trigger. But
>> usually they are reliably caught by KASAN. If any of them would lead
>> to silent memory corruption, we would got dozens of assorted crashes
>> throughout the kernel. We've seen that at some points, but not
>> recently. So I would assume that memory is not corrupted in all these
>> cases:
>> https://syzkaller.appspot.com/bug?id=2503c576cabb08d41812e732b390141f01a59545
>
> This BUG clarifies the {+.+.}:
>
> 4 locks held by kworker/0:145/381:
> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] work_static include/linux/workqueue.h:198 [inline]
> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] set_work_data kernel/workqueue.c:619 [inline]
> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] set_work_pool_and_clear_pending kernel/workqueue.c:646 [inline]
> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] process_one_work+0xb12/0x1bb0 kernel/workqueue.c:2084
> #1: ((work_completion)(&data->destroy_work)){+.+.}, at: [<00000000bbdd2115>] process_one_work+0xb89/0x1bb0 kernel/workqueue.c:2088
> #2: (rtnl_mutex){+.+.}, at: [<000000009c9d14f8>] rtnl_lock+0x17/0x20 net/core/rtnetlink.c:74
> #3: (rcu_sched_state.exp_mutex){+.+.}, at: [<000000001ba1a807>] exp_funnel_lock kernel/rcu/tree_exp.h:272 [inline]
> #3: (rcu_sched_state.exp_mutex){+.+.}, at: [<000000001ba1a807>] _synchronize_rcu_expedited.constprop.72+0x9fa/0xac0 kernel/rcu/tree_exp.h:596
>
> There we have rtnl_mutex locked and the {..} is like above. It's definitely locked
> since there is one more lock after it.
>
> This BUG happen because of there are many rtnl_mutex waiters while owner
> is synchronizing RCU. Rather clear for me in comparison to the topic's hung.
You mean that it's not hanged, but rather needs more than 2 minutes to
complete, right?
>> I wonder if it can be just that slow, but not actually hanged... net
>> namespace destruction is super slow, so perhaps under heavy load it
>> all stalls for minutes...
^ permalink raw reply
* Re: INFO: task hung in ip6gre_exit_batch_net
From: Kirill Tkhai @ 2018-06-07 18:54 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: syzbot, Christian Brauner, David Miller, David Ahern,
Florian Westphal, Jiri Benc, LKML, Xin Long, mschiffer, netdev,
syzkaller-bugs, Vladislav Yasevich
In-Reply-To: <CACT4Y+YBY2yGJBHgqGhAcOguag9JUkGkeOz1acgKd=KR3q+noQ@mail.gmail.com>
On 07.06.2018 21:23, Dmitry Vyukov wrote:
> On Tue, Jun 5, 2018 at 3:55 PM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>> On 05.06.2018 12:36, Dmitry Vyukov wrote:
>>> On Tue, Jun 5, 2018 at 11:03 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>>>> Hi, Dmirty!
>>>>
>>>> On 04.06.2018 18:22, Dmitry Vyukov wrote:
>>>>> On Mon, Jun 4, 2018 at 5:03 PM, syzbot
>>>>> <syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com> wrote:
>>>>>> Hello,
>>>>>>
>>>>>> syzbot found the following crash on:
>>>>>>
>>>>>> HEAD commit: bc2dbc5420e8 Merge branch 'akpm' (patches from Andrew)
>>>>>> git tree: upstream
>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=164e42b7800000
>>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=982e2df1b9e60b02
>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=bf78a74f82c1cf19069e
>>>>>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>>>>>
>>>>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>>>>
>>>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>>>> Reported-by: syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com
>>>>>
>>>>> Another hang on rtnl lock:
>>>>>
>>>>> #syz dup: INFO: task hung in netdev_run_todo
>>>>>
>>>>> May be related to "unregister_netdevice: waiting for DEV to become free":
>>>>> https://syzkaller.appspot.com/bug?id=1a97a5bd119fd97995f752819fd87840ab9479a9
>>>
>>> netdev_wait_allrefs does not hold rtnl lock during waiting, so it must
>>> be something different.
>>>
>>>
>>>>> Any other explanations for massive hangs on rtnl lock for minutes?
>>>>
>>>> To exclude the situation, when a task exists with rtnl_mutex held:
>>>>
>>>> would the pr_warn() from print_held_locks_bug() be included in the console output
>>>> if they appear?
>>>
>>> Yes, everything containing "WARNING:" is detected as bug.
>>
>> OK, then dead task not releasing the lock is excluded.
>>
>> One more assumption: someone corrupted memory around rtnl_mutex and it looks like locked.
>> (I track lockdep "(rtnl_mutex){+.+.}" prints in initial message as "nobody owns rtnl_mutex").
>> There may help a crash dump of the VM.
>
> I can't find any legend for these +'s and .'s, but {+.+.} is present
> in large amounts in just any task hung report for different mutexes,
> so I would not expect that it means corruption.
>
> Are dozens of known corruptions that syzkaller can trigger. But
> usually they are reliably caught by KASAN. If any of them would lead
> to silent memory corruption, we would got dozens of assorted crashes
> throughout the kernel. We've seen that at some points, but not
> recently. So I would assume that memory is not corrupted in all these
> cases:
> https://syzkaller.appspot.com/bug?id=2503c576cabb08d41812e732b390141f01a59545
This BUG clarifies the {+.+.}:
4 locks held by kworker/0:145/381:
#0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] work_static include/linux/workqueue.h:198 [inline]
#0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] set_work_data kernel/workqueue.c:619 [inline]
#0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] set_work_pool_and_clear_pending kernel/workqueue.c:646 [inline]
#0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] process_one_work+0xb12/0x1bb0 kernel/workqueue.c:2084
#1: ((work_completion)(&data->destroy_work)){+.+.}, at: [<00000000bbdd2115>] process_one_work+0xb89/0x1bb0 kernel/workqueue.c:2088
#2: (rtnl_mutex){+.+.}, at: [<000000009c9d14f8>] rtnl_lock+0x17/0x20 net/core/rtnetlink.c:74
#3: (rcu_sched_state.exp_mutex){+.+.}, at: [<000000001ba1a807>] exp_funnel_lock kernel/rcu/tree_exp.h:272 [inline]
#3: (rcu_sched_state.exp_mutex){+.+.}, at: [<000000001ba1a807>] _synchronize_rcu_expedited.constprop.72+0x9fa/0xac0 kernel/rcu/tree_exp.h:596
There we have rtnl_mutex locked and the {..} is like above. It's definitely locked
since there is one more lock after it.
This BUG happen because of there are many rtnl_mutex waiters while owner
is synchronizing RCU. Rather clear for me in comparison to the topic's hung.
> I wonder if it can be just that slow, but not actually hanged... net
> namespace destruction is super slow, so perhaps under heavy load it
> all stalls for minutes...
Thanks,
Kirill
^ permalink raw reply
* Re: INFO: task hung in ip6gre_exit_batch_net
From: Dmitry Vyukov @ 2018-06-07 18:23 UTC (permalink / raw)
To: Kirill Tkhai
Cc: syzbot, Christian Brauner, David Miller, David Ahern,
Florian Westphal, Jiri Benc, LKML, Xin Long, mschiffer, netdev,
syzkaller-bugs, Vladislav Yasevich
In-Reply-To: <e71c0df1-d83c-030c-7c97-13a923aca1b3@virtuozzo.com>
On Tue, Jun 5, 2018 at 3:55 PM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> On 05.06.2018 12:36, Dmitry Vyukov wrote:
>> On Tue, Jun 5, 2018 at 11:03 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>>> Hi, Dmirty!
>>>
>>> On 04.06.2018 18:22, Dmitry Vyukov wrote:
>>>> On Mon, Jun 4, 2018 at 5:03 PM, syzbot
>>>> <syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com> wrote:
>>>>> Hello,
>>>>>
>>>>> syzbot found the following crash on:
>>>>>
>>>>> HEAD commit: bc2dbc5420e8 Merge branch 'akpm' (patches from Andrew)
>>>>> git tree: upstream
>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=164e42b7800000
>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=982e2df1b9e60b02
>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=bf78a74f82c1cf19069e
>>>>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>>>>
>>>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>>>
>>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>>> Reported-by: syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com
>>>>
>>>> Another hang on rtnl lock:
>>>>
>>>> #syz dup: INFO: task hung in netdev_run_todo
>>>>
>>>> May be related to "unregister_netdevice: waiting for DEV to become free":
>>>> https://syzkaller.appspot.com/bug?id=1a97a5bd119fd97995f752819fd87840ab9479a9
>>
>> netdev_wait_allrefs does not hold rtnl lock during waiting, so it must
>> be something different.
>>
>>
>>>> Any other explanations for massive hangs on rtnl lock for minutes?
>>>
>>> To exclude the situation, when a task exists with rtnl_mutex held:
>>>
>>> would the pr_warn() from print_held_locks_bug() be included in the console output
>>> if they appear?
>>
>> Yes, everything containing "WARNING:" is detected as bug.
>
> OK, then dead task not releasing the lock is excluded.
>
> One more assumption: someone corrupted memory around rtnl_mutex and it looks like locked.
> (I track lockdep "(rtnl_mutex){+.+.}" prints in initial message as "nobody owns rtnl_mutex").
> There may help a crash dump of the VM.
I can't find any legend for these +'s and .'s, but {+.+.} is present
in large amounts in just any task hung report for different mutexes,
so I would not expect that it means corruption.
Are dozens of known corruptions that syzkaller can trigger. But
usually they are reliably caught by KASAN. If any of them would lead
to silent memory corruption, we would got dozens of assorted crashes
throughout the kernel. We've seen that at some points, but not
recently. So I would assume that memory is not corrupted in all these
cases:
https://syzkaller.appspot.com/bug?id=2503c576cabb08d41812e732b390141f01a59545
I wonder if it can be just that slow, but not actually hanged... net
namespace destruction is super slow, so perhaps under heavy load it
all stalls for minutes...
> Also, there may be a locking code BUG, but this seems the least probable for me.
>
> Kirill
^ permalink raw reply
* KASAN: null-ptr-deref Write in xdp_umem_unaccount_pages
From: syzbot @ 2018-06-07 18:17 UTC (permalink / raw)
To: bjorn.topel, davem, linux-kernel, magnus.karlsson, netdev,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 1c8c5a9d38f6 Merge git://git.kernel.org/pub/scm/linux/kern..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13a72bdf800000
kernel config: https://syzkaller.appspot.com/x/.config?x=4f1acdf888c9d4e9
dashboard link: https://syzkaller.appspot.com/bug?extid=979217770b09ebf5c407
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=12aca2af800000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=161d4ddf800000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+979217770b09ebf5c407@syzkaller.appspotmail.com
RDX: 0000000000000004 RSI: 000000000000011b RDI: 0000000000000004
RBP: 00000000006cb018 R08: 0000000000000018 R09: 00007fffc4750032
R10: 0000000020000040 R11: 0000000000000246 R12: 0000000000000005
R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
==================================================================
BUG: KASAN: null-ptr-deref in atomic64_sub
include/asm-generic/atomic-instrumented.h:144 [inline]
BUG: KASAN: null-ptr-deref in atomic_long_sub
include/asm-generic/atomic-long.h:199 [inline]
BUG: KASAN: null-ptr-deref in xdp_umem_unaccount_pages.isra.4+0x3d/0x80
net/xdp/xdp_umem.c:135
Write of size 8 at addr 0000000000000060 by task syz-executor246/4527
CPU: 1 PID: 4527 Comm: syz-executor246 Not tainted 4.17.0+ #89
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1b9/0x294 lib/dump_stack.c:113
kasan_report_error mm/kasan/report.c:352 [inline]
kasan_report.cold.7+0x6d/0x2fe mm/kasan/report.c:412
check_memory_region_inline mm/kasan/kasan.c:260 [inline]
check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
kasan_check_write+0x14/0x20 mm/kasan/kasan.c:278
atomic64_sub include/asm-generic/atomic-instrumented.h:144 [inline]
atomic_long_sub include/asm-generic/atomic-long.h:199 [inline]
xdp_umem_unaccount_pages.isra.4+0x3d/0x80 net/xdp/xdp_umem.c:135
xdp_umem_reg net/xdp/xdp_umem.c:334 [inline]
xdp_umem_create+0xd6c/0x10f0 net/xdp/xdp_umem.c:349
xsk_setsockopt+0x443/0x550 net/xdp/xsk.c:531
__sys_setsockopt+0x1bd/0x390 net/socket.c:1935
__do_sys_setsockopt net/socket.c:1946 [inline]
__se_sys_setsockopt net/socket.c:1943 [inline]
__x64_sys_setsockopt+0xbe/0x150 net/socket.c:1943
do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440549
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 5b 14 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fffc475d008 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000440549
RDX: 0000000000000004 RSI: 000000000000011b RDI: 0000000000000004
RBP: 00000000006cb018 R08: 0000000000000018 R09: 00007fffc4750032
R10: 0000000020000040 R11: 0000000000000246 R12: 0000000000000005
R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] bpf: attach type BPF_LIRC_MODE2 should not depend on CONFIG_BPF_CGROUP
From: Y Song @ 2018-06-07 18:14 UTC (permalink / raw)
To: Sean Young
Cc: Daniel Borkmann, Matthias Reichl, linux-media, LKML,
Alexei Starovoitov, Mauro Carvalho Chehab, netdev,
Devin Heitmueller, Quentin Monnet
In-Reply-To: <20180606210939.q3vviyc4b2h6gu3c@gofer.mess.org>
On Wed, Jun 6, 2018 at 2:09 PM, Sean Young <sean@mess.org> wrote:
> Compile bpf_prog_{attach,detach,query} even if CONFIG_BPF_CGROUP is not
> set.
It should be CONFIG_CGROUP_BPF here. The same for subject line.
Today, if CONFIG_CGROUP_BPF is not defined. Users will get an -EINVAL
if they try to attach/detach/query.
I am not sure what is the motivation behind this change. Could you explain more?
>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> include/linux/bpf-cgroup.h | 31 +++++++++++
> kernel/bpf/cgroup.c | 110 +++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 105 ++---------------------------------
> 3 files changed, 145 insertions(+), 101 deletions(-)
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 975fb4cf1bb7..ee67cd35f426 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -188,12 +188,43 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> \
> __ret; \
> })
> +int sockmap_get_from_fd(const union bpf_attr *attr, int type, bool attach);
> +int cgroup_bpf_prog_attach(const union bpf_attr *attr,
> + enum bpf_prog_type ptype);
> +int cgroup_bpf_prog_detach(const union bpf_attr *attr,
> + enum bpf_prog_type ptype);
> +int cgroup_bpf_prog_query(const union bpf_attr *attr,
> + union bpf_attr __user *uattr);
> #else
>
> struct cgroup_bpf {};
> static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
> static inline int cgroup_bpf_inherit(struct cgroup *cgrp) { return 0; }
>
> +static inline int sockmap_get_from_fd(const union bpf_attr *attr,
> + int type, bool attach)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int cgroup_bpf_prog_attach(const union bpf_attr *attr,
> + enum bpf_prog_type ptype)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int cgroup_bpf_prog_detach(const union bpf_attr *attr,
> + enum bpf_prog_type ptype)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int cgroup_bpf_prog_query(const union bpf_attr *attr,
> + union bpf_attr __user *uattr)
> +{
> + return -EINVAL;
> +}
> +
> #define cgroup_bpf_enabled (0)
> #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)
> #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk,skb) ({ 0; })
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index f7c00bd6f8e4..d6e18f9dc0c4 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -428,6 +428,116 @@ int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
> return ret;
> }
>
> +int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> + enum bpf_attach_type attach_type)
> +{
> + switch (prog->type) {
> + case BPF_PROG_TYPE_CGROUP_SOCK:
> + case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> + return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> + default:
> + return 0;
> + }
> +}
> +
> +int sockmap_get_from_fd(const union bpf_attr *attr, int type, bool attach)
> +{
> + struct bpf_prog *prog = NULL;
> + int ufd = attr->target_fd;
> + struct bpf_map *map;
> + struct fd f;
> + int err;
> +
> + f = fdget(ufd);
> + map = __bpf_map_get(f);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + if (attach) {
> + prog = bpf_prog_get_type(attr->attach_bpf_fd, type);
> + if (IS_ERR(prog)) {
> + fdput(f);
> + return PTR_ERR(prog);
> + }
> + }
> +
> + err = sock_map_prog(map, prog, attr->attach_type);
> + if (err) {
> + fdput(f);
> + if (prog)
> + bpf_prog_put(prog);
> + return err;
> + }
> +
> + fdput(f);
> + return 0;
> +}
> +
> +int cgroup_bpf_prog_attach(const union bpf_attr *attr, enum bpf_prog_type ptype)
> +{
> + struct bpf_prog *prog;
> + struct cgroup *cgrp;
> + int ret;
> +
> + prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
> + if (IS_ERR(prog))
> + return PTR_ERR(prog);
> +
> + if (bpf_prog_attach_check_attach_type(prog, attr->attach_type)) {
> + bpf_prog_put(prog);
> + return -EINVAL;
> + }
> +
> + cgrp = cgroup_get_from_fd(attr->target_fd);
> + if (IS_ERR(cgrp)) {
> + bpf_prog_put(prog);
> + return PTR_ERR(cgrp);
> + }
> +
> + ret = cgroup_bpf_attach(cgrp, prog, attr->attach_type,
> + attr->attach_flags);
> + if (ret)
> + bpf_prog_put(prog);
> + cgroup_put(cgrp);
> +
> + return ret;
> +}
> +
> +int cgroup_bpf_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype)
> +{
> + struct bpf_prog *prog;
> + struct cgroup *cgrp;
> + int ret;
> +
> + cgrp = cgroup_get_from_fd(attr->target_fd);
> + if (IS_ERR(cgrp))
> + return PTR_ERR(cgrp);
> +
> + prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
> + if (IS_ERR(prog))
> + prog = NULL;
> +
> + ret = cgroup_bpf_detach(cgrp, prog, attr->attach_type, 0);
> + if (prog)
> + bpf_prog_put(prog);
> + cgroup_put(cgrp);
> + return ret;
> +}
> +
> +int cgroup_bpf_prog_query(const union bpf_attr *attr,
> + union bpf_attr __user *uattr)
> +{
> + struct cgroup *cgrp;
> + int ret;
> +
> + cgrp = cgroup_get_from_fd(attr->query.target_fd);
> + if (IS_ERR(cgrp))
> + return PTR_ERR(cgrp);
> + ret = cgroup_bpf_query(cgrp, attr, uattr);
> + cgroup_put(cgrp);
> + return ret;
> +}
> +
> /**
> * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering
> * @sk: The socket sending or receiving traffic
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0fa20624707f..52fa44856623 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1489,65 +1489,14 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
> return err;
> }
>
> -#ifdef CONFIG_CGROUP_BPF
> -
> -static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> - enum bpf_attach_type attach_type)
> -{
> - switch (prog->type) {
> - case BPF_PROG_TYPE_CGROUP_SOCK:
> - case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> - return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> - default:
> - return 0;
> - }
> -}
> -
> #define BPF_PROG_ATTACH_LAST_FIELD attach_flags
>
> -static int sockmap_get_from_fd(const union bpf_attr *attr,
> - int type, bool attach)
> -{
> - struct bpf_prog *prog = NULL;
> - int ufd = attr->target_fd;
> - struct bpf_map *map;
> - struct fd f;
> - int err;
> -
> - f = fdget(ufd);
> - map = __bpf_map_get(f);
> - if (IS_ERR(map))
> - return PTR_ERR(map);
> -
> - if (attach) {
> - prog = bpf_prog_get_type(attr->attach_bpf_fd, type);
> - if (IS_ERR(prog)) {
> - fdput(f);
> - return PTR_ERR(prog);
> - }
> - }
> -
> - err = sock_map_prog(map, prog, attr->attach_type);
> - if (err) {
> - fdput(f);
> - if (prog)
> - bpf_prog_put(prog);
> - return err;
> - }
> -
> - fdput(f);
> - return 0;
> -}
> -
> #define BPF_F_ATTACH_MASK \
> (BPF_F_ALLOW_OVERRIDE | BPF_F_ALLOW_MULTI)
>
> static int bpf_prog_attach(const union bpf_attr *attr)
> {
> enum bpf_prog_type ptype;
> - struct bpf_prog *prog;
> - struct cgroup *cgrp;
> - int ret;
>
> if (!capable(CAP_NET_ADMIN))
> return -EPERM;
> @@ -1593,28 +1542,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> return -EINVAL;
> }
>
> - prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
> - if (IS_ERR(prog))
> - return PTR_ERR(prog);
> -
> - if (bpf_prog_attach_check_attach_type(prog, attr->attach_type)) {
> - bpf_prog_put(prog);
> - return -EINVAL;
> - }
> -
> - cgrp = cgroup_get_from_fd(attr->target_fd);
> - if (IS_ERR(cgrp)) {
> - bpf_prog_put(prog);
> - return PTR_ERR(cgrp);
> - }
> -
> - ret = cgroup_bpf_attach(cgrp, prog, attr->attach_type,
> - attr->attach_flags);
> - if (ret)
> - bpf_prog_put(prog);
> - cgroup_put(cgrp);
> -
> - return ret;
> + return cgroup_bpf_prog_attach(attr, ptype);
> }
>
> #define BPF_PROG_DETACH_LAST_FIELD attach_type
> @@ -1622,9 +1550,6 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> static int bpf_prog_detach(const union bpf_attr *attr)
> {
> enum bpf_prog_type ptype;
> - struct bpf_prog *prog;
> - struct cgroup *cgrp;
> - int ret;
>
> if (!capable(CAP_NET_ADMIN))
> return -EPERM;
> @@ -1667,19 +1592,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> return -EINVAL;
> }
>
> - cgrp = cgroup_get_from_fd(attr->target_fd);
> - if (IS_ERR(cgrp))
> - return PTR_ERR(cgrp);
> -
> - prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
> - if (IS_ERR(prog))
> - prog = NULL;
> -
> - ret = cgroup_bpf_detach(cgrp, prog, attr->attach_type, 0);
> - if (prog)
> - bpf_prog_put(prog);
> - cgroup_put(cgrp);
> - return ret;
> + return cgroup_bpf_prog_detach(attr, ptype);
> }
>
> #define BPF_PROG_QUERY_LAST_FIELD query.prog_cnt
> @@ -1687,9 +1600,6 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> static int bpf_prog_query(const union bpf_attr *attr,
> union bpf_attr __user *uattr)
> {
> - struct cgroup *cgrp;
> - int ret;
> -
> if (!capable(CAP_NET_ADMIN))
> return -EPERM;
> if (CHECK_ATTR(BPF_PROG_QUERY))
> @@ -1717,14 +1627,9 @@ static int bpf_prog_query(const union bpf_attr *attr,
> default:
> return -EINVAL;
> }
> - cgrp = cgroup_get_from_fd(attr->query.target_fd);
> - if (IS_ERR(cgrp))
> - return PTR_ERR(cgrp);
> - ret = cgroup_bpf_query(cgrp, attr, uattr);
> - cgroup_put(cgrp);
> - return ret;
> +
> + return cgroup_bpf_prog_query(attr, uattr);
> }
> -#endif /* CONFIG_CGROUP_BPF */
>
> #define BPF_PROG_TEST_RUN_LAST_FIELD test.duration
>
> @@ -2371,7 +2276,6 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> case BPF_OBJ_GET:
> err = bpf_obj_get(&attr);
> break;
> -#ifdef CONFIG_CGROUP_BPF
> case BPF_PROG_ATTACH:
> err = bpf_prog_attach(&attr);
> break;
> @@ -2381,7 +2285,6 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> case BPF_PROG_QUERY:
> err = bpf_prog_query(&attr, uattr);
> break;
> -#endif
> case BPF_PROG_TEST_RUN:
> err = bpf_prog_test_run(&attr, uattr);
> break;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] bpf: attach type BPF_LIRC_MODE2 should not depend on CONFIG_BPF_CGROUP
From: Simon Horman @ 2018-06-07 18:12 UTC (permalink / raw)
To: Sean Young
Cc: Daniel Borkmann, Matthias Reichl, linux-media, linux-kernel,
Alexei Starovoitov, Mauro Carvalho Chehab, netdev,
Devin Heitmueller, Y Song, Quentin Monnet
In-Reply-To: <20180606210939.q3vviyc4b2h6gu3c@gofer.mess.org>
On Wed, Jun 06, 2018 at 10:09:40PM +0100, Sean Young wrote:
> Compile bpf_prog_{attach,detach,query} even if CONFIG_BPF_CGROUP is not
> set.
>
> Signed-off-by: Sean Young <sean@mess.org>
Hi Sean,
This patch seems to involve rather extensive refactoring, the details of
which are not explained, in order to make a change whose motivation is also
not explained. I for one would value a more extensive changelog if
not a series of patches which implement discrete changes.
Thanks!
^ permalink raw reply
* Re: [PATCH 1/3] net: phy: Check phy_driver ready before accessing
From: Andrew Lunn @ 2018-06-07 18:12 UTC (permalink / raw)
To: Brandon Maier
Cc: netdev, f.fainelli, davem, michal.simek, Clayton Shotwell,
Kristopher Cory, linux-kernel
In-Reply-To: <CA+fik51YC14k8yMLRkybMXQNn6uWB0u6aQUd1GMZrQtOF=cd3A@mail.gmail.com>
> Agreed. Another thing that looks suspicious to me is the driver
> overrides the private data of the device it's attaching too, in the
> `priv->phy_dev->priv = priv` bit. Seems like that could cause all
> sorts of driver corruption problems.
Ah, yes. That is very broken. Many PHYs will just explode sometime
later, since they use phdev->priv.
> But fixing that is going to require more drastic changes to how this
> driver works. So it'd be worth applying this patch in the mean time.
Patches welcome.
Andrew
^ permalink raw reply
* Re: [PATCH bpf] bpf: reject passing modified ctx to helper functions
From: Edward Cree @ 2018-06-07 18:09 UTC (permalink / raw)
To: Daniel Borkmann, ast; +Cc: netdev
In-Reply-To: <20180607154003.5368-1-daniel@iogearbox.net>
On 07/06/18 16:40, Daniel Borkmann wrote:
> As commit 28e33f9d78ee ("bpf: disallow arithmetic operations on
> context pointer") already describes, f1174f77b50c ("bpf/verifier:
> rework value tracking") removed the specific white-listed cases
> we had previously where we would allow for pointer arithmetic in
> order to further generalize it, and allow e.g. context access via
> modified registers. While the dereferencing of modified context
> pointers had been forbidden through 28e33f9d78ee, syzkaller did
> recently manage to trigger several KASAN splats for slab out of
> bounds access and use after frees by simply passing a modified
> context pointer to a helper function which would then do the bad
> access since verifier allowed it in adjust_ptr_min_max_vals().
>
> Rejecting arithmetic on ctx pointer in adjust_ptr_min_max_vals()
> generally could break existing programs as there's a valid use
> case in tracing in combination with passing the ctx to helpers as
> bpf_probe_read(), where the register then becomes unknown at
> verification time due to adding a non-constant offset to it. An
> access sequence may look like the following:
>
> offset = args->filename; /* field __data_loc filename */
> bpf_probe_read(&dst, len, (char *)args + offset); // args is ctx
>
> There are two options: i) we could special case the ctx and as
> soon as we add a constant or bounded offset to it (hence ctx type
> wouldn't change) we could turn the ctx into an unknown scalar, or
> ii) we generalize the sanity test for ctx member access into a
> small helper and assert it on the ctx register that was passed
> as a function argument. Fwiw, latter is more obvious and less
> complex at the same time, and one case that may potentially be
> legitimate in future for ctx member access at least would be for
> ctx to carry a const offset. Therefore, fix follows approach
> from ii) and adds test cases to BPF kselftests.
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Reported-by: syzbot+3d0b2441dbb71751615e@syzkaller.appspotmail.com
> Reported-by: syzbot+c8504affd4fdd0c1b626@syzkaller.appspotmail.com
> Reported-by: syzbot+e5190cb881d8660fb1a3@syzkaller.appspotmail.com
> Reported-by: syzbot+efae31b384d5badbd620@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Edward Cree <ecree@solarflare.com>
^ permalink raw reply
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: Al Viro @ 2018-06-07 18:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: syzbot, avagin, davem, dingtianhong, edumazet, elena.reshetova,
jasowang, kvm, linux-kernel, matthew, mingo, netdev, pabeni,
syzkaller-bugs, virtualization, willemb
In-Reply-To: <20180607205611-mutt-send-email-mst@kernel.org>
On Thu, Jun 07, 2018 at 08:59:06PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote:
> > On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote:
> > > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617
> > >
> > > Subject: vhost: fix info leak
> > >
> > > Fixes: CVE-2018-1118
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index f0be5f35ab28..9beefa6ed1ce 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> > > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> > > if (!node)
> > > return NULL;
> > > +
> > > + /* Make sure all padding within the structure is initialized. */
> > > + memset(&node->msg, 0, sizeof node->msg);
> >
> > Umm... Maybe kzalloc(), then? You have
> >
> > struct vhost_msg_node {
> > struct vhost_msg msg;
> > struct vhost_virtqueue *vq;
> > struct list_head node;
> > };
> >
> > and that's what, 68 bytes in msg, then either 4 bytes pointer or
> > 4 bytes padding + 8 bytes pointer, then two pointers? How much
> > does explicit partial memset() save you here?
>
> Yes but 0 isn't a nop here so if this struct is used without
> a sensible initialization, it will crash elsewhere.
> I prefer KASAN to catch such uses.
>
>
> > > node->vq = vq;
> > > node->msg.type = type;
IDGI - what would your variant catch that kzalloc + 2 assignments won't?
Accesses to uninitialized ->node? Because that's the only difference in
what is and is not initialized between those variants...
^ permalink raw reply
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: Michael S. Tsirkin @ 2018-06-07 17:59 UTC (permalink / raw)
To: Al Viro
Cc: syzbot, avagin, davem, dingtianhong, edumazet, elena.reshetova,
jasowang, kvm, linux-kernel, matthew, mingo, netdev, pabeni,
syzkaller-bugs, virtualization, willemb
In-Reply-To: <20180607174355.GR30522@ZenIV.linux.org.uk>
On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote:
> On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote:
> > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617
> >
> > Subject: vhost: fix info leak
> >
> > Fixes: CVE-2018-1118
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index f0be5f35ab28..9beefa6ed1ce 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> > struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> > if (!node)
> > return NULL;
> > +
> > + /* Make sure all padding within the structure is initialized. */
> > + memset(&node->msg, 0, sizeof node->msg);
>
> Umm... Maybe kzalloc(), then? You have
>
> struct vhost_msg_node {
> struct vhost_msg msg;
> struct vhost_virtqueue *vq;
> struct list_head node;
> };
>
> and that's what, 68 bytes in msg, then either 4 bytes pointer or
> 4 bytes padding + 8 bytes pointer, then two pointers? How much
> does explicit partial memset() save you here?
Yes but 0 isn't a nop here so if this struct is used without
a sensible initialization, it will crash elsewhere.
I prefer KASAN to catch such uses.
> > node->vq = vq;
> > node->msg.type = type;
> > return node;
^ permalink raw reply
* Re: [PATCH] selftests: bpf: fix urandom_read build issue
From: Y Song @ 2018-06-07 17:52 UTC (permalink / raw)
To: Anders Roxell
Cc: Alexei Starovoitov, Daniel Borkmann, Shuah Khan, netdev, LKML,
linux-kselftest
In-Reply-To: <20180607105712.553-1-anders.roxell@linaro.org>
On Thu, Jun 7, 2018 at 3:57 AM, Anders Roxell <anders.roxell@linaro.org> wrote:
> gcc complains that urandom_read gets built twice.
>
> gcc -o tools/testing/selftests/bpf/urandom_read
> -static urandom_read.c -Wl,--build-id
> gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../lib/bpf
> -I../../../../include/generated -I../../../include urandom_read.c
> urandom_read -lcap -lelf -lrt -lpthread -o
> tools/testing/selftests/bpf/urandom_read
> gcc: fatal error: input file
> ‘tools/testing/selftests/bpf/urandom_read’ is the
> same as output file
> compilation terminated.
> ../lib.mk:110: recipe for target
> 'tools/testing/selftests/bpf/urandom_read' failed
What is the build/make command to reproduce the above failure?
> To fix this issue remove the urandom_read target and so target
> TEST_CUSTOM_PROGS gets used.
>
> Fixes: 81f77fd0deeb ("bpf: add selftest for stackmap with BPF_F_STACK_BUILD_ID")
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
> tools/testing/selftests/bpf/Makefile | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 607ed8729c06..67285591ffd7 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -16,10 +16,8 @@ LDLIBS += -lcap -lelf -lrt -lpthread
> TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
> all: $(TEST_CUSTOM_PROGS)
>
> -$(TEST_CUSTOM_PROGS): urandom_read
> -
> -urandom_read: urandom_read.c
> - $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
> +$(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
> + $(CC) -o $@ -static $< -Wl,--build-id
>
> # Order correspond to 'make run_tests' order
> TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH bpf] bpf: reject passing modified ctx to helper functions
From: Y Song @ 2018-06-07 17:45 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev, Edward Cree
In-Reply-To: <20180607154003.5368-1-daniel@iogearbox.net>
On Thu, Jun 7, 2018 at 8:40 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> As commit 28e33f9d78ee ("bpf: disallow arithmetic operations on
> context pointer") already describes, f1174f77b50c ("bpf/verifier:
> rework value tracking") removed the specific white-listed cases
> we had previously where we would allow for pointer arithmetic in
> order to further generalize it, and allow e.g. context access via
> modified registers. While the dereferencing of modified context
> pointers had been forbidden through 28e33f9d78ee, syzkaller did
> recently manage to trigger several KASAN splats for slab out of
> bounds access and use after frees by simply passing a modified
> context pointer to a helper function which would then do the bad
> access since verifier allowed it in adjust_ptr_min_max_vals().
>
> Rejecting arithmetic on ctx pointer in adjust_ptr_min_max_vals()
> generally could break existing programs as there's a valid use
> case in tracing in combination with passing the ctx to helpers as
> bpf_probe_read(), where the register then becomes unknown at
> verification time due to adding a non-constant offset to it. An
> access sequence may look like the following:
>
> offset = args->filename; /* field __data_loc filename */
> bpf_probe_read(&dst, len, (char *)args + offset); // args is ctx
>
> There are two options: i) we could special case the ctx and as
> soon as we add a constant or bounded offset to it (hence ctx type
> wouldn't change) we could turn the ctx into an unknown scalar, or
> ii) we generalize the sanity test for ctx member access into a
> small helper and assert it on the ctx register that was passed
> as a function argument. Fwiw, latter is more obvious and less
> complex at the same time, and one case that may potentially be
> legitimate in future for ctx member access at least would be for
> ctx to carry a const offset. Therefore, fix follows approach
> from ii) and adds test cases to BPF kselftests.
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Reported-by: syzbot+3d0b2441dbb71751615e@syzkaller.appspotmail.com
> Reported-by: syzbot+c8504affd4fdd0c1b626@syzkaller.appspotmail.com
> Reported-by: syzbot+e5190cb881d8660fb1a3@syzkaller.appspotmail.com
> Reported-by: syzbot+efae31b384d5badbd620@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Tested with bcc for the above ctx + unknown_offset usage for bpf_probe_read.
No breakage.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> kernel/bpf/verifier.c | 48 +++++++++++++++---------
> tools/testing/selftests/bpf/test_verifier.c | 58 ++++++++++++++++++++++++++++-
> 2 files changed, 88 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d6403b5..cced0c1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1617,6 +1617,30 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env,
> }
> #endif
>
> +static int check_ctx_reg(struct bpf_verifier_env *env,
> + const struct bpf_reg_state *reg, int regno)
> +{
> + /* Access to ctx or passing it to a helper is only allowed in
> + * its original, unmodified form.
> + */
> +
> + if (reg->off) {
> + verbose(env, "dereference of modified ctx ptr R%d off=%d disallowed\n",
> + regno, reg->off);
> + return -EACCES;
> + }
> +
> + if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
> + char tn_buf[48];
> +
> + tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
> + verbose(env, "variable ctx access var_off=%s disallowed\n", tn_buf);
> + return -EACCES;
> + }
> +
> + return 0;
> +}
> +
> /* truncate register to smaller size (in bytes)
> * must be called with size < BPF_REG_SIZE
> */
> @@ -1686,24 +1710,11 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> verbose(env, "R%d leaks addr into ctx\n", value_regno);
> return -EACCES;
> }
> - /* ctx accesses must be at a fixed offset, so that we can
> - * determine what type of data were returned.
> - */
> - if (reg->off) {
> - verbose(env,
> - "dereference of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
> - regno, reg->off, off - reg->off);
> - return -EACCES;
> - }
> - if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
> - char tn_buf[48];
>
> - tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
> - verbose(env,
> - "variable ctx access var_off=%s off=%d size=%d",
> - tn_buf, off, size);
> - return -EACCES;
> - }
> + err = check_ctx_reg(env, reg, regno);
> + if (err < 0)
> + return err;
> +
> err = check_ctx_access(env, insn_idx, off, size, t, ®_type);
> if (!err && t == BPF_READ && value_regno >= 0) {
> /* ctx access returns either a scalar, or a
> @@ -1984,6 +1995,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> expected_type = PTR_TO_CTX;
> if (type != expected_type)
> goto err_type;
> + err = check_ctx_reg(env, reg, regno);
> + if (err < 0)
> + return err;
> } else if (arg_type_is_mem_ptr(arg_type)) {
> expected_type = PTR_TO_STACK;
> /* One exception here. In case function allows for NULL to be
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 7cb1d74..2ecd27b 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -8647,7 +8647,7 @@ static struct bpf_test tests[] = {
> offsetof(struct __sk_buff, mark)),
> BPF_EXIT_INSN(),
> },
> - .errstr = "dereference of modified ctx ptr R1 off=68+8, ctx+const is allowed, ctx+const+const is not",
> + .errstr = "dereference of modified ctx ptr",
> .result = REJECT,
> .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> },
> @@ -12258,6 +12258,62 @@ static struct bpf_test tests[] = {
> .result = ACCEPT,
> .retval = 5,
> },
> + {
> + "pass unmodified ctx pointer to helper",
> + .insns = {
> + BPF_MOV64_IMM(BPF_REG_2, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_csum_update),
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .result = ACCEPT,
> + },
> + {
> + "pass modified ctx pointer to helper, 1",
> + .insns = {
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -612),
> + BPF_MOV64_IMM(BPF_REG_2, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_csum_update),
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .result = REJECT,
> + .errstr = "dereference of modified ctx ptr",
> + },
> + {
> + "pass modified ctx pointer to helper, 2",
> + .insns = {
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -612),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_get_socket_cookie),
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .result_unpriv = REJECT,
> + .result = REJECT,
> + .errstr_unpriv = "dereference of modified ctx ptr",
> + .errstr = "dereference of modified ctx ptr",
> + },
> + {
> + "pass modified ctx pointer to helper, 3",
> + .insns = {
> + BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, 0),
> + BPF_ALU64_IMM(BPF_AND, BPF_REG_3, 4),
> + BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
> + BPF_MOV64_IMM(BPF_REG_2, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_csum_update),
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .result = REJECT,
> + .errstr = "variable ctx access var_off=(0x0; 0x4)",
> + },
> };
>
> static int probe_filter_length(const struct bpf_insn *fp)
> --
> 2.9.5
>
^ permalink raw reply
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: Al Viro @ 2018-06-07 17:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: syzbot, avagin, davem, dingtianhong, edumazet, elena.reshetova,
jasowang, kvm, linux-kernel, matthew, mingo, netdev, pabeni,
syzkaller-bugs, virtualization, willemb
In-Reply-To: <20180607183627-mutt-send-email-mst@kernel.org>
On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote:
> #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617
>
> Subject: vhost: fix info leak
>
> Fixes: CVE-2018-1118
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index f0be5f35ab28..9beefa6ed1ce 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> if (!node)
> return NULL;
> +
> + /* Make sure all padding within the structure is initialized. */
> + memset(&node->msg, 0, sizeof node->msg);
Umm... Maybe kzalloc(), then? You have
struct vhost_msg_node {
struct vhost_msg msg;
struct vhost_virtqueue *vq;
struct list_head node;
};
and that's what, 68 bytes in msg, then either 4 bytes pointer or
4 bytes padding + 8 bytes pointer, then two pointers? How much
does explicit partial memset() save you here?
> node->vq = vq;
> node->msg.type = type;
> return node;
^ permalink raw reply
* Re: [PATCH 2/3] net: phy: xgmiitorgmii: Use correct mdio bus
From: Brandon Maier @ 2018-06-07 17:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, f.fainelli, davem, michal.simek, Clayton Shotwell,
Kristopher Cory, linux-kernel
In-Reply-To: <20180607164920.GC25513@lunn.ch>
On Thu, Jun 7, 2018 at 11:49 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> I think the assumption was they are both on the same bus.
> However, they don't need to be.
That was my assumption as well. We ran into a scenario where they were
on separate buses.
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Thanks
^ permalink raw reply
* Re: [PATCH 1/3] net: phy: Check phy_driver ready before accessing
From: Brandon Maier @ 2018-06-07 17:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, f.fainelli, davem, michal.simek, Clayton Shotwell,
Kristopher Cory, linux-kernel
In-Reply-To: <20180607165227.GD25513@lunn.ch>
On Thu, Jun 7, 2018 at 11:52 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> FYI: net-next is closed at the moment. Please resubmit these in two
> weeks time.
Ah, I didn't see networking/netdev-FAQ.txt. I'll resubmit these then.
> I'm sure there are more issues like this in the code. e.g. there is
> no attempt made to hold a reference to the child phy. So it could be
> unbound. priv->phy_drv->read_status(phydev) is then going to do bad
> things.
>
Agreed. Another thing that looks suspicious to me is the driver
overrides the private data of the device it's attaching too, in the
`priv->phy_dev->priv = priv` bit. Seems like that could cause all
sorts of driver corruption problems.
But fixing that is going to require more drastic changes to how this
driver works. So it'd be worth applying this patch in the mean time.
^ permalink raw reply
* Re: KMSAN: uninit-value in ebt_stp_mt_check (2)
From: Dmitry Vyukov @ 2018-06-07 17:34 UTC (permalink / raw)
To: syzbot
Cc: bridge, coreteam, David Miller, Florian Westphal,
Jozsef Kadlecsik, LKML, netdev, netfilter-devel,
Pablo Neira Ayuso, stephen hemminger, syzkaller-bugs
In-Reply-To: <0000000000009a3b5b056e109e42@google.com>
On Thu, Jun 7, 2018 at 7:29 PM, syzbot
<syzbot+da4494182233c23a5fcf@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: c6a6aed994b6 kmsan: remove dead code to trigger syzbot build
> git tree: https://github.com/google/kmsan.git/master
> console output: https://syzkaller.appspot.com/x/log.txt?x=17bde74f800000
> kernel config: https://syzkaller.appspot.com/x/.config?x=848e40757852af3e
> dashboard link: https://syzkaller.appspot.com/bug?extid=da4494182233c23a5fcf
> compiler: clang version 7.0.0 (trunk 334104)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+da4494182233c23a5fcf@syzkaller.appspotmail.com
Hi Stephen,
This strikes again.
The first part of the check is par->nft_compat. I don't see where
ebt_check_entry initializes it. Should it? Or matches should not look at it?
> ==================================================================
> BUG: KMSAN: uninit-value in ebt_stp_mt_check+0x24b/0x450
> net/bridge/netfilter/ebt_stp.c:162
> CPU: 0 PID: 12006 Comm: syz-executor7 Not tainted 4.17.0+ #3
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x185/0x1d0 lib/dump_stack.c:113
> kmsan_report+0x149/0x260 mm/kmsan/kmsan.c:1084
> __msan_warning_32+0x6e/0xc0 mm/kmsan/kmsan_instr.c:620
> ebt_stp_mt_check+0x24b/0x450 net/bridge/netfilter/ebt_stp.c:162
> xt_check_match+0x1438/0x1650 net/netfilter/x_tables.c:506
> ebt_check_match net/bridge/netfilter/ebtables.c:372 [inline]
> ebt_check_entry net/bridge/netfilter/ebtables.c:702 [inline]
> translate_table+0x4e88/0x6120 net/bridge/netfilter/ebtables.c:943
> do_replace_finish+0x1258/0x2ea0 net/bridge/netfilter/ebtables.c:999
> do_replace+0x719/0x780 net/bridge/netfilter/ebtables.c:1138
> do_ebt_set_ctl+0x2ab/0x3c0 net/bridge/netfilter/ebtables.c:1517
> nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
> nf_setsockopt+0x47c/0x4e0 net/netfilter/nf_sockopt.c:115
> ip_setsockopt+0x24b/0x2b0 net/ipv4/ip_sockglue.c:1251
> udp_setsockopt+0x108/0x1b0 net/ipv4/udp.c:2416
> sock_common_setsockopt+0x13b/0x170 net/core/sock.c:3039
> __sys_setsockopt+0x496/0x540 net/socket.c:1903
> __do_sys_setsockopt net/socket.c:1914 [inline]
> __se_sys_setsockopt net/socket.c:1911 [inline]
> __x64_sys_setsockopt+0x15c/0x1c0 net/socket.c:1911
> do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x4559f9
> RSP: 002b:00007f45b9246c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> RAX: ffffffffffffffda RBX: 00007f45b92476d4 RCX: 00000000004559f9
> RDX: 0000000000000080 RSI: 0000000000000000 RDI: 0000000000000014
> RBP: 000000000072bea0 R08: 0000000000000300 R09: 0000000000000000
> R10: 0000000020000480 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 00000000004c0d6d R14: 00000000004d07c8 R15: 0000000000000000
>
> Local variable description: ----mtpar.i@translate_table
> Variable was created at:
> translate_table+0xbb/0x6120 net/bridge/netfilter/ebtables.c:831
> do_replace_finish+0x1258/0x2ea0 net/bridge/netfilter/ebtables.c:999
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/0000000000009a3b5b056e109e42%40google.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: syzbot @ 2018-06-07 17:31 UTC (permalink / raw)
To: avagin, davem, dingtianhong, edumazet, elena.reshetova, jasowang,
kvm, linux-kernel, matthew, mingo, mst, netdev, pabeni,
syzkaller-bugs, viro, virtualization, willemb
In-Reply-To: <20180607183629-mutt-send-email-mst@kernel.org>
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger
crash:
Reported-and-tested-by:
syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com
Tested on:
commit: c6a6aed994b6 kmsan: remove dead code to trigger syzbot build
git tree: https://github.com/google/kmsan.git/master
kernel config: https://syzkaller.appspot.com/x/.config?x=848e40757852af3e
compiler: clang version 7.0.0 (trunk 334104)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1119eddf800000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply
* [PATCH net-next] bpfilter: fix OUTPUT_FORMAT
From: Alexei Starovoitov @ 2018-06-07 17:29 UTC (permalink / raw)
To: David S . Miller; +Cc: daniel, netdev, kernel-team
From: Alexei Starovoitov <ast@fb.com>
CONFIG_OUTPUT_FORMAT is x86 only macro.
Used objdump to extract elf file format.
Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Reported-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
net/bpfilter/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
index aafa72001fcd..e0bbe7583e58 100644
--- a/net/bpfilter/Makefile
+++ b/net/bpfilter/Makefile
@@ -21,7 +21,7 @@ endif
# which bpfilter_kern.c passes further into umh blob loader at run-time
quiet_cmd_copy_umh = GEN $@
cmd_copy_umh = echo ':' > $(obj)/.bpfilter_umh.o.cmd; \
- $(OBJCOPY) -I binary -O $(CONFIG_OUTPUT_FORMAT) \
+ $(OBJCOPY) -I binary -O `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
-B `$(OBJDUMP) -f $<|grep architecture|cut -d, -f1|cut -d' ' -f2` \
--rename-section .data=.init.rodata $< $@
--
2.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox