Do not manually decrement rt's refcounter in nd6_lookup() and let the

callers rtfree(9) it.

Inputs and ok bluhm@

5 files changed, 75 insertions, 50 deletions

diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c
index add6c62a814..9d3f1c5453a 100644
--- a/sys/netinet6/icmp6.c
+++ b/sys/netinet6/icmp6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: icmp6.c,v 1.171 2015/09/11 22:00:36 claudio Exp $	*/
+/*	$OpenBSD: icmp6.c,v 1.172 2015/09/18 14:26:22 mpi Exp $	*/
 /*	$KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $	*/
 
 /*
@@ -1677,24 +1677,22 @@ icmp6_redirect_output(struct mbuf *m0, struct rtentry *rt)
 
 	{
 		/* target lladdr option */
-		struct rtentry *rt_nexthop = NULL;
+		struct rtentry *nrt;
 		int len;
 		struct sockaddr_dl *sdl;
 		struct nd_opt_hdr *nd_opt;
 		char *lladdr;
 
-		rt_nexthop = nd6_lookup(nexthop, 0, ifp, ifp->if_rdomain);
-		if (!rt_nexthop)
-			goto nolladdropt;
 		len = sizeof(*nd_opt) + ifp->if_addrlen;
 		len = (len + 7) & ~7;	/* round by 8 */
 		/* safety check */
 		if (len + (p - (u_char *)ip6) > maxlen)
 			goto nolladdropt;
-		if (!(rt_nexthop->rt_flags & RTF_GATEWAY) &&
-		    (rt_nexthop->rt_flags & RTF_LLINFO) &&
-		    (rt_nexthop->rt_gateway->sa_family == AF_LINK) &&
-		    (sdl = (struct sockaddr_dl *)rt_nexthop->rt_gateway) &&
+		nrt = nd6_lookup(nexthop, 0, ifp, ifp->if_rdomain);
+		if ((nrt != NULL) &&
+		    (nrt->rt_flags & (RTF_GATEWAY|RTF_LLINFO)) == RTF_LLINFO &&
+		    (nrt->rt_gateway->sa_family == AF_LINK) &&
+		    (sdl = (struct sockaddr_dl *)nrt->rt_gateway) &&
 		    sdl->sdl_alen) {
 			nd_opt = (struct nd_opt_hdr *)p;
 			nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
@@ -1703,6 +1701,7 @@ icmp6_redirect_output(struct mbuf *m0, struct rtentry *rt)
 			bcopy(LLADDR(sdl), lladdr, ifp->if_addrlen);
 			p += len;
 		}
+		rtfree(nrt);
 	}
   nolladdropt:;
 
