Skip to contentSkip to conversation

Patch "net: do not process device backlog during unregistration" has been added to the 3.14-stable tree

PATCH [email protected] 1 participant 1 message Sep 26, 2015

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

net: do not process device backlog during unregistration

to the 3.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
net-do-not-process-device-backlog-during-unregistration.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From foo@baz Sat Sep 26 11:19:08 PDT 2015

From: Julian Anastasov <[email protected]>
Date: Thu, 9 Jul 2015 09:59:09 +0300
Subject: net: do not process device backlog during unregistration

From: Julian Anastasov <[email protected]>

[ Upstream commit e9e4dd3267d0c5234c5c0f47440456b10875dec9 ]

commit 381c759d9916 ("ipv4: Avoid crashing in ip_error") fixes a problem where processed packet comes from device with destroyed inetdev (dev->ip_ptr). This is not expected because inetdev_destroy is called in NETDEV_UNREGISTER phase and packets should not be processed after dev_close_many() and synchronize_net(). Above fix is still required because inetdev_destroy can be called for other reasons. But it shows the real problem: backlog can keep packets for long time and they do not hold reference to device. Such packets are then delivered to upper levels at the same time when device is unregistered.
Calling flush_backlog after NETDEV_UNREGISTER_FINAL still accounts all packets from backlog but before that some packets continue to be delivered to upper levels long after the synchronize_net call which is supposed to wait the last ones. Also, as Eric pointed out, processed packets, mostly from other devices, can continue to add new packets to backlog.

Fix the problem by moving flush_backlog early, after the device driver is stopped and before the synchronize_net() call. Then use netif_running check to make sure we do not add more packets to backlog. We have to do it in enqueue_to_backlog context when the local IRQ is disabled. As result, after the flush_backlog and synchronize_net sequence all packets should be accounted.

Thanks to Eric W. Biederman for the test script and his valuable feedback!

Fixes 6e583ce5242f ("net: eliminate refcounting in backlog queue")
Cc Eric W. Biederman <[email protected]>
Cc Stephen Hemminger <[email protected]>
---
 net/core/dev.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
net/core/dev.c +4 -2
@@ -3214,6 +3214,8 @@ static int enqueue_to_backlog(struct sk_
 	local_irq_save(flags);
 
 	rps_lock(sd);
+	if (!netif_running(skb->dev))
+		goto drop;
 	qlen = skb_queue_len(&sd->input_pkt_queue);
 	if (qlen <= netdev_max_backlog && !skb_flow_limit(skb, qlen)) {
 		if (skb_queue_len(&sd->input_pkt_queue)) {
@@ -3235,6 +3237,7 @@ enqueue:
 		goto enqueue;
 	}
 
+drop:
 	sd->dropped++;
 	rps_unlock(sd);
 
@@ -5694,6 +5697,7 @@ static void rollback_registered_many(str
 		unlist_netdevice(dev);
 
 		dev->reg_state = NETREG_UNREGISTERING;
+		on_each_cpu(flush_backlog, dev, 1);
 	}
 
 	synchronize_net();
@@ -6310,8 +6314,6 @@ void netdev_run_todo(void)
 
 		dev->reg_state = NETREG_UNREGISTERED;
 
-		on_each_cpu(flush_backlog, dev, 1);
-
 		netdev_wait_allrefs(dev);
 
 		/* paranoia */

Patches currently in stable-queue which might be from [email protected] are

queue-3.14/net-call-rcu_read_lock-early-in-process_backlog.patch queue-3.14/net-do-not-process-device-backlog-during-unregistration.patch