From e520a0ec7a758fda4db3d6ff0e78ab2c0013225e Mon Sep 17 00:00:00 2001 From: mpi Date: Mon, 27 Apr 2015 14:51:44 +0000 Subject: [PATCH] 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@ --- sys/netinet6/in6_ifattach.c | 13 +++---------- sys/netinet6/nd6.c | 20 ++------------------ sys/netinet6/nd6_rtr.c | 5 +++-- 3 files changed, 8 insertions(+), 30 deletions(-) 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); } -- 2.20.1