This diff removes a tricky "rt->rt_refcnt--". It changes the behavior
of nd6_lookup() to always return a route entry with a valid reference
count. That means that rtfree(9) should be called after nd6_lookup().
There's on hairy case in nd6_output(). This is because of the dance
done in rt_checkgate(). This will be simplified with my rtalloc(9)
rewrite.
I'd like to have careful reviews. I'm running with this for a couple
of months but I doubt I'm exercising all the code paths.
Index: netinet6/icmp6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.171
diff -u -p -r1.171 icmp6.c
--- netinet6/icmp6.c 11 Sep 2015 22:00:36 -0000 1.171
+++ netinet6/icmp6.c 12 Sep 2015 09:08:32 -0000
@@ -1677,24 +1677,21 @@ icmp6_redirect_output(struct mbuf *m0, s
{
/* target lladdr option */
- struct rtentry *rt_nexthop = NULL;
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) &&
+ rt = nd6_lookup(nexthop, 0, ifp, ifp->if_rdomain);
+ if ((rt != NULL) &&
+ (rt->rt_flags & (RTF_GATEWAY|RTF_LLINFO)) == RTF_LLINFO &&
+ (rt->rt_gateway->sa_family == AF_LINK) &&
+ (sdl = (struct sockaddr_dl *)rt->rt_gateway) &&
sdl->sdl_alen) {
nd_opt = (struct nd_opt_hdr *)p;
nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
@@ -1703,6 +1700,7 @@ icmp6_redirect_output(struct mbuf *m0, s
bcopy(LLADDR(sdl), lladdr, ifp->if_addrlen);
p += len;
}
+ rtfree(rt);
}
nolladdropt:;
Index: netinet6/in6_src.c
===================================================================
RCS file: /cvs/src/sys/netinet6/in6_src.c,v
retrieving revision 1.61
diff -u -p -r1.61 in6_src.c
--- netinet6/in6_src.c 11 Sep 2015 20:16:03 -0000 1.61
+++ netinet6/in6_src.c 12 Sep 2015 09:08:32 -0000
@@ -232,11 +232,12 @@ in6_selectsrc(struct in6_addr **in6src,
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);
Index: netinet6/nd6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.150
diff -u -p -r1.150 nd6.c
--- netinet6/nd6.c 10 Sep 2015 17:52:05 -0000 1.150
+++ netinet6/nd6.c 12 Sep 2015 09:10:14 -0000
@@ -631,7 +631,7 @@ nd6_lookup(struct in6_addr *addr6, int c
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 c
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 c
} 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 c
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
* 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);
}
@@ -1281,9 +1283,11 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
}
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;
}
@@ -1291,6 +1295,7 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
nbi->asked = ln->ln_asked;
nbi->isrouter = ln->ln_router;
nbi->expire = ln->ln_expire;
+ rtfree(rt);
splx(s);
break;
@@ -1339,7 +1344,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru
*/
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)
@@ -1351,6 +1356,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru
} else {
/* do nothing if static ndp is set */
if (rt->rt_flags & RTF_STATIC)
+ rtfree(rt);
return;
is_newentry = 0;
}
@@ -1360,6 +1366,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru
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;
@@ -1535,6 +1542,8 @@ fail:
*/
if (do_update && ln->ln_router && (ifp->if_xflags & IFXF_AUTOCONF6))
defrouter_select();
+
+ rtfree(rt);
}
void
@@ -1563,7 +1572,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)
@@ -1571,7 +1579,7 @@ nd6_output(struct ifnet *ifp, struct mbu
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;
@@ -1620,10 +1628,14 @@ nd6_output(struct ifnet *ifp, struct mbu
* 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 &&
@@ -1635,7 +1647,10 @@ nd6_output(struct ifnet *ifp, struct mbu
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 */
@@ -1699,13 +1714,11 @@ nd6_output(struct ifnet *ifp, struct mbu
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)
Index: netinet6/nd6_nbr.c
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.95
diff -u -p -r1.95 nd6_nbr.c
--- netinet6/nd6_nbr.c 11 Sep 2015 20:13:22 -0000 1.95
+++ netinet6/nd6_nbr.c 12 Sep 2015 09:08:37 -0000
@@ -888,6 +888,7 @@ nd6_na_input(struct mbuf *m, int off, in
}
freeit:
+ rtfree(rt);
m_freem(m);
if_put(ifp);
return;
Index: netinet6/nd6_rtr.c
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.123
diff -u -p -r1.123 nd6_rtr.c
--- netinet6/nd6_rtr.c 11 Sep 2015 08:17:06 -0000 1.123
+++ netinet6/nd6_rtr.c 12 Sep 2015 09:10:15 -0000
@@ -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,7 +1541,7 @@ 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) {
@@ -1545,7 +1550,9 @@ find_pfxlist_reachable_router(struct nd_
(ln = (struct llinfo_nd6 *)rt->rt_llinfo) &&
ND6_IS_LLINFO_PROBREACH(ln))
break; /* found */
+ rtfree(rt);
}
+ rtfree(rt);
return (pfxrtr);
}
