Fix a route use after free in IPv6 multicast route. Move the

mrt6_mcast6_del() out of the rtable_walk().  This avoids recursion
to prevent stack overflow.  Also it allows freeing the route outside
of the walk.  Now mrt6_mcast_del() frees the route only when it is
deleted from the routing table.  If that fails, it must not be
freed.  After the route is returned by mf6c_find(), it is reference
counted.  Then we need a rtfree(), but not in the other case.
Name mrt6_mcast_add() and mrt6_mcast_del() consistently.
Move rt_timer_remove_all() into mrt6_mcast_del().
Reported-by: syzbot+af7d510593d74c825960@syzkaller.appspotmail.com
OK mpi@

1 files changed, 36 insertions, 35 deletions

diff --git a/sys/netinet6/ip6_mroute.c b/sys/netinet6/ip6_mroute.c
index 4188f58506b..d30d7cd29cf 100644
--- a/sys/netinet6/ip6_mroute.c
+++ b/sys/netinet6/ip6_mroute.c
@@ -174,9 +174,9 @@ int del_m6fc(struct socket *, struct mf6cctl *);
 struct ifnet *mrt6_iflookupbymif(mifi_t, unsigned int);
 struct rtentry *mf6c_find(struct ifnet *, struct in6_addr *,
     struct in6_addr *, unsigned int);
-struct rtentry *mrt6_mcast6_add(struct ifnet *, struct sockaddr *,
+struct rtentry *mrt6_mcast_add(struct ifnet *, struct sockaddr *,
     struct sockaddr *);
-int mrt6_mcast6_del(struct rtentry *, unsigned int);
+void mrt6_mcast_del(struct rtentry *, unsigned int);
 void mf6c_expire_route(struct rtentry *, struct rttimer *);
 
 /*
@@ -505,11 +505,7 @@ mrouter6_rtwalk_delete(struct rtentry *rt, void *arg, unsigned int rtableid)
 	    (RTF_HOST | RTF_MULTICAST))
 		return 0;
 
-	/* Remove all timers related to this route. */
-	rt_timer_remove_all(rt);
-	mrt6_mcast6_del(rt, rtableid);
-
-	return 0;
+	return EEXIST;
 }
 
 /*
@@ -521,11 +517,22 @@ ip6_mrouter_done(struct socket *so)
 	struct inpcb *inp = sotoinpcb(so);
 	struct ifnet *ifp;
 	unsigned int rtableid = inp->inp_rtableid;
+	int error;
 
 	NET_ASSERT_LOCKED();
 
 	/* Delete all remaining installed multicast routes. */
-	rtable_walk(rtableid, AF_INET6, NULL, mrouter6_rtwalk_delete, NULL);
+	do {
+		struct rtentry *rt = NULL;
+
+		error = rtable_walk(rtableid, AF_INET6, &rt,
+		    mrouter6_rtwalk_delete, NULL);
+		if (rt != NULL && error == EEXIST) {
+			mrt6_mcast_del(rt, rtableid);
+			error = EAGAIN;
+		}
+		rtfree(rt);
+	} while (error == EAGAIN);
 
 	/* Unregister all interfaces in the domain. */
 	TAILQ_FOREACH(ifp, &ifnet, if_list) {
@@ -655,7 +662,7 @@ mf6c_add_route(struct ifnet *ifp, struct sockaddr *origin,
 	char bsrc[INET6_ADDRSTRLEN], bdst[INET6_ADDRSTRLEN];
 #endif /* MCAST_DEBUG */
 
-	rt = mrt6_mcast6_add(ifp, origin, group);
+	rt = mrt6_mcast_add(ifp, origin, group);
 	if (rt == NULL)
 		return ENOENT;
 
@@ -665,7 +672,8 @@ mf6c_add_route(struct ifnet *ifp, struct sockaddr *origin,
 		    inet_ntop(AF_INET6, origin, bsrc, sizeof(bsrc)),
 		    inet_ntop(AF_INET6, group, bdst, sizeof(bdst)),
 		    mf6cc->mf6cc_parent, ifp->if_xname);
-		mrt6_mcast6_del(rt, rtableid);
+		mrt6_mcast_del(rt, rtableid);
+		rtfree(rt);
 		return ENOMEM;
 	}
 
@@ -720,9 +728,8 @@ mf6c_update(struct mf6cctl *mf6cc, int wait, unsigned int rtableid)
 			    inet_ntop(AF_INET6,
 			    &mf6cc->mf6cc_mcastgrp.sin6_addr, bdst,
 			    sizeof(bdst)), mifi, ifp->if_xname);
-
-			rt_timer_remove_all(rt);
-			mrt6_mcast6_del(rt, rtableid);
+			mrt6_mcast_del(rt, rtableid);
+			rtfree(rt);
 			continue;
 		}
 
@@ -834,9 +841,8 @@ del_m6fc(struct socket *so, struct mf6cctl *mfccp)
 
 	while ((rt = mf6c_find(NULL, &mfccp->mf6cc_origin.sin6_addr,
 	    &mfccp->mf6cc_mcastgrp.sin6_addr, rtableid)) != NULL) {
-		/* Remove all timers related to this route. */
-		rt_timer_remove_all(rt);
-		mrt6_mcast6_del(rt, rtableid);
+		mrt6_mcast_del(rt, rtableid);
+		rtfree(rt);
 	}
 
 	return 0;
@@ -1005,8 +1011,7 @@ mf6c_expire_route(struct rtentry *rt, struct rttimer *rtt)
 		return;
 	}
 
-	rt_timer_remove_all(rt);
-	mrt6_mcast6_del(rt, rtableid);
+	mrt6_mcast_del(rt, rtableid);
 }
 
 /*
@@ -1230,7 +1235,7 @@ mf6c_find(struct ifnet *ifp, struct in6_addr *origin, struct in6_addr *group,
 }
 
 struct rtentry *
-mrt6_mcast6_add(struct ifnet *ifp, struct sockaddr *origin,
+mrt6_mcast_add(struct ifnet *ifp, struct sockaddr *origin,
     struct sockaddr *group)
 {
 	struct ifaddr *ifa;
@@ -1256,28 +1261,24 @@ mrt6_mcast6_add(struct ifnet *ifp, struct sockaddr *origin,
 	return mf6c_find(ifp, NULL, &satosin6(group)->sin6_addr, rtableid);
 }
 
-int
-mrt6_mcast6_del(struct rtentry *rt, unsigned int rtableid)
+void
+mrt6_mcast_del(struct rtentry *rt, unsigned int rtableid)
 {
 	struct ifnet *ifp;
-	int rv;
+	int error;
+
+	/* Remove all timers related to this route. */
+	rt_timer_remove_all(rt);
 
 	free(rt->rt_llinfo, M_MRTABLE, sizeof(struct mf6c));
 	rt->rt_llinfo = NULL;
 
-	if ((ifp = if_get(rt->rt_ifidx)) == NULL) {
-		DPRINTF("if_get(%d) failed", rt->rt_ifidx);
-		rtfree(rt);
-		return ENOENT;
-	}
-
-	rv = rtdeletemsg(rt, ifp, rtableid);
+	ifp = if_get(rt->rt_ifidx);
+	if (ifp == NULL)
+		return;
+	error = rtdeletemsg(rt, ifp, rtableid);
 	if_put(ifp);
-	if (rv != 0) {
-		DPRINTF("rtdeletemsg failed %d", rv);
-		rtfree(rt);
-		return rv;
-	}
 
-	return 0;
+	if (error)
+		DPRINTF("delete route error %d\n", error);
 }