Do not clean the multicast records of an interface when it is destroyed

(unplugged).  Even if it makes no sense to keep them around if the
interface is no more, we cannot safely remove them since pcb multicast
options might keep a pointer to them.

Fixes a user after free introduced by the multicast address linking
rewrite and reported by Alexey Suslikov, thanks!

ok claudio@

4 files changed, 37 insertions, 35 deletions

diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c
index 51fbb15fc38..2babdd30dfd 100644
--- a/sys/netinet/igmp.c
+++ b/sys/netinet/igmp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: igmp.c,v 1.36 2013/11/28 10:16:44 mpi Exp $	*/
+/*	$OpenBSD: igmp.c,v 1.37 2014/01/21 10:18:26 mpi Exp $	*/
 /*	$NetBSD: igmp.c,v 1.15 1996/02/13 23:41:25 christos Exp $	*/
 
 /*
@@ -128,7 +128,7 @@ rti_fill(struct in_multi *inm)
 	struct router_info *rti;
 
 	for (rti = rti_head; rti != 0; rti = rti->rti_next) {
-		if (rti->rti_ifp == inm->inm_ifp) {
+		if (rti->rti_ifp->if_index == inm->inm_ifidx) {
 			inm->inm_rti = rti;
 			if (rti->rti_type == IGMP_v1_ROUTER)
 				return (IGMP_v1_HOST_MEMBERSHIP_REPORT);
@@ -141,7 +141,7 @@ rti_fill(struct in_multi *inm)
 					   M_MRTABLE, M_NOWAIT);
 	if (rti == NULL)
 		return (-1);
-	rti->rti_ifp = inm->inm_ifp;
+	rti->rti_ifp = if_get(inm->inm_ifidx);
 	rti->rti_type = IGMP_v2_ROUTER;
 	rti->rti_next = rti_head;
 	rti_head = rti;
@@ -462,13 +462,16 @@ igmp_input(struct mbuf *m, ...)
 void
 igmp_joingroup(struct in_multi *inm)
 {
-	int i, s = splsoftnet();
+	struct ifnet* ifp;
+	int i, s;
+
+	ifp = if_get(inm->inm_ifidx);
+	s = splsoftnet();
 
 	inm->inm_state = IGMP_IDLE_MEMBER;
 
 	if (!IN_LOCAL_GROUP(inm->inm_addr.s_addr) &&
-	    inm->inm_ifp &&
-	    (inm->inm_ifp->if_flags & IFF_LOOPBACK) == 0) {
+	    ifp && (ifp->if_flags & IFF_LOOPBACK) == 0) {
 		if ((i = rti_fill(inm)) == -1) {
 			splx(s);
 			return;
@@ -486,15 +489,17 @@ igmp_joingroup(struct in_multi *inm)
 void
 igmp_leavegroup(struct in_multi *inm)
 {
+	struct ifnet* ifp;
+	int s;
 
-	int s = splsoftnet();
+	ifp = if_get(inm->inm_ifidx);
+	s = splsoftnet();
 
 	switch (inm->inm_state) {
 	case IGMP_DELAYING_MEMBER:
 	case IGMP_IDLE_MEMBER:
 		if (!IN_LOCAL_GROUP(inm->inm_addr.s_addr) &&
-		    inm->inm_ifp &&
-		    (inm->inm_ifp->if_flags & IFF_LOOPBACK) == 0)
+		    ifp && (ifp->if_flags & IFF_LOOPBACK) == 0)
 			if (inm->inm_rti->rti_type != IGMP_v1_ROUTER)
 				igmp_sendpkt(inm, IGMP_HOST_LEAVE_MESSAGE,
 				    INADDR_ALLROUTERS_GROUP);
@@ -616,7 +621,7 @@ igmp_sendpkt(struct in_multi *inm, int type, in_addr_t addr)
 	m->m_data -= sizeof(struct ip);
 	m->m_len += sizeof(struct ip);
 
-	imo.imo_multicast_ifp = inm->inm_ifp;
+	imo.imo_multicast_ifp = if_get(inm->inm_ifidx);
 	imo.imo_multicast_ttl = 1;
 
 	/*
diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index a12b241d27a..d552303747d 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in.c,v 1.90 2013/12/31 03:24:44 tedu Exp $	*/
+/*	$OpenBSD: in.c,v 1.91 2014/01/21 10:18:26 mpi Exp $	*/
 /*	$NetBSD: in.c,v 1.26 1996/02/13 23:41:39 christos Exp $	*/
 
 /*
@@ -949,7 +949,7 @@ in_addmulti(struct in_addr *ap, struct ifnet *ifp)
 		inm->inm_sin.sin_family = AF_INET;
 		inm->inm_sin.sin_addr = *ap;
 		inm->inm_refcnt = 1;
-		inm->inm_ifp = ifp;
+		inm->inm_ifidx = ifp->if_index;
 		inm->inm_ifma.ifma_addr = sintosa(&inm->inm_sin);
 
 		/*
@@ -992,20 +992,24 @@ in_delmulti(struct in_multi *inm)
 		 * we are leaving the multicast group.
 		 */
 		igmp_leavegroup(inm);