diff --git a/sys/netinet6/in6_src.c b/sys/netinet6/in6_src.c
index 9212a19b75c..8ab08e25cfb 100644
--- a/sys/netinet6/in6_src.c
+++ b/sys/netinet6/in6_src.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in6_src.c,v 1.61 2015/09/11 20:16:03 claudio Exp $	*/
+/*	$OpenBSD: in6_src.c,v 1.62 2015/09/18 14:26:22 mpi Exp $	*/
 /*	$KAME: in6_src.c,v 1.36 2001/02/06 04:08:17 itojun Exp $	*/
 
 /*
@@ -232,11 +232,12 @@ in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
 			sin6_next = satosin6(opts->ip6po_nexthop);
 			rt = nd6_lookup(&sin6_next->sin6_addr, 1, NULL,
 			    rtableid);
-			if (rt) {
+			if (rt != NULL) {
 				ia6 = in6_ifawithscope(rt->rt_ifp, dst,
 				    rtableid);
 				if (ia6 == NULL)
 					ia6 = ifatoia6(rt->rt_ifa);
+				rtfree(rt);
 			}
 			if (ia6 == NULL)
 				return (EADDRNOTAVAIL);
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index d735e511f4f..db8e896965d 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6.c,v 1.153 2015/09/13 17:53:44 mpi Exp $	*/
+/*	$OpenBSD: nd6.c,v 1.154 2015/09/18 14:26:22 mpi Exp $	*/
 /*	$KAME: nd6.c,v 1.280 2002/06/08 19:52:07 itojun Exp $	*/
 
 /*
@@ -631,7 +631,7 @@ nd6_lookup(struct in6_addr *addr6, int create, struct ifnet *ifp,
 	flags = (create) ? (RT_REPORT|RT_RESOLVE) : 0;
 
 	rt = rtalloc(sin6tosa(&sin6), flags, rtableid);
-	if (rt && (rt->rt_flags & RTF_LLINFO) == 0) {
+	if (rt != NULL && (rt->rt_flags & RTF_LLINFO) == 0) {
 		/*
 		 * This is the case for the default route.
 		 * If we want to create a neighbor cache for the address, we
@@ -643,7 +643,7 @@ nd6_lookup(struct in6_addr *addr6, int create, struct ifnet *ifp,
 			rt = NULL;
 		}
 	}
-	if (!rt) {
+	if (rt == NULL) {
 		if (create && ifp) {
 			struct rt_addrinfo info;
 			int error;
@@ -683,7 +683,6 @@ nd6_lookup(struct in6_addr *addr6, int create, struct ifnet *ifp,
 		} else
 			return (NULL);
 	}
-	rt->rt_refcnt--;
 	/*
 	 * Validation for the entry.
 	 * Note that the check for rt_llinfo is necessary because a cloned
@@ -706,6 +705,7 @@ nd6_lookup(struct in6_addr *addr6, int create, struct ifnet *ifp,
 			    inet_ntop(AF_INET6, addr6, addr, sizeof(addr)),
 			    ifp ? ifp->if_xname : "unspec"));
 		}
+		rtfree(rt);
 		return (NULL);
 	}
 	return (rt);
@@ -769,9 +769,11 @@ nd6_is_addr_neighbor(struct sockaddr_in6 *addr, struct ifnet *ifp)
 	 * Even if the address matches none of our addresses, it might be
 	 * in the neighbor cache.
 	 */
-	if ((rt = nd6_lookup(&addr->sin6_addr, 0, ifp,
-	    ifp->if_rdomain)) != NULL)
+	rt = nd6_lookup(&addr->sin6_addr, 0, ifp, ifp->if_rdomain);
+	if (rt != NULL) {
+		rtfree(rt);
 		return (1);
+	}
 
 	return (0);
 }
@@ -1274,9 +1276,11 @@ nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
 		}
 
 		s = splsoftnet();
-		if ((rt = nd6_lookup(&nb_addr, 0, ifp, ifp->if_rdomain)) == NULL ||
+		rt = nd6_lookup(&nb_addr, 0, ifp, ifp->if_rdomain);
+		if (rt == NULL ||
 		    (ln = (struct llinfo_nd6 *)rt->rt_llinfo) == NULL) {
 			error = EINVAL;
+			rtfree(rt);
 			splx(s);
 			break;
 		}
@@ -1284,6 +1288,7 @@ nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
 		nbi->asked = ln->ln_asked;
 		nbi->isrouter = ln->ln_router;
 		nbi->expire = ln->ln_expire;
+		rtfree(rt);
 		splx(s);
 
 		break;
@@ -1332,7 +1337,7 @@ nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr *from, char *lladdr,
 	 */
 
 	rt = nd6_lookup(from, 0, ifp, ifp->if_rdomain);
-	if (!rt) {
+	if (rt == NULL) {
 #if 0
 		/* nothing must be done if there's no lladdr */
 		if (!lladdr || !lladdrlen)
@@ -1343,8 +1348,10 @@ nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr *from, char *lladdr,
 		is_newentry = 1;
 	} else {
 		/* do nothing if static ndp is set */
-		if (rt->rt_flags & RTF_STATIC)
+		if (rt->rt_flags & RTF_STATIC) {
+			rtfree(rt);
 			return;
+		}
 		is_newentry = 0;
 	}
 
@@ -1353,6 +1360,7 @@ nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr *from, char *lladdr,
 	if ((rt->rt_flags & (RTF_GATEWAY | RTF_LLINFO)) != RTF_LLINFO) {
 fail:
 		(void)nd6_free(rt, 0);
+		rtfree(rt);
 		return;
 	}
 	ln = (struct llinfo_nd6 *)rt->rt_llinfo;
@@ -1528,6 +1536,8 @@ fail:
 	 */
 	if (do_update && ln->ln_router && (ifp->if_xflags & IFXF_AUTOCONF6))
 		defrouter_select();
+
+	rtfree(rt);
 }
 
 void
@@ -1556,7 +1566,6 @@ nd6_slowtimo(void *ignored_arg)
 	splx(s);
 }
 
