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 983b4c0..1aa40a7 100644 (file)
--- 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 607f1a6..971a894 100644 (file)
--- 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 e1e7cb5..8d6637f 100644 (file)
--- 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);
 }