linux-kernel - [PATCH 4.1 124/159] net: do not process device backlog during unregistration

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: <20150926205320.815881542@linuxfoundation.org>
Date:	Sat, 26 Sep 2015 13:56:10 -0700
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	linux-kernel@...r.kernel.org
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	stable@...r.kernel.org,
	Vittorio Gambaletta <linuxbugs@...tgam.net>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Stephen Hemminger <stephen@...workplumber.org>,
	Julian Anastasov <ja@....bg>,
	"David S. Miller" <davem@...emloft.net>
Subject: [PATCH 4.1 124/159] net: do not process device backlog during unregistration

4.1-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Julian Anastasov <ja@....bg>

[ 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!

Reported-by: Vittorio Gambaletta <linuxbugs@...tgam.net>
Fixes: 6e583ce5242f ("net: eliminate refcounting in backlog queue")
Cc: Eric W. Biederman <ebiederm@...ssion.com>
Cc: Stephen Hemminger <stephen@...workplumber.org>
Signed-off-by: Julian Anastasov <ja@....bg>
Signed-off-by: David S. Miller <davem@...emloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 net/core/dev.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3337,6 +3337,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 (qlen) {
@@ -3358,6 +3360,7 @@ enqueue:
 		goto enqueue;
 	}

+drop:
 	sd->dropped++;
 	rps_unlock(sd);

@@ -6023,6 +6026,7 @@ static void rollback_registered_many(str
 		unlist_netdevice(dev);

 		dev->reg_state = NETREG_UNREGISTERING;
+		on_each_cpu(flush_backlog, dev, 1);
 	}

 	synchronize_net();
@@ -6647,8 +6651,6 @@ void netdev_run_todo(void)

 		dev->reg_state = NETREG_UNREGISTERED;

-		on_each_cpu(flush_backlog, dev, 1);
-
 		netdev_wait_allrefs(dev);

 		/* paranoia */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/