-		ifp = inm->inm_ifp;
+		ifp = if_get(inm->inm_ifidx);
 
 		/*
 		 * Notify the network driver to update its multicast
 		 * reception filter.
 		 */
-		satosin(&ifr.ifr_addr)->sin_len = sizeof(struct sockaddr_in);
-		satosin(&ifr.ifr_addr)->sin_family = AF_INET;
-		satosin(&ifr.ifr_addr)->sin_addr = inm->inm_addr;
-		(*ifp->if_ioctl)(ifp, SIOCDELMULTI, (caddr_t)&ifr);
-
-		s = splsoftnet();
-		TAILQ_REMOVE(&ifp->if_maddrlist, &inm->inm_ifma, ifma_list);
-		splx(s);
+		if (ifp != NULL) {
+			satosin(&ifr.ifr_addr)->sin_len =
+			    sizeof(struct sockaddr_in);
+			satosin(&ifr.ifr_addr)->sin_family = AF_INET;
+			satosin(&ifr.ifr_addr)->sin_addr = inm->inm_addr;
+			(*ifp->if_ioctl)(ifp, SIOCDELMULTI, (caddr_t)&ifr);
+
+			s = splsoftnet();
+			TAILQ_REMOVE(&ifp->if_maddrlist, &inm->inm_ifma,
+			    ifma_list);
+			splx(s);
+		}
 
 		free(inm, M_IPMADDR);
 	}
@@ -1017,7 +1021,6 @@ void
 in_ifdetach(struct ifnet *ifp)
 {
 	struct ifaddr *ifa, *next;
-	struct ifmaddr *ifma, *mnext;
 
 	/* nuke any of IPv4 addresses we have */
 	TAILQ_FOREACH_SAFE(ifa, &ifp->if_addrlist, ifa_list, next) {
@@ -1025,12 +1028,4 @@ in_ifdetach(struct ifnet *ifp)
 			continue;
 		in_purgeaddr(ifa);
 	}
-
-	TAILQ_FOREACH_SAFE(ifma, &ifp->if_maddrlist, ifma_list, mnext) {
-		if (ifma->ifma_addr->sa_family != AF_INET)
-			continue;
-
-		ifma->ifma_refcnt = 1;
-		in_delmulti(ifmatoinm(ifma));
-	}
 }
diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h
index 91a233a1bab..a3c388fe3e1 100644
--- a/sys/netinet/in_var.h
+++ b/sys/netinet/in_var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in_var.h,v 1.32 2014/01/13 09:48:12 mpi Exp $	*/
+/*	$OpenBSD: in_var.h,v 1.33 2014/01/21 10:18:26 mpi Exp $	*/
 /*	$NetBSD: in_var.h,v 1.16 1996/02/13 23:42:15 christos Exp $	*/
 
 /*
@@ -115,7 +115,7 @@ struct router_info {
 struct in_multi {
 	struct ifmaddr		inm_ifma;   /* Protocol-independent info */
 #define inm_refcnt		inm_ifma.ifma_refcnt
-#define inm_ifp			inm_ifma.ifma_ifp
+#define inm_ifidx		inm_ifma.ifma_ifidx
 
 	struct sockaddr_in	inm_sin;   /* IPv4 multicast address */
 #define inm_addr		inm_sin.sin_addr
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index 1c1c0b534a7..ffd49ec9ba5 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_output.c,v 1.254 2014/01/09 06:29:06 tedu Exp $	*/
+/*	$OpenBSD: ip_output.c,v 1.255 2014/01/21 10:18:26 mpi Exp $	*/
 /*	$NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $	*/
 
 /*
@@ -1833,7 +1833,8 @@ ip_setmoptions(int optname, struct ip_moptions **imop, struct mbuf *m,
 		 * membership slots are full.
 		 */
 		for (i = 0; i < imo->imo_num_memberships; ++i) {
-			if (imo->imo_membership[i]->inm_ifp == ifp &&
+			if (imo->imo_membership[i]->inm_ifidx
+						== ifp->if_index &&
 			    imo->imo_membership[i]->inm_addr.s_addr
 						== mreq->imr_multiaddr.s_addr)
 				break;
@@ -1916,7 +1917,8 @@ ip_setmoptions(int optname, struct ip_moptions **imop, struct mbuf *m,
 		 */
 		for (i = 0; i < imo->imo_num_memberships; ++i) {
 			if ((ifp == NULL ||
-			     imo->imo_membership[i]->inm_ifp == ifp) &&
+			    imo->imo_membership[i]->inm_ifidx ==
+			        ifp->if_index) &&
 			     imo->imo_membership[i]->inm_addr.s_addr ==
 			     mreq->imr_multiaddr.s_addr)
 				break;