netdev - Re: [PATCH net] ipv4: Avoid crashing in ip

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] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.11.1505240858440.1473@ja.home.ssi.bg>
Date:	Sun, 24 May 2015 13:52:35 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Eric Dumazet <eric.dumazet@...il.com>
cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	"Vittorio G (VittGam)" <linuxbugs@...tgam.net>,
	security@...nel.org, Greg KH <greg@...ah.com>, luto@...capital.net,
	Willy Tarreau <w@....eu>
Subject: Re: [PATCH net] ipv4: Avoid crashing in ip_error


	Hello,

On Sat, 23 May 2015, Eric Dumazet wrote:

> > 	Not sure if that can help but both RCU_INIT_POINTER
> > calls in veth_dellink can be before the unregister_netdevice_queue
> > ones. May be it will help for the case when veth_dellink is
> > called with head=NULL. As result, veth_xmit will free all
> > flying skbs to peer that is unregistered first.
> 
> Good point, but note that we never call dellink() method with head==NULL
> in current kernels.

	Can it be a general problem with process_backlog
delivering packets even after the ndo_stop call?
It can happen even for devices that use napi_disable.
synchronize_net simply misses packets processed on
another CPU because process_backlog is not always in
RCU read section.

	What about moving flush_backlog before
NETDEV_UNREGISTER, it was after NETDEV_UNREGISTER_FINAL.
It will work like a synchronize_net call.

rollback_registered_many:
1. __dev_close_many: Commit netif_running, ndo_stop
2. on_each_cpu(flush_backlog, dev, 1): may be its place
can be additionally tuned...
3. synchronize_net: not needed anymore?
4. call_netdevice_notifiers(NETDEV_UNREGISTER, dev)

Compile-tested only:

diff --git a/net/core/dev.c b/net/core/dev.c
index c7ba038..e402661 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6034,6 +6036,7 @@ static void rollback_registered_many(struct list_head *head)
 		unlist_netdevice(dev);
 
 		dev->reg_state = NETREG_UNREGISTERING;
+		on_each_cpu(flush_backlog, dev, 1);
 	}
 
 	synchronize_net();
@@ -6657,8 +6660,6 @@ void netdev_run_todo(void)
 
 		dev->reg_state = NETREG_UNREGISTERED;
 
-		on_each_cpu(flush_backlog, dev, 1);
-
 		netdev_wait_allrefs(dev);
 
 		/* paranoia */
Regards

--
Julian Anastasov <ja@....bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html