-#define senderr(e) { error = (e); goto bad;}
 int
 nd6_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr_in6 *dst,
     struct rtentry *rt0)
@@ -1564,7 +1573,7 @@ nd6_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr_in6 *dst,
 	struct mbuf *m = m0;
 	struct rtentry *rt = rt0;
 	struct llinfo_nd6 *ln = NULL;
-	int error = 0;
+	int created = 0, error = 0;
 
 	if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr))
 		goto sendpkt;
@@ -1613,10 +1622,14 @@ nd6_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr_in6 *dst,
 		 * the condition below is not very efficient.  But we believe
 		 * it is tolerable, because this should be a rare case.
 		 */
-		if (nd6_is_addr_neighbor(dst, ifp) &&
-		    (rt = nd6_lookup(&dst->sin6_addr, 1, ifp,
-		     ifp->if_rdomain)) != NULL)
-			ln = (struct llinfo_nd6 *)rt->rt_llinfo;
+		if (nd6_is_addr_neighbor(dst, ifp)) {
+			rt = nd6_lookup(&dst->sin6_addr, 1, ifp,
+			    ifp->if_rdomain);
+			if (rt != NULL) {
+				created = 1;
+				ln = (struct llinfo_nd6 *)rt->rt_llinfo;
+			}
+		}
 	}
 	if (!ln || !rt) {
 		if ((ifp->if_flags & IFF_POINTOPOINT) == 0 &&
@@ -1628,7 +1641,10 @@ nd6_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr_in6 *dst,
 			    inet_ntop(AF_INET6, &dst->sin6_addr,
 				addr, sizeof(addr)),
 			    ln, rt);
-			senderr(EIO);	/* XXX: good error? */
+			m_freem(m);
+			if (created)
+				rtfree(rt);
+			return (EIO);	/* XXX: good error? */
 		}
 
 		goto sendpkt;	/* send anyway */
@@ -1689,16 +1705,16 @@ nd6_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr_in6 *dst,
 		    (long)ND_IFINFO(ifp)->retrans * hz / 1000);
 		nd6_ns_output(ifp, NULL, &dst->sin6_addr, ln, 0);
 	}
+	if (created)
+		rtfree(rt);
 	return (0);
 
   sendpkt:
-	return (ifp->if_output(ifp, m, sin6tosa(dst), rt));
-
-  bad:
-	m_freem(m);
+	error = ifp->if_output(ifp, m, sin6tosa(dst), rt);
+	if (created)
+		rtfree(rt);
 	return (error);
 }
-#undef senderr
 
 int
 nd6_need_cache(struct ifnet *ifp)
diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
index 460dd322f6b..c578e844b8f 100644
--- a/sys/netinet6/nd6_nbr.c
+++ b/sys/netinet6/nd6_nbr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_nbr.c,v 1.95 2015/09/11 20:13:22 claudio Exp $	*/
+/*	$OpenBSD: nd6_nbr.c,v 1.96 2015/09/18 14:26:22 mpi Exp $	*/
 /*	$KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $	*/
 
 /*
@@ -580,7 +580,7 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
 	int lladdrlen = 0;
 	struct ifaddr *ifa;
 	struct llinfo_nd6 *ln;
-	struct rtentry *rt;
+	struct rtentry *rt = NULL;
 	struct sockaddr_dl *sdl;
 	union nd_opts ndopts;
 	char addr[INET6_ADDRSTRLEN], addr0[INET6_ADDRSTRLEN];
@@ -888,6 +888,7 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
 	}
 
  freeit:
+ 	rtfree(rt);
 	m_freem(m);
 	if_put(ifp);
 	return;
diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index e95970d3ea1..4bc7686c840 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_rtr.c,v 1.124 2015/09/12 20:50:17 mpi Exp $	*/
+/*	$OpenBSD: nd6_rtr.c,v 1.125 2015/09/18 14:26:22 mpi Exp $	*/
 /*	$KAME: nd6_rtr.c,v 1.97 2001/02/07 11:09:13 itojun Exp $	*/
 
 /*
@@ -804,12 +804,15 @@ defrouter_select(void)
 	TAILQ_FOREACH(dr, &nd_defrouter, dr_entry) {
 		if (!(dr->ifp->if_xflags & IFXF_AUTOCONF6))
 			continue;
-		if (!selected_dr &&
-		    (rt = nd6_lookup(&dr->rtaddr, 0, dr->ifp,
-		     dr->ifp->if_rdomain)) &&
-		    (ln = (struct llinfo_nd6 *)rt->rt_llinfo) &&
-		    ND6_IS_LLINFO_PROBREACH(ln)) {
-			selected_dr = dr;
+		if (!selected_dr) {
+			rt = nd6_lookup(&dr->rtaddr, 0, dr->ifp,
+			    dr->ifp->if_rdomain);
+			if ((rt != NULL) &&
+			    (ln = (struct llinfo_nd6 *)rt->rt_llinfo) &&
+			    ND6_IS_LLINFO_PROBREACH(ln)) {
+				selected_dr = dr;
+			}
+			rtfree(rt);
 		}
 
 		if (dr->installed && !installed_dr)
@@ -833,13 +836,15 @@ defrouter_select(void)
 			selected_dr = TAILQ_FIRST(&nd_defrouter);
 		else
 			selected_dr = TAILQ_NEXT(installed_dr, dr_entry);
-	} else if (installed_dr &&
-	    (rt = nd6_lookup(&installed_dr->rtaddr, 0, installed_dr->ifp,
-	     installed_dr->ifp->if_rdomain)) &&
-	    (ln = (struct llinfo_nd6 *)rt->rt_llinfo) &&
-	    ND6_IS_LLINFO_PROBREACH(ln) &&
-	    rtpref(selected_dr) <= rtpref(installed_dr)) {
-		selected_dr = installed_dr;
+	} else if (installed_dr) {
+		rt = nd6_lookup(&installed_dr->rtaddr, 0, installed_dr->ifp,
+		    installed_dr->ifp->if_rdomain);
+		if ((rt != NULL) && (ln = (struct llinfo_nd6 *)rt->rt_llinfo) &&
+		    ND6_IS_LLINFO_PROBREACH(ln) &&
+		    rtpref(selected_dr) <= rtpref(installed_dr)) {
+			selected_dr = installed_dr;
+		}
+		rtfree(rt);
 	}
 
 	/*
@@ -1536,15 +1541,18 @@ struct nd_pfxrouter *
 find_pfxlist_reachable_router(struct nd_prefix *pr)
 {
 	struct nd_pfxrouter *pfxrtr;
-	struct rtentry *rt;
+	struct rtentry *rt = NULL;
 	struct llinfo_nd6 *ln;
 
 	LIST_FOREACH(pfxrtr, &pr->ndpr_advrtrs, pfr_entry) {
 		if ((rt = nd6_lookup(&pfxrtr->router->rtaddr, 0,
 		    pfxrtr->router->ifp, pfxrtr->router->ifp->if_rdomain)) &&
 		    (ln = (struct llinfo_nd6 *)rt->rt_llinfo) &&
-		    ND6_IS_LLINFO_PROBREACH(ln))
+		    ND6_IS_LLINFO_PROBREACH(ln)) {
+			rtfree(rt);
 			break;	/* found */
+		}
+		rtfree(rt);
 	}
 
 	return (pfxrtr);