Skip to content

Commit 22f6bbb

Browse files
ecree-solarflaredavem330
authored andcommitted
net: use skb_list_del_init() to remove from RX sublists
list_del() leaves the skb->next pointer poisoned, which can then lead to a crash in e.g. OVS forwarding. For example, setting up an OVS VXLAN forwarding bridge on sfc as per: ======== $ ovs-vsctl show 5dfd9c47-f04b-4aaa-aa96-4fbb0a522a30 Bridge "br0" Port "br0" Interface "br0" type: internal Port "enp6s0f0" Interface "enp6s0f0" Port "vxlan0" Interface "vxlan0" type: vxlan options: {key="1", local_ip="10.0.0.5", remote_ip="10.0.0.4"} ovs_version: "2.5.0" ======== (where 10.0.0.5 is an address on enp6s0f1) and sending traffic across it will lead to the following panic: ======== general protection fault: 0000 [#1] SMP PTI CPU: 5 PID: 0 Comm: swapper/5 Not tainted 4.20.0-rc3-ehc+ #701 Hardware name: Dell Inc. PowerEdge R710/0M233H, BIOS 6.4.0 07/23/2013 RIP: 0010:dev_hard_start_xmit+0x38/0x200 Code: 53 48 89 fb 48 83 ec 20 48 85 ff 48 89 54 24 08 48 89 4c 24 18 0f 84 ab 01 00 00 48 8d 86 90 00 00 00 48 89 f5 48 89 44 24 10 <4c> 8b 33 48 c7 03 00 00 00 00 48 8b 05 c7 d1 b3 00 4d 85 f6 0f 95 RSP: 0018:ffff888627b437e0 EFLAGS: 00010202 RAX: 0000000000000000 RBX: dead000000000100 RCX: ffff88862279c000 RDX: ffff888614a342c0 RSI: 0000000000000000 RDI: 0000000000000000 RBP: ffff888618a88000 R08: 0000000000000001 R09: 00000000000003e8 R10: 0000000000000000 R11: ffff888614a34140 R12: 0000000000000000 R13: 0000000000000062 R14: dead000000000100 R15: ffff888616430000 FS: 0000000000000000(0000) GS:ffff888627b40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f6d2bc6d000 CR3: 000000000200a000 CR4: 00000000000006e0 Call Trace: <IRQ> __dev_queue_xmit+0x623/0x870 ? masked_flow_lookup+0xf7/0x220 [openvswitch] ? ep_poll_callback+0x101/0x310 do_execute_actions+0xaba/0xaf0 [openvswitch] ? __wake_up_common+0x8a/0x150 ? __wake_up_common_lock+0x87/0xc0 ? queue_userspace_packet+0x31c/0x5b0 [openvswitch] ovs_execute_actions+0x47/0x120 [openvswitch] ovs_dp_process_packet+0x7d/0x110 [openvswitch] ovs_vport_receive+0x6e/0xd0 [openvswitch] ? dst_alloc+0x64/0x90 ? rt_dst_alloc+0x50/0xd0 ? ip_route_input_slow+0x19a/0x9a0 ? __udp_enqueue_schedule_skb+0x198/0x1b0 ? __udp4_lib_rcv+0x856/0xa30 ? __udp4_lib_rcv+0x856/0xa30 ? cpumask_next_and+0x19/0x20 ? find_busiest_group+0x12d/0xcd0 netdev_frame_hook+0xce/0x150 [openvswitch] __netif_receive_skb_core+0x205/0xae0 __netif_receive_skb_list_core+0x11e/0x220 netif_receive_skb_list+0x203/0x460 ? __efx_rx_packet+0x335/0x5e0 [sfc] efx_poll+0x182/0x320 [sfc] net_rx_action+0x294/0x3c0 __do_softirq+0xca/0x297 irq_exit+0xa6/0xb0 do_IRQ+0x54/0xd0 common_interrupt+0xf/0xf </IRQ> ======== So, in all listified-receive handling, instead pull skbs off the lists with skb_list_del_init(). Fixes: 9af86f9 ("net: core: fix use-after-free in __netif_receive_skb_list_core") Fixes: 7da517a ("net: core: Another step of skb receive list processing") Fixes: a4ca8b7 ("net: ipv4: fix drop handling in ip_list_rcv() and ip_list_rcv_finish()") Fixes: d8269e2 ("net: ipv6: listify ipv6_rcv() and ip6_rcv_finish()") Signed-off-by: Edward Cree <ecree@solarflare.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 64d4790 commit 22f6bbb

File tree

3 files changed

+8
-8
lines changed

3 files changed

+8
-8
lines changed

net/core/dev.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5014,7 +5014,7 @@ static void __netif_receive_skb_list_core(struct list_head *head, bool pfmemallo
50145014
struct net_device *orig_dev = skb->dev;
50155015
struct packet_type *pt_prev = NULL;
50165016

5017-
list_del(&skb->list);
5017+
skb_list_del_init(skb);
50185018
__netif_receive_skb_core(skb, pfmemalloc, &pt_prev);
50195019
if (!pt_prev)
50205020
continue;
@@ -5170,7 +5170,7 @@ static void netif_receive_skb_list_internal(struct list_head *head)
51705170
INIT_LIST_HEAD(&sublist);
51715171
list_for_each_entry_safe(skb, next, head, list) {
51725172
net_timestamp_check(netdev_tstamp_prequeue, skb);
5173-
list_del(&skb->list);
5173+
skb_list_del_init(skb);
51745174
if (!skb_defer_rx_timestamp(skb))
51755175
list_add_tail(&skb->list, &sublist);
51765176
}
@@ -5181,7 +5181,7 @@ static void netif_receive_skb_list_internal(struct list_head *head)
51815181
rcu_read_lock();
51825182
list_for_each_entry_safe(skb, next, head, list) {
51835183
xdp_prog = rcu_dereference(skb->dev->xdp_prog);
5184-
list_del(&skb->list);
5184+
skb_list_del_init(skb);
51855185
if (do_xdp_generic(xdp_prog, skb) == XDP_PASS)
51865186
list_add_tail(&skb->list, &sublist);
51875187
}
@@ -5200,7 +5200,7 @@ static void netif_receive_skb_list_internal(struct list_head *head)
52005200

52015201
if (cpu >= 0) {
52025202
/* Will be handled, remove from list */
5203-
list_del(&skb->list);
5203+
skb_list_del_init(skb);
52045204
enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
52055205
}
52065206
}

net/ipv4/ip_input.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ static void ip_list_rcv_finish(struct net *net, struct sock *sk,
547547
list_for_each_entry_safe(skb, next, head, list) {
548548
struct dst_entry *dst;
549549

550-
list_del(&skb->list);
550+
skb_list_del_init(skb);
551551
/* if ingress device is enslaved to an L3 master device pass the
552552
* skb to its handler for processing
553553
*/
@@ -594,7 +594,7 @@ void ip_list_rcv(struct list_head *head, struct packet_type *pt,
594594
struct net_device *dev = skb->dev;
595595
struct net *net = dev_net(dev);
596596

597-
list_del(&skb->list);
597+
skb_list_del_init(skb);
598598
skb = ip_rcv_core(skb, net);
599599
if (skb == NULL)
600600
continue;

net/ipv6/ip6_input.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
9595
list_for_each_entry_safe(skb, next, head, list) {
9696
struct dst_entry *dst;
9797

98-
list_del(&skb->list);
98+
skb_list_del_init(skb);
9999
/* if ingress device is enslaved to an L3 master device pass the
100100
* skb to its handler for processing
101101
*/
@@ -296,7 +296,7 @@ void ipv6_list_rcv(struct list_head *head, struct packet_type *pt,
296296
struct net_device *dev = skb->dev;
297297
struct net *net = dev_net(dev);
298298

299-
list_del(&skb->list);
299+
skb_list_del_init(skb);
300300
skb = ip6_rcv_core(skb, dev, net);
301301
if (skb == NULL)
302302
continue;

0 commit comments

Comments
 (0)