From: mpi <mpi@openbsd.org>
Date: Mon, 27 Apr 2015 14:51:44 +0000 (+0000)
Subject: Do not call nd6_purge() before purging the IPv6 addresses of a detached
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e520a0ec7a758fda4db3d6ff0e78ab2c0013225e;p=openbsd

Do not call nd6_purge() before purging the IPv6 addresses of a detached
interface.

Fix a use after free introduced in r1.98 of netinet6/in6.c and recently
exposed by a crazy pool/malloc damage finder being currently refined by
dlg@ and deraadt@.

ok mikeb@, henning@
---

diff --git a/sys/netinet6/in6_ifattach.c b/sys/netinet6/in6_ifattach.c
index 983b4c03eea..1aa40a7baf2 100644
--- a/sys/netinet6/in6_ifattach.c
+++ b/sys/netinet6/in6_ifattach.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in6_ifattach.c,v 1.86 2015/03/14 03:38:52 jsg Exp $	*/
+/*	$OpenBSD: in6_ifattach.c,v 1.87 2015/04/27 14:51:44 mpi Exp $	*/
 /*	$KAME: in6_ifattach.c,v 1.124 2001/07/18 08:32:51 jinmei Exp $	*/
 
 /*
@@ -600,9 +600,6 @@ in6_ifdetach(struct ifnet *ifp)
 	ip6_mrouter_detach(ifp);
 #endif
 
-	/* remove neighbor management table */
-	nd6_purge(ifp);
-
 	/* nuke any of IPv6 addresses we have */
 	TAILQ_FOREACH_SAFE(ifa, &ifp->if_addrlist, ifa_list, next) {
 		if (ifa->ifa_addr->sa_family != AF_INET6)
@@ -612,12 +609,8 @@ in6_ifdetach(struct ifnet *ifp)
 	}
 
 	/*
-	 * remove neighbor management table.  we call it twice just to make
-	 * sure we nuke everything.  maybe we need just one call.
-	 * XXX: since the first call did not release addresses, some prefixes
-	 * might remain.  We should call nd6_purge() again to release the
-	 * prefixes after removing all addresses above.
-	 * (Or can we just delay calling nd6_purge until at this point?)
+	 * Remove neighbor management table.  Must be called after
+	 * purging addresses.
 	 */
 	nd6_purge(ifp);
 
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index 607f1a6073e..971a8948a1a 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6.c,v 1.134 2015/04/17 11:04:02 mikeb Exp $	*/
+/*	$OpenBSD: nd6.c,v 1.135 2015/04/27 14:51:44 mpi Exp $	*/
 /*	$KAME: nd6.c,v 1.280 2002/06/08 19:52:07 itojun Exp $	*/
 
 /*
@@ -590,24 +590,8 @@ nd6_purge(struct ifnet *ifp)
 
 	/* Nuke prefix list entries toward ifp */
 	LIST_FOREACH_SAFE(pr, &nd_prefix, ndpr_entry, npr) {
-		if (pr->ndpr_ifp == ifp) {
-			/*
-			 * Because if_detach() does *not* release prefixes
-			 * while purging addresses the reference count will
-			 * still be above zero. We therefore reset it to
-			 * make sure that the prefix really gets purged.
-			 */
-			pr->ndpr_refcnt = 0;
-			/*
-			 * Previously, pr->ndpr_addr is removed as well,
-			 * but I strongly believe we don't have to do it.
-			 * nd6_purge() is only called from in6_ifdetach(),
-			 * which removes all the associated interface addresses
-			 * by itself.
-			 * (jinmei@kame.net 20010129)
-			 */
+		if (pr->ndpr_ifp == ifp)
 			prelist_remove(pr);
-		}
 	}
 
 	if (ifp->if_xflags & IFXF_AUTOCONF6) {
diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index e1e7cb5b59c..8d6637fbb8e 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_rtr.c,v 1.101 2015/03/25 17:39:33 florian Exp $	*/
+/*	$OpenBSD: nd6_rtr.c,v 1.102 2015/04/27 14:51:44 mpi Exp $	*/
 /*	$KAME: nd6_rtr.c,v 1.97 2001/02/07 11:09:13 itojun Exp $	*/
 
 /*
@@ -1452,7 +1452,8 @@ nd6_addr_add(void *prptr)
 		pfxlist_onlink_check();
 
 	/* Decrement prefix refcount now that the task is done. */
-	pr->ndpr_refcnt--;
+	if (--pr->ndpr_refcnt == 0)
+		prelist_remove(pr);
 
 	splx(s);
 }