Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net: rcu lock and preempt disable missing around generic xdp

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@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

John Fastabend and committed by
David S. Miller
bbbe211c 109980b8

+16 -9
+16 -9
net/core/dev.c
··· 3981 3981 trace_netif_rx(skb); 3982 3982 3983 3983 if (static_key_false(&generic_xdp_needed)) { 3984 - int ret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), 3985 - skb); 3984 + int ret; 3985 + 3986 + preempt_disable(); 3987 + rcu_read_lock(); 3988 + ret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb); 3989 + rcu_read_unlock(); 3990 + preempt_enable(); 3986 3991 3987 3992 /* Consider XDP consuming the packet a success from 3988 3993 * the netdev point of view we do not want to count ··· 4505 4500 if (skb_defer_rx_timestamp(skb)) 4506 4501 return NET_RX_SUCCESS; 4507 4502 4508 - rcu_read_lock(); 4509 - 4510 4503 if (static_key_false(&generic_xdp_needed)) { 4511 - int ret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), 4512 - skb); 4504 + int ret; 4513 4505 4514 - if (ret != XDP_PASS) { 4515 - rcu_read_unlock(); 4506 + preempt_disable(); 4507 + rcu_read_lock(); 4508 + ret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb); 4509 + rcu_read_unlock(); 4510 + preempt_enable(); 4511 + 4512 + if (ret != XDP_PASS) 4516 4513 return NET_RX_DROP; 4517 - } 4518 4514 } 4519 4515 4516 + rcu_read_lock(); 4520 4517 #ifdef CONFIG_RPS 4521 4518 if (static_key_false(&rps_needed)) { 4522 4519 struct rps_dev_flow voidflow, *rflow = &voidflow;