| From e9e4dd3267d0c5234c5c0f47440456b10875dec9 Mon Sep 17 00:00:00 2001 |
| From: Julian Anastasov <ja@ssi.bg> |
| Date: Thu, 9 Jul 2015 09:59:09 +0300 |
| Subject: net: do not process device backlog during unregistration |
| |
| commit e9e4dd3267d0c5234c5c0f47440456b10875dec9 upstream. |
| |
| 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! |
| |
| Reported-by: Vittorio Gambaletta <linuxbugs@vittgam.net> |
| Fixes: 6e583ce5242f ("net: eliminate refcounting in backlog queue") |
| Cc: Eric W. Biederman <ebiederm@xmission.com> |
| Cc: Stephen Hemminger <stephen@networkplumber.org> |
| Signed-off-by: Julian Anastasov <ja@ssi.bg> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| [lizf: Backported to 3.4: adjust context] |
| Signed-off-by: Zefan Li <lizefan@huawei.com> |
| --- |
| net/core/dev.c | 6 ++++-- |
| 1 file changed, 4 insertions(+), 2 deletions(-) |
| |
| --- a/net/core/dev.c |
| +++ b/net/core/dev.c |
| @@ -2880,6 +2880,8 @@ static int enqueue_to_backlog(struct sk_ |
| local_irq_save(flags); |
| |
| rps_lock(sd); |
| + if (!netif_running(skb->dev)) |
| + goto drop; |
| if (skb_queue_len(&sd->input_pkt_queue) <= netdev_max_backlog) { |
| if (skb_queue_len(&sd->input_pkt_queue)) { |
| enqueue: |
| @@ -2900,6 +2902,7 @@ enqueue: |
| goto enqueue; |
| } |
| |
| +drop: |
| sd->dropped++; |
| rps_unlock(sd); |
| |
| @@ -5228,6 +5231,7 @@ static void rollback_registered_many(str |
| unlist_netdevice(dev); |
| |
| dev->reg_state = NETREG_UNREGISTERING; |
| + on_each_cpu(flush_backlog, dev, 1); |
| } |
| |
| synchronize_net(); |
| @@ -5791,8 +5795,6 @@ void netdev_run_todo(void) |
| |
| dev->reg_state = NETREG_UNREGISTERED; |
| |
| - on_each_cpu(flush_backlog, dev, 1); |
| - |
| netdev_wait_allrefs(dev); |
| |
| /* paranoia */ |