* 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
* KMSAN: uninit-value in ebt_stp_mt_check (2)
From: syzbot @ 2018-06-07 17:29 UTC (permalink / raw)
To: bridge, coreteam, davem, fw, kadlec, linux-kernel, netdev,
netfilter-devel, pablo, stephen, syzkaller-bugs
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
==================================================================
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.
^ permalink raw reply
* [PATCH net-next] umh: fix race condition
From: Alexei Starovoitov @ 2018-06-07 17:23 UTC (permalink / raw)
To: David S . Miller
Cc: daniel, mcgrof, netdev, dvyukov, linux-kernel, kernel-team
kasan reported use-after-free:
BUG: KASAN: use-after-free in call_usermodehelper_exec_work+0x2d3/0x310 kernel/umh.c:195
Write of size 4 at addr ffff8801d9202370 by task kworker/u4:2/50
Workqueue: events_unbound call_usermodehelper_exec_work
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1b9/0x294 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
__asan_report_store4_noabort+0x17/0x20 mm/kasan/report.c:437
call_usermodehelper_exec_work+0x2d3/0x310 kernel/umh.c:195
process_one_work+0xc1e/0x1b50 kernel/workqueue.c:2145
worker_thread+0x1cc/0x1440 kernel/workqueue.c:2279
kthread+0x345/0x410 kernel/kthread.c:240
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
The reason is that 'sub_info' cannot be accessed out of parent task
context, since it will be freed by the child.
Instead remember the pid in the child task.
Fixes: 449325b52b7a ("umh: introduce fork_usermode_blob() helper")
Reported-by: syzbot+2c73319c406f1987d156@syzkaller.appspotmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/umh.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/umh.c b/kernel/umh.c
index 30db93fd7e39..c449858946af 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -99,6 +99,7 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
+ sub_info->pid = task_pid_nr(current);
if (sub_info->file)
retval = do_execve_file(sub_info->file,
sub_info->argv, sub_info->envp);
@@ -191,8 +192,6 @@ static void call_usermodehelper_exec_work(struct work_struct *work)
if (pid < 0) {
sub_info->retval = pid;
umh_complete(sub_info);
- } else {
- sub_info->pid = pid;
}
}
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net] failover: eliminate callback hell
From: Michael S. Tsirkin @ 2018-06-07 17:22 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Alexander Duyck, Samudrala, Sridhar, Jiri Pirko, KY Srinivasan,
Haiyang Zhang, David Miller, Netdev, Stephen Hemminger
In-Reply-To: <20180607091742.461dff83@xeon-e3>
On Thu, Jun 07, 2018 at 09:17:42AM -0700, Stephen Hemminger wrote:
> On Thu, 7 Jun 2018 18:41:31 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > > > Why would DPDK care what we do in the kernel? Isn't it just slapping
> > > > vfio-pci on the netdevs it sees?
> > >
> > > Alex, you are correct for Intel devices; but DPDK on Azure is not Intel based.,.
> > > The DPDK support uses:
> > > * Mellanox MLX5 which uses the Infinband hooks to do DMA directly to
> > > userspace. This means VF netdev device must exist and be visible.
> > > * Slow path using kernel netvsc device, TAP and BPF to get exception
> > > path packets to userspace.
> > > * A autodiscovery mechanism that to set all this up that relies on
> > > 2 device model and sysfs.
> >
> > Could you describe what does it look for exactly? What will break if
> > instead of MLX5 being a child of the PV, it's a child of the failover
> > device?
>
> So in DPDK there is an internal four device model:
> 1. failsafe is like failover in your model
> 2. TAP is used like netvsc in kernel
> 3. MLX5 is the VF
> 4. vdev_netvsc is a pseudo device whose only reason to exist
> is to glue everything together.
>
> Digging deeper inside...
>
> Vdev_netvsc does:
> * driver is started in a convuluted way off device arguments
> * probe routine for driver runs
> - scans list of kernel interfaces in sysfs
> - matches those using VMBUS
Could you tell a bit more what does this step entail?
> - skip netvsc devices that have an IPV4 route
> * scan for PCI devices that have same MAC address as kernel netvsc
> devices discovered in previous step
> * add these interfaces to arguments to failsafe
>
> Then failsafe configures based on arguments on device
>
> The code works but is specific to the Azure hardware model, and exposes lots
> of things to application that it should not have to care about.
>
> If you try and walk through this code in DPDK, you will see why I have developed
> a dislike for high levels of indirection.
>
>
>
Thanks that was helpful! I'll try to poke at it next week. Just from
the description it seems the kernel is merely used to locate the MAC
address through sysfs and that for this DPDK code to keep working the
hidden device must be hidden from it in sysfs - is that a fair summary?
--
MST
^ permalink raw reply
* Re: [RFC PATCH 1/2] net: macb: Add CAP to disable hardware TX checksum offloading
From: Jennifer Dahm @ 2018-06-07 16:43 UTC (permalink / raw)
To: Nicolas Ferre, netdev, David S . Miller; +Cc: Nathan Sullivan
In-Reply-To: <ad25fd2b-a3a0-28d5-4dd1-a7ccc935fff2@microchip.com>
Hi Nicolas,
On 06/04/2018 10:13 AM, Nicolas Ferre wrote:
> On 25/05/2018 at 23:44, Jennifer Dahm wrote:
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index 3e93df5..a5d564b 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -3360,8 +3360,12 @@ static int macb_init(struct platform_device
>> *pdev)
>> dev->hw_features |= MACB_NETIF_LSO;
>> /* Checksum offload is only available on gem with packet
>> buffer */
>> - if (macb_is_gem(bp) && !(bp->caps & MACB_CAPS_FIFO_MODE))
>> - dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
>> + if (macb_is_gem(bp) && !(bp->caps & MACB_CAPS_FIFO_MODE)) {
>> + if (!(bp->caps & MACB_CAPS_DISABLE_TX_HW_CSUM))
>
> Why not the other way around? negating a "disabled" feature is always
> challenge ;-)
>
>> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
>> + else
>> + dev->hw_features |= NETIF_F_RXCSUM;
>> + }
>> if (bp->caps & MACB_CAPS_SG_DISABLED)
>> dev->hw_features &= ~NETIF_F_SG;
>> dev->features = dev->hw_features;
I can switch the ordering of the if-else clauses if that's what you're
nitpicking. ;)
Alternatively, if you're asking why the flag is used to disable rather
than enable checksum offloading: I was working under the assumption that
this was an isolated bug, and so an opt-out would require less
maintainance than an opt-in. If we discover that this is a problem
across a wide variety of Cadence IP, it would definitely make sense to
replace it with an opt-in (i.e. MACB_CAPS_TX_HW_CSUM_ENABLED).
Best,
Jennifer Dahm
^ permalink raw reply
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: Michael S. Tsirkin @ 2018-06-07 17:10 UTC (permalink / raw)
To: syzbot
Cc: avagin, davem, dingtianhong, edumazet, elena.reshetova, jasowang,
kvm, linux-kernel, matthew, mingo, netdev, pabeni, syzkaller-bugs,
viro, virtualization, willemb
In-Reply-To: <000000000000cf4578056ab12452@google.com>
#syz test: https://github.com/google/kmsan.git master
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);
node->vq = vq;
node->msg.type = type;
return node;
^ permalink raw reply related
* Re: KMSAN: uninit-value in _copy_to_iter (2)
From: syzbot @ 2018-06-07 17:04 UTC (permalink / raw)
To: avagin, davem, dingtianhong, dvyukov, edumazet, elena.reshetova,
jasowang, kvm, linux-kernel, matthew, mingo, mst, netdev, pabeni,
syzkaller-bugs, viro, virtualization, willemb
In-Reply-To: <CACT4Y+btyLi+z09EO0JU-7AS_OfRbWiZvGn-KP9fndevh+xFNw@mail.gmail.com>
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=17dc3a8f800000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply
* Re: KASAN: use-after-free Write in bpf_tcp_close
From: Dmitry Vyukov @ 2018-06-07 16:58 UTC (permalink / raw)
To: Daniel Borkmann
Cc: syzbot, Alexei Starovoitov, LKML, netdev, syzkaller-bugs,
John Fastabend
In-Reply-To: <ca3eff64-adb0-6110-38ca-2e07b6ecc3d6@iogearbox.net>
On Mon, May 28, 2018 at 12:15 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> [ +John ]
>
> On 05/27/2018 10:06 PM, syzbot wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: ff4fb475cea8 Merge branch 'btf-uapi-cleanups'
>> git tree: bpf-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=12b3d577800000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=b632d8e2c2ab2c1
>> dashboard link: https://syzkaller.appspot.com/bug?extid=31025a5f3f7650081204
>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=109a2f37800000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=171a727b800000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+31025a5f3f7650081204@syzkaller.appspotmail.com
>
> Should be fixed by: https://patchwork.ozlabs.org/patch/920695/
#syz fix: bpf: sockhash fix race with bpf_tcp_close and map delete
>> ==================================================================
>> BUG: KASAN: use-after-free in cmpxchg_size include/asm-generic/atomic-instrumented.h:355 [inline]
>> BUG: KASAN: use-after-free in bpf_tcp_close+0x6f5/0xf80 kernel/bpf/sockmap.c:265
>> Write of size 8 at addr ffff8801ca277680 by task syz-executor749/9723
>>
>> CPU: 0 PID: 9723 Comm: syz-executor749 Not tainted 4.17.0-rc4+ #19
>> 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
>> print_address_description+0x6c/0x20b mm/kasan/report.c:256
>> kasan_report_error mm/kasan/report.c:354 [inline]
>> kasan_report.cold.7+0x242/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
>> cmpxchg_size include/asm-generic/atomic-instrumented.h:355 [inline]
>> bpf_tcp_close+0x6f5/0xf80 kernel/bpf/sockmap.c:265
>> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
>> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
>> sock_release+0x96/0x1b0 net/socket.c:594
>> sock_close+0x16/0x20 net/socket.c:1149
>> __fput+0x34d/0x890 fs/file_table.c:209
>> ____fput+0x15/0x20 fs/file_table.c:243
>> task_work_run+0x1e4/0x290 kernel/task_work.c:113
>> exit_task_work include/linux/task_work.h:22 [inline]
>> do_exit+0x1aee/0x2730 kernel/exit.c:865
>> do_group_exit+0x16f/0x430 kernel/exit.c:968
>> __do_sys_exit_group kernel/exit.c:979 [inline]
>> __se_sys_exit_group kernel/exit.c:977 [inline]
>> __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
>> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>> RIP: 0033:0x440a59
>> RSP: 002b:00007ffdadf92488 EFLAGS: 00000206 ORIG_RAX: 00000000000000e7
>> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000440a59
>> RDX: 0000000000440a59 RSI: 0000000000000020 RDI: 0000000000000000
>> RBP: 0000000000000000 R08: 00000000004002c8 R09: 0000000000401ea0
>> R10: 00000000004002c8 R11: 0000000000000206 R12: 000000000001b5ac
>> R13: 0000000000401ea0 R14: 0000000000000000 R15: 0000000000000000
>>
>> Allocated by task 9723:
>> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>> set_track mm/kasan/kasan.c:460 [inline]
>> kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
>> __do_kmalloc_node mm/slab.c:3682 [inline]
>> __kmalloc_node+0x47/0x70 mm/slab.c:3689
>> kmalloc_node include/linux/slab.h:554 [inline]
>> bpf_map_area_alloc+0x3f/0x90 kernel/bpf/syscall.c:144
>> sock_map_alloc+0x376/0x410 kernel/bpf/sockmap.c:1555
>> find_and_alloc_map kernel/bpf/syscall.c:126 [inline]
>> map_create+0x393/0x1010 kernel/bpf/syscall.c:448
>> __do_sys_bpf kernel/bpf/syscall.c:2128 [inline]
>> __se_sys_bpf kernel/bpf/syscall.c:2105 [inline]
>> __x64_sys_bpf+0x300/0x4f0 kernel/bpf/syscall.c:2105
>> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Freed by task 4521:
>> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>> set_track mm/kasan/kasan.c:460 [inline]
>> __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
>> kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
>> __cache_free mm/slab.c:3498 [inline]
>> kfree+0xd9/0x260 mm/slab.c:3813
>> kvfree+0x61/0x70 mm/util.c:440
>> bpf_map_area_free+0x15/0x20 kernel/bpf/syscall.c:155
>> sock_map_remove_complete kernel/bpf/sockmap.c:1443 [inline]
>> sock_map_free+0x408/0x540 kernel/bpf/sockmap.c:1619
>> bpf_map_free_deferred+0xba/0xf0 kernel/bpf/syscall.c:259
>> process_one_work+0xc1e/0x1b50 kernel/workqueue.c:2145
>> worker_thread+0x1cc/0x1440 kernel/workqueue.c:2279
>> kthread+0x345/0x410 kernel/kthread.c:238
>> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>>
>> The buggy address belongs to the object at ffff8801ca277680
>> which belongs to the cache kmalloc-1024 of size 1024
>> The buggy address is located 0 bytes inside of
>> 1024-byte region [ffff8801ca277680, ffff8801ca277a80)
>> The buggy address belongs to the page:
>> page:ffffea0007289d80 count:1 mapcount:0 mapping:ffff8801ca276000 index:0x0 compound_mapcount: 0
>> flags: 0x2fffc0000008100(slab|head)
>> raw: 02fffc0000008100 ffff8801ca276000 0000000000000000 0000000100000007
>> raw: ffffea0006d12b20 ffffea000763bba0 ffff8801da800ac0 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff8801ca277580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff8801ca277600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>> ffff8801ca277680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>> ^
>> ffff8801ca277700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>> ffff8801ca277780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>> ==================================================================
>>
>>
>> ---
>> 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
>
> --
> 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/ca3eff64-adb0-6110-38ca-2e07b6ecc3d6%40iogearbox.net.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: general protection fault in bpf_tcp_close
From: Dmitry Vyukov @ 2018-06-07 16:58 UTC (permalink / raw)
To: Daniel Borkmann
Cc: syzbot, Alexei Starovoitov, LKML, netdev, syzkaller-bugs,
John Fastabend
In-Reply-To: <7257c52b-7632-e313-6e80-a4bc1f8a6bf0@iogearbox.net>
On Mon, May 28, 2018 at 12:15 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> [ +John ]
>
> On 05/26/2018 11:13 AM, syzbot wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: fd0bfa8d6e04 Merge branch 'bpf-af-xdp-cleanups'
>> git tree: bpf-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=11da9427800000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=b632d8e2c2ab2c1
>> dashboard link: https://syzkaller.appspot.com/bug?extid=0ce137753c78f7b6acc1
>> 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+0ce137753c78f7b6acc1@syzkaller.appspotmail.com
>
> Should be fixed by: https://patchwork.ozlabs.org/patch/920695/
#syz fix: bpf: sockhash fix race with bpf_tcp_close and map delete
>> kasan: CONFIG_KASAN_INLINE enabled
>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>> general protection fault: 0000 [#1] SMP KASAN
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 0 PID: 12139 Comm: syz-executor2 Not tainted 4.17.0-rc4+ #17
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> RIP: 0010:__hlist_del include/linux/list.h:649 [inline]
>> RIP: 0010:hlist_del_rcu include/linux/rculist.h:427 [inline]
>> RIP: 0010:bpf_tcp_close+0x7d2/0xf80 kernel/bpf/sockmap.c:271
>> RSP: 0018:ffff8801a8f8ef70 EFLAGS: 00010a02
>> RAX: ffffed00351f1dfd RBX: dffffc0000000000 RCX: dead000000000200
>> RDX: 0000000000000000 RSI: 1bd5a00000000040 RDI: ffff8801cb710910
>> RBP: ffff8801a8f8f110 R08: ffffed003350ac9d R09: ffffed003350ac9c
>> R10: ffffed003350ac9c R11: ffff88019a8564e3 R12: ffff8801cb710380
>> R13: ffff8801b17ea6e0 R14: ffff8801cb710398 R15: ffff8801cb710900
>> FS: 00007f9890c43700(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007fde1a668000 CR3: 000000019dca2000 CR4: 00000000001406f0
>> DR0: 00000000200001c0 DR1: 00000000200001c0 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
>> Call Trace:
>> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
>> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
>> sock_release+0x96/0x1b0 net/socket.c:594
>> sock_close+0x16/0x20 net/socket.c:1149
>> __fput+0x34d/0x890 fs/file_table.c:209
>> ____fput+0x15/0x20 fs/file_table.c:243
>> task_work_run+0x1e4/0x290 kernel/task_work.c:113
>> exit_task_work include/linux/task_work.h:22 [inline]
>> do_exit+0x1aee/0x2730 kernel/exit.c:865
>> do_group_exit+0x16f/0x430 kernel/exit.c:968
>> get_signal+0x886/0x1960 kernel/signal.c:2469
>> do_signal+0x98/0x2040 arch/x86/kernel/signal.c:810
>> exit_to_usermode_loop+0x28a/0x310 arch/x86/entry/common.c:162
>> prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
>> syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
>> do_syscall_64+0x6ac/0x800 arch/x86/entry/common.c:290
>> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>> RIP: 0033:0x455a09
>> RSP: 002b:00007f9890c42ce8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
>> RAX: fffffffffffffe00 RBX: 000000000072bec8 RCX: 0000000000455a09
>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000000000072bec8
>> RBP: 000000000072bec8 R08: 0000000000000000 R09: 000000000072bea0
>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
>> R13: 00007ffcb48ac3ff R14: 00007f9890c439c0 R15: 0000000000000000
>> Code: ff 48 c1 e9 03 80 3c 19 00 0f 85 a9 05 00 00 49 8b 4f 18 48 8b 85 98 fe ff ff 48 89 ce c6 00 00 48 c1 ee 03 48 89 95 d8 fe ff ff <80> 3c 1e 00 0f 85 c6 05 00 00 48 8b 85 98 fe ff ff 48 85 d2 48
>> RIP: __hlist_del include/linux/list.h:649 [inline] RSP: ffff8801a8f8ef70
>> RIP: hlist_del_rcu include/linux/rculist.h:427 [inline] RSP: ffff8801a8f8ef70
>> RIP: bpf_tcp_close+0x7d2/0xf80 kernel/bpf/sockmap.c:271 RSP: ffff8801a8f8ef70
>> ---[ end trace e81227e93c7e7b75 ]---
>>
>>
>> ---
>> 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/7257c52b-7632-e313-6e80-a4bc1f8a6bf0%40iogearbox.net.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 1/3] net: phy: Check phy_driver ready before accessing
From: Andrew Lunn @ 2018-06-07 16:54 UTC (permalink / raw)
To: Brandon Maier
Cc: netdev, f.fainelli, davem, michal.simek, clayton.shotwell,
kristopher.cory, linux-kernel
In-Reply-To: <20180607155348.149665-1-brandon.maier@rockwellcollins.com>
On Thu, Jun 07, 2018 at 10:53:46AM -0500, Brandon Maier wrote:
> Since a phy_device is added to the global mdio_bus list during
> phy_device_register(), but a phy_device's phy_driver doesn't get
> attached until phy_probe(). It's possible of_phy_find_device() in
> xgmiitorgmii will return a valid phy with a NULL phy_driver. Leading to
> a NULL pointer access during the memcpy().
Hi Brandon
FYI: net-next is closed at the moment. Please resubmit these in two
weeks time.
Andrew
^ permalink raw reply
* Re: [PATCH 1/3] net: phy: Check phy_driver ready before accessing
From: Andrew Lunn @ 2018-06-07 16:52 UTC (permalink / raw)
To: Brandon Maier
Cc: netdev, f.fainelli, davem, michal.simek, clayton.shotwell,
kristopher.cory, linux-kernel
In-Reply-To: <20180607155348.149665-1-brandon.maier@rockwellcollins.com>
On Thu, Jun 07, 2018 at 10:53:46AM -0500, Brandon Maier wrote:
> Since a phy_device is added to the global mdio_bus list during
> phy_device_register(), but a phy_device's phy_driver doesn't get
> attached until phy_probe(). It's possible of_phy_find_device() in
> xgmiitorgmii will return a valid phy with a NULL phy_driver. Leading to
> a NULL pointer access during the memcpy().
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.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 2/3] net: phy: xgmiitorgmii: Use correct mdio bus
From: Andrew Lunn @ 2018-06-07 16:49 UTC (permalink / raw)
To: Brandon Maier
Cc: netdev, f.fainelli, davem, michal.simek, clayton.shotwell,
kristopher.cory, linux-kernel
In-Reply-To: <20180607155348.149665-2-brandon.maier@rockwellcollins.com>
On Thu, Jun 07, 2018 at 10:53:47AM -0500, Brandon Maier wrote:
> The xgmiitorgmii is using the mii_bus of the device it's attached too,
> instead of the bus it was given during probe.
Hi Brandon
I think the assumption was they are both on the same bus.
However, they don't need to be.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 3/3] net: phy: xgmiitorgmii: Check read_status results
From: Andrew Lunn @ 2018-06-07 16:43 UTC (permalink / raw)
To: Brandon Maier
Cc: netdev, f.fainelli, davem, michal.simek, clayton.shotwell,
kristopher.cory, linux-kernel
In-Reply-To: <20180607155348.149665-3-brandon.maier@rockwellcollins.com>
On Thu, Jun 07, 2018 at 10:53:48AM -0500, Brandon Maier wrote:
> We're ignoring the result of the attached phy device's read_status().
> Return it so we can detect errors.
>
> Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] xsk: Fix umem fill/completion queue mmap on 32-bit
From: Björn Töpel @ 2018-06-07 16:34 UTC (permalink / raw)
To: geert
Cc: David Miller, Björn Töpel, Karlsson, Magnus, ast,
Arnd Bergmann, akpm, Netdev, linux-mm, LKML
In-Reply-To: <1528378654-1484-1-git-send-email-geert@linux-m68k.org>
Den tors 7 juni 2018 kl 15:37 skrev Geert Uytterhoeven <geert@linux-m68k.org>:
>
> With gcc-4.1.2 on 32-bit:
>
> net/xdp/xsk.c:663: warning: integer constant is too large for ‘long’ type
> net/xdp/xsk.c:665: warning: integer constant is too large for ‘long’ type
>
> Add the missing "ULL" suffixes to the large XDP_UMEM_PGOFF_*_RING values
> to fix this.
>
> net/xdp/xsk.c:663: warning: comparison is always false due to limited range of data type
> net/xdp/xsk.c:665: warning: comparison is always false due to limited range of data type
>
> "unsigned long" is 32-bit on 32-bit systems, hence the offset is
> truncated, and can never be equal to any of the XDP_UMEM_PGOFF_*_RING
> values. Use loff_t (and the required cast) to fix this.
>
> Fixes: 423f38329d267969 ("xsk: add umem fill queue support and mmap")
> Fixes: fe2308328cd2f26e ("xsk: add umem completion queue support and mmap")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Compile-tested only.
> ---
> include/uapi/linux/if_xdp.h | 4 ++--
> net/xdp/xsk.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
> index 1fa0e977ea8d0224..caed8b1614ffc0aa 100644
> --- a/include/uapi/linux/if_xdp.h
> +++ b/include/uapi/linux/if_xdp.h
> @@ -63,8 +63,8 @@ struct xdp_statistics {
> /* Pgoff for mmaping the rings */
> #define XDP_PGOFF_RX_RING 0
> #define XDP_PGOFF_TX_RING 0x80000000
> -#define XDP_UMEM_PGOFF_FILL_RING 0x100000000
> -#define XDP_UMEM_PGOFF_COMPLETION_RING 0x180000000
> +#define XDP_UMEM_PGOFF_FILL_RING 0x100000000ULL
> +#define XDP_UMEM_PGOFF_COMPLETION_RING 0x180000000ULL
>
> /* Rx/Tx descriptor */
> struct xdp_desc {
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index c6ed2454f7ce55e8..36919a254ba370c3 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -643,7 +643,7 @@ static int xsk_getsockopt(struct socket *sock, int level, int optname,
> static int xsk_mmap(struct file *file, struct socket *sock,
> struct vm_area_struct *vma)
> {
> - unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
> + loff_t offset = (loff_t)vma->vm_pgoff << PAGE_SHIFT;
> unsigned long size = vma->vm_end - vma->vm_start;
> struct xdp_sock *xs = xdp_sk(sock->sk);
> struct xsk_queue *q = NULL;
> --
> 2.7.4
>
Thanks Geert!
Acked-by: Björn Töpel <bjorn.topel@intel.com>
^ permalink raw reply
* Re: nfp: bpf: perf event output helpers support
From: Jakub Kicinski @ 2018-06-07 16:33 UTC (permalink / raw)
To: Kees Cook; +Cc: Quentin Monnet, Daniel Borkmann, Network Development, LKML
In-Reply-To: <CAGXu5jJq774LZY1xUCKxG3Z4MkJ_JBJP7veB5gUCy4meb_PtSA@mail.gmail.com>
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.
^ permalink raw reply
* Re: [PATCH] net-fq: Add WARN_ON check for null flow.
From: Ben Greear @ 2018-06-07 16:33 UTC (permalink / raw)
To: Eric Dumazet, netdev
In-Reply-To: <577ce32e-cbee-c5d8-da12-81075e924b23@gmail.com>
On 06/07/2018 09:17 AM, Eric Dumazet wrote:
>
>
> On 06/07/2018 09:06 AM, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> While testing an ath10k firmware that often crashed under load,
>> I was seeing kernel crashes as well. One of them appeared to
>> be a dereference of a NULL flow object in fq_tin_dequeue.
>>
>> I have since fixed the firmware flaw, but I think it would be
>> worth adding the WARN_ON in case the problem appears again.
>>
>> common_interrupt+0xf/0xf
>> </IRQ>
>>
>
> Please find the exact commit that brought this bug,
> and add a corresponding Fixes: tag
It will be a total pain to bisect this problem since my test
case that causes this is running my modified firmware (and a buggy one at that),
modified ath10k driver (to work with this firmware and support my test case easily),
and the failure case appears to cause multiple different-but-probably-related
crashes and often hangs or reboots the test system.
Probably this is all caused by some nasty race or buggy logic related to
dealing with a crashed ath10k firmware tearing down txq logic from the
bottom up. There have been many such bugs in the past, I and others fixed a few,
and very likely more remain.
For what it is worth, I didn't see this crash in 4.13, and I spent some time
testing buggy firmware there occasionally.
If someone else has interest in debugging the ath10k driver, I will be happy to generate
a mostly-stock firmware image with ability to crash in the TX path and give it to them.
It will crash the stock upstream code reliably in my experience.
Thanks,
Ben
>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>> include/net/fq_impl.h | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/include/net/fq_impl.h b/include/net/fq_impl.h
>> index be7c0fa..e40354d 100644
>> --- a/include/net/fq_impl.h
>> +++ b/include/net/fq_impl.h
>> @@ -80,6 +80,9 @@ static struct sk_buff *fq_tin_dequeue(struct fq *fq,
>>
>> flow = list_first_entry(head, struct fq_flow, flowchain);
>>
>> + if (WARN_ON_ONCE(!flow))
>> + return NULL;
>> +
>> if (flow->deficit <= 0) {
>> flow->deficit += fq->quantum;
>> list_move_tail(&flow->flowchain,
>>
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
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