netdev - Re: [net PATCH 1/3] net: rcu lock and preempt disable missing around generic xdp

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

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

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

Message-ID: <94e9e0f9-403a-72c7-62d2-797f6fd1bc99@fb.com>
Date:   Fri, 8 Sep 2017 15:38:14 -0700
From:   Alexei Starovoitov <ast@...com>
To:     John Fastabend <john.fastabend@...il.com>, <davem@...emloft.net>
CC:     <netdev@...r.kernel.org>, <daniel@...earbox.net>
Subject: Re: [net PATCH 1/3] net: rcu lock and preempt disable missing around
 generic xdp

On 9/8/17 2:00 PM, John Fastabend wrote:
> do_xdp_generic must be called inside rcu critical section with preempt
> disabled to ensure BPF programs are valid and per-cpu variables used
> for redirect operations are consistent. This patch ensures this is true
> and fixes the splat below.
>
> The netif_receive_skb_internal() code path is now broken into two rcu
> critical sections. I decided it was better to limit the preempt_enable/disable
> block to just the xdp static key portion and the fallout is more
> rcu_read_lock/unlock calls. Seems like the best option to me.
>
> [  607.596901] =============================
> [  607.596906] WARNING: suspicious RCU usage
> [  607.596912] 4.13.0-rc4+ #570 Not tainted
> [  607.596917] -----------------------------
> [  607.596923] net/core/dev.c:3948 suspicious rcu_dereference_check() usage!
> [  607.596927]
> [  607.596927] other info that might help us debug this:
> [  607.596927]
> [  607.596933]
> [  607.596933] rcu_scheduler_active = 2, debug_locks = 1
> [  607.596938] 2 locks held by pool/14624:
> [  607.596943]  #0:  (rcu_read_lock_bh){......}, at: [<ffffffff95445ffd>] ip_finish_output2+0x14d/0x890
> [  607.596973]  #1:  (rcu_read_lock_bh){......}, at: [<ffffffff953c8e3a>] __dev_queue_xmit+0x14a/0xfd0
> [  607.597000]
> [  607.597000] stack backtrace:
> [  607.597006] CPU: 5 PID: 14624 Comm: pool Not tainted 4.13.0-rc4+ #570
> [  607.597011] Hardware name: Dell Inc. Precision Tower 5810/0HHV7N, BIOS A17 03/01/2017
> [  607.597016] Call Trace:
> [  607.597027]  dump_stack+0x67/0x92
> [  607.597040]  lockdep_rcu_suspicious+0xdd/0x110
> [  607.597054]  do_xdp_generic+0x313/0xa50
> [  607.597068]  ? time_hardirqs_on+0x5b/0x150
> [  607.597076]  ? mark_held_locks+0x6b/0xc0
> [  607.597088]  ? netdev_pick_tx+0x150/0x150
> [  607.597117]  netif_rx_internal+0x205/0x3f0
> [  607.597127]  ? do_xdp_generic+0xa50/0xa50
> [  607.597144]  ? lock_downgrade+0x2b0/0x2b0
> [  607.597158]  ? __lock_is_held+0x93/0x100
> [  607.597187]  netif_rx+0x119/0x190
> [  607.597202]  loopback_xmit+0xfd/0x1b0
> [  607.597214]  dev_hard_start_xmit+0x127/0x4e0
>
> Fixes: d445516966dc ("net: xdp: support xdp generic on virtual devices")
> Fixes: b5cdae3291f7 ("net: Generic XDP")
> Acked-by: Daniel Borkmann <daniel@...earbox.net>
> Signed-off-by: John Fastabend <john.fastabend@...il.com>

argh, so it's due to virtual devices and loopback.
Not pretty, but have to agree I don't see another way of fixing it.

Acked-by: Alexei Starovoitov <ast@...nel.org>