Module Name: src Committed By: ozaki-r Date: Fri Jul 17 02:21:08 UTC 2015
Modified Files: src/sys/net: if.c route.c route.h rtsock.c src/sys/netinet: if_atm.c ip_output.c src/sys/netinet6: icmp6.c nd6.c nd6.h nd6_nbr.c nd6_rtr.c Log Message: Reform use of rt_refcnt rt_refcnt of rtentry was used in bad manners, for example, direct rt_refcnt++ and rt_refcnt-- outside route.c, "rt->rt_refcnt++; rtfree(rt);" idiom, and touching rt after rt->rt_refcnt--. These abuses seem to be needed because rt_refcnt manages only references between rtentry and doesn't take care of references during packet processing (IOW references from local variables). In order to reduce the above abuses, the latter cases should be counted by rt_refcnt as well as the former cases. This change improves consistency of use of rt_refcnt: - rtentry is always accessed with rt_refcnt incremented - rtentry's rt_refcnt is decremented after use (rtfree is always used instead of rt_refcnt--) - functions returning rtentry increment its rt_refcnt (and caller rtfree it) Note that rt_refcnt prevents rtentry from being freed but doesn't prevent rtentry from being updated. Toward MP-safe, we need to provide another protection for rtentry, e.g., locks. (Or introduce a better data structure allowing concurrent readers during updates.) To generate a diff of this commit: cvs rdiff -u -r1.316 -r1.317 src/sys/net/if.c cvs rdiff -u -r1.145 -r1.146 src/sys/net/route.c cvs rdiff -u -r1.91 -r1.92 src/sys/net/route.h cvs rdiff -u -r1.171 -r1.172 src/sys/net/rtsock.c cvs rdiff -u -r1.34 -r1.35 src/sys/netinet/if_atm.c cvs rdiff -u -r1.243 -r1.244 src/sys/netinet/ip_output.c cvs rdiff -u -r1.170 -r1.171 src/sys/netinet6/icmp6.c cvs rdiff -u -r1.164 -r1.165 src/sys/netinet6/nd6.c cvs rdiff -u -r1.65 -r1.66 src/sys/netinet6/nd6.h cvs rdiff -u -r1.108 -r1.109 src/sys/netinet6/nd6_nbr.c cvs rdiff -u -r1.100 -r1.101 src/sys/netinet6/nd6_rtr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/net/if.c diff -u src/sys/net/if.c:1.316 src/sys/net/if.c:1.317 --- src/sys/net/if.c:1.316 Mon Jun 29 09:40:36 2015 +++ src/sys/net/if.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: if.c,v 1.316 2015/06/29 09:40:36 ozaki-r Exp $ */ +/* $NetBSD: if.c,v 1.317 2015/07/17 02:21:08 ozaki-r Exp $ */ /*- * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc. @@ -90,7 +90,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.316 2015/06/29 09:40:36 ozaki-r Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.317 2015/07/17 02:21:08 ozaki-r Exp $"); #if defined(_KERNEL_OPT) #include "opt_inet.h" @@ -963,20 +963,23 @@ if_rt_walktree(struct rtentry *rt, void { struct ifnet *ifp = (struct ifnet *)v; int error; + struct rtentry *retrt; if (rt->rt_ifp != ifp) return 0; /* Delete the entry. */ - ++rt->rt_refcnt; error = rtrequest(RTM_DELETE, rt_getkey(rt), rt->rt_gateway, - rt_mask(rt), rt->rt_flags, NULL); - KASSERT((rt->rt_flags & RTF_UP) == 0); - rt->rt_ifp = NULL; - rtfree(rt); - if (error != 0) + rt_mask(rt), rt->rt_flags, &retrt); + if (error == 0) { + KASSERT(retrt == rt); + KASSERT((retrt->rt_flags & RTF_UP) == 0); + retrt->rt_ifp = NULL; + rtfree(retrt); + } else { printf("%s: warning: unable to delete rtentry @ %p, " "error = %d\n", ifp->if_xname, rt, error); + } return ERESTART; } Index: src/sys/net/route.c diff -u src/sys/net/route.c:1.145 src/sys/net/route.c:1.146 --- src/sys/net/route.c:1.145 Mon Jun 8 08:21:49 2015 +++ src/sys/net/route.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: route.c,v 1.145 2015/06/08 08:21:49 roy Exp $ */ +/* $NetBSD: route.c,v 1.146 2015/07/17 02:21:08 ozaki-r Exp $ */ /*- * Copyright (c) 1998, 2008 The NetBSD Foundation, Inc. @@ -94,7 +94,7 @@ #include "opt_route.h" #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.145 2015/06/08 08:21:49 roy Exp $"); +__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.146 2015/07/17 02:21:08 ozaki-r Exp $"); #include <sys/param.h> #ifdef RTFLUSH_DEBUG @@ -355,7 +355,8 @@ rtcache(struct route *ro) } /* - * Packet routing routines. + * Packet routing routines. If success, refcnt of a returned rtentry + * will be incremented. The caller has to rtfree it by itself. */ struct rtentry * rtalloc1(const struct sockaddr *dst, int report) @@ -378,6 +379,7 @@ rtalloc1(const struct sockaddr *dst, int } KASSERT(newrt != NULL); rt = newrt; + rt->rt_refcnt++; if (rt->rt_flags & RTF_XRESOLVE) { msgtype = RTM_RESOLVE; goto miss; @@ -533,13 +535,15 @@ out: } /* - * Delete a route and generate a message + * Delete a route and generate a message. + * It doesn't free a passed rt. */ static int rtdeletemsg(struct rtentry *rt) { int error; struct rt_addrinfo info; + struct rtentry *retrt; /* * Request the new route so that the entry is not actually @@ -551,15 +555,12 @@ rtdeletemsg(struct rtentry *rt) info.rti_info[RTAX_NETMASK] = rt_mask(rt); info.rti_info[RTAX_GATEWAY] = rt->rt_gateway; info.rti_flags = rt->rt_flags; - error = rtrequest1(RTM_DELETE, &info, &rt); + error = rtrequest1(RTM_DELETE, &info, &retrt); rt_missmsg(RTM_DELETE, &info, info.rti_flags, error); - /* Adjust the refcount */ - if (error == 0 && rt->rt_refcnt <= 0) { - rt->rt_refcnt++; - rtfree(rt); - } + if (error == 0) + rtfree(retrt); return error; } @@ -617,8 +618,9 @@ ifa_ifwithroute(int flags, const struct struct rtentry *rt = rtalloc1(dst, 0); if (rt == NULL) return NULL; - rt->rt_refcnt--; - if ((ifa = rt->rt_ifa) == NULL) + ifa = rt->rt_ifa; + rtfree(rt); + if (ifa == NULL) return NULL; } if (ifa->ifa_addr->sa_family != dst->sa_family) { @@ -630,6 +632,10 @@ ifa_ifwithroute(int flags, const struct return ifa; } +/* + * If it suceeds and ret_nrt isn't NULL, refcnt of ret_nrt is incremented. + * The caller has to rtfree it by itself. + */ int rtrequest(int req, const struct sockaddr *dst, const struct sockaddr *gateway, const struct sockaddr *netmask, int flags, struct rtentry **ret_nrt) @@ -644,6 +650,32 @@ rtrequest(int req, const struct sockaddr return rtrequest1(req, &info, ret_nrt); } +/* + * It's a utility function to add/remove a route to/from the routing table + * and tell user processes the addition/removal on success. + */ +int +rtrequest_newmsg(const int req, const struct sockaddr *dst, + const struct sockaddr *gateway, const struct sockaddr *netmask, + const int flags) +{ + int error; + struct rtentry *ret_nrt = NULL; + + KASSERT(req == RTM_ADD || req == RTM_DELETE); + + error = rtrequest(req, dst, gateway, netmask, flags, &ret_nrt); + if (error != 0) + return error; + + KASSERT(ret_nrt != NULL); + + rt_newmsg(req, ret_nrt); /* tell user process */ + rtfree(ret_nrt); + + return 0; +} + int rt_getifa(struct rt_addrinfo *info) { @@ -688,6 +720,10 @@ rt_getifa(struct rt_addrinfo *info) return 0; } +/* + * If it suceeds and ret_nrt isn't NULL, refcnt of ret_nrt is incremented. + * The caller has to rtfree it by itself. + */ int rtrequest1(int req, struct rt_addrinfo *info, struct rtentry **ret_nrt) { @@ -743,9 +779,11 @@ rtrequest1(int req, struct rt_addrinfo * ifa->ifa_rtrequest(RTM_DELETE, rt, info); } rttrash++; - if (ret_nrt) + if (ret_nrt) { *ret_nrt = rt; - else if (rt->rt_refcnt <= 0) { + rt->rt_refcnt++; + } else if (rt->rt_refcnt <= 0) { + /* Adjust the refcount */ rt->rt_refcnt++; rtfree(rt); } @@ -974,10 +1012,12 @@ rtinit(struct ifaddr *ifa, int cmd, int rt_maskedcopy(odst, dst, ifa->ifa_netmask); } if ((rt = rtalloc1(dst, 0)) != NULL) { - rt->rt_refcnt--; - if (rt->rt_ifa != ifa) + if (rt->rt_ifa != ifa) { + rtfree(rt); return (flags & RTF_HOST) ? EHOSTUNREACH : ENETUNREACH; + } + rtfree(rt); } } memset(&info, 0, sizeof(info)); @@ -996,17 +1036,14 @@ rtinit(struct ifaddr *ifa, int cmd, int error = rtrequest1((cmd == RTM_LLINFO_UPD) ? RTM_GET : cmd, &info, &nrt); if (error != 0 || (rt = nrt) == NULL) - ; - else switch (cmd) { + return error; + + switch (cmd) { case RTM_DELETE: - rt_newmsg(cmd, nrt); - if (rt->rt_refcnt <= 0) { - rt->rt_refcnt++; - rtfree(rt); - } + rt_newmsg(cmd, rt); + rtfree(rt); break; case RTM_LLINFO_UPD: - rt->rt_refcnt--; RT_DPRINTF("%s: updating%s\n", __func__, ((rt->rt_flags & RTF_LLINFO) == 0) ? " (no llinfo)" : ""); @@ -1023,10 +1060,10 @@ rtinit(struct ifaddr *ifa, int cmd, int if (cmd == RTM_LLINFO_UPD && ifa->ifa_rtrequest != NULL) ifa->ifa_rtrequest(RTM_LLINFO_UPD, rt, &info); - rt_newmsg(RTM_CHANGE, nrt); + rt_newmsg(RTM_CHANGE, rt); + rtfree(rt); break; case RTM_ADD: - rt->rt_refcnt--; if (rt->rt_ifa != ifa) { printf("rtinit: wrong ifa (%p) was (%p)\n", ifa, rt->rt_ifa); @@ -1039,7 +1076,8 @@ rtinit(struct ifaddr *ifa, int cmd, int if (ifa->ifa_rtrequest != NULL) ifa->ifa_rtrequest(RTM_ADD, rt, &info); } - rt_newmsg(cmd, nrt); + rt_newmsg(cmd, rt); + rtfree(rt); break; } return error; @@ -1085,18 +1123,9 @@ rt_ifa_localrequest(int cmd, struct ifad rt_replace_ifa(nrt, ifa); rt_newaddrmsg(cmd, ifa, e, nrt); - if (nrt) { - if (cmd == RTM_DELETE) { - if (nrt->rt_refcnt <= 0) { - /* XXX: we should free the entry ourselves. */ - nrt->rt_refcnt++; - rtfree(nrt); - } - } else { - /* the cmd must be RTM_ADD here */ - nrt->rt_refcnt--; - } - } + if (nrt != NULL) + rtfree(nrt); + return e; } @@ -1120,7 +1149,7 @@ rt_ifa_addlocal(struct ifaddr *ifa) rt_newaddrmsg(RTM_NEWADDR, ifa, 0, NULL); } if (rt != NULL) - rt->rt_refcnt--; + rtfree(rt); return e; } @@ -1161,7 +1190,7 @@ rt_ifa_remlocal(struct ifaddr *ifa, stru } else rt_newaddrmsg(RTM_DELADDR, ifa, 0, NULL); if (rt != NULL) - rt->rt_refcnt--; + rtfree(rt); return e; } Index: src/sys/net/route.h diff -u src/sys/net/route.h:1.91 src/sys/net/route.h:1.92 --- src/sys/net/route.h:1.91 Thu Apr 30 09:57:38 2015 +++ src/sys/net/route.h Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: route.h,v 1.91 2015/04/30 09:57:38 ozaki-r Exp $ */ +/* $NetBSD: route.h,v 1.92 2015/07/17 02:21:08 ozaki-r Exp $ */ /* * Copyright (c) 1980, 1986, 1993 @@ -387,6 +387,8 @@ int rtrequest(int, const struct sockaddr const struct sockaddr *, const struct sockaddr *, int, struct rtentry **); int rtrequest1(int, struct rt_addrinfo *, struct rtentry **); +int rtrequest_newmsg(const int, const struct sockaddr *, + const struct sockaddr *, const struct sockaddr *, const int); int rt_ifa_addlocal(struct ifaddr *); int rt_ifa_remlocal(struct ifaddr *, struct ifaddr *); @@ -401,6 +403,15 @@ const struct sockaddr * struct sockaddr * rt_gettag(struct rtentry *); +static inline struct rtentry * +rt_get_gwroute(struct rtentry *rt) +{ + if (rt->rt_gwroute == NULL) + return NULL; + rt->rt_gwroute->rt_refcnt++; + return rt->rt_gwroute; +} + void rtcache_copy(struct route *, const struct route *); void rtcache_free(struct route *); struct rtentry * Index: src/sys/net/rtsock.c diff -u src/sys/net/rtsock.c:1.171 src/sys/net/rtsock.c:1.172 --- src/sys/net/rtsock.c:1.171 Sat May 2 17:18:03 2015 +++ src/sys/net/rtsock.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: rtsock.c,v 1.171 2015/05/02 17:18:03 rtr Exp $ */ +/* $NetBSD: rtsock.c,v 1.172 2015/07/17 02:21:08 ozaki-r Exp $ */ /* * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. @@ -61,7 +61,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: rtsock.c,v 1.171 2015/05/02 17:18:03 rtr Exp $"); +__KERNEL_RCSID(0, "$NetBSD: rtsock.c,v 1.172 2015/07/17 02:21:08 ozaki-r Exp $"); #ifdef _KERNEL_OPT #include "opt_inet.h" @@ -494,16 +494,16 @@ COMPATNAME(route_output)(struct mbuf *m, senderr(EINVAL); } error = rtrequest1(rtm->rtm_type, &info, &saved_nrt); - if (error == 0 && saved_nrt) { + if (error == 0) { rt_setmetrics(rtm->rtm_inits, rtm, saved_nrt); - saved_nrt->rt_refcnt--; + rtfree(saved_nrt); } break; case RTM_DELETE: error = rtrequest1(rtm->rtm_type, &info, &saved_nrt); if (error == 0) { - (rt = saved_nrt)->rt_refcnt++; + rt = saved_nrt; goto report; } break; @@ -515,6 +515,7 @@ COMPATNAME(route_output)(struct mbuf *m, * info.rti_info[RTAX_NETMASK] before * searching. It did not used to do that. --dyoung */ + rt = NULL; error = rtrequest1(RTM_GET, &info, &rt); if (error != 0) senderr(error); Index: src/sys/netinet/if_atm.c diff -u src/sys/netinet/if_atm.c:1.34 src/sys/netinet/if_atm.c:1.35 --- src/sys/netinet/if_atm.c:1.34 Mon Nov 10 18:52:51 2014 +++ src/sys/netinet/if_atm.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: if_atm.c,v 1.34 2014/11/10 18:52:51 maxv Exp $ */ +/* $NetBSD: if_atm.c,v 1.35 2015/07/17 02:21:08 ozaki-r Exp $ */ /* * Copyright (c) 1996 Charles D. Cranor and Washington University. @@ -30,7 +30,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: if_atm.c,v 1.34 2014/11/10 18:52:51 maxv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_atm.c,v 1.35 2015/07/17 02:21:08 ozaki-r Exp $"); #include "opt_inet.h" #include "opt_natm.h" @@ -221,10 +221,11 @@ failed: */ int -atmresolve(struct rtentry *rt, struct mbuf *m, const struct sockaddr *dst, +atmresolve(struct rtentry *rt0, struct mbuf *m, const struct sockaddr *dst, struct atm_pseudohdr *desten /* OUT */) { const struct sockaddr_dl *sdl; + struct rtentry *rt = rt0; if (m->m_flags & (M_BCAST|M_MCAST)) { log(LOG_INFO, "atmresolve: BCAST/MCAST packet detected/dumped\n"); @@ -233,13 +234,14 @@ atmresolve(struct rtentry *rt, struct mb if (rt == NULL) { rt = RTALLOC1(dst, 0); - if (rt == NULL) goto bad; /* failed */ - rt->rt_refcnt--; /* don't keep LL references */ + if (rt == NULL) + goto bad; /* failed */ if ((rt->rt_flags & RTF_GATEWAY) != 0 || - (rt->rt_flags & RTF_LLINFO) == 0 || - /* XXX: are we using LLINFO? */ - rt->rt_gateway->sa_family != AF_LINK) { - goto bad; + (rt->rt_flags & RTF_LLINFO) == 0 || + /* XXX: are we using LLINFO? */ + rt->rt_gateway->sa_family != AF_LINK) { + rtfree(rt); + goto bad; } } @@ -257,12 +259,16 @@ atmresolve(struct rtentry *rt, struct mb * is resolved; otherwise, try to resolve. */ - if (sdl->sdl_family == AF_LINK && sdl->sdl_alen == sizeof(*desten)) { memcpy(desten, CLLADDR(sdl), sdl->sdl_alen); + if (rt != rt0) + rtfree(rt); return (1); /* ok, go for it! */ } + if (rt != rt0) + rtfree(rt); + /* * we got an entry, but it doesn't have valid link address * info in it (it is prob. the interface route, which has Index: src/sys/netinet/ip_output.c diff -u src/sys/netinet/ip_output.c:1.243 src/sys/netinet/ip_output.c:1.244 --- src/sys/netinet/ip_output.c:1.243 Tue Jul 14 08:44:59 2015 +++ src/sys/netinet/ip_output.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: ip_output.c,v 1.243 2015/07/14 08:44:59 ozaki-r Exp $ */ +/* $NetBSD: ip_output.c,v 1.244 2015/07/17 02:21:08 ozaki-r Exp $ */ /* * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. @@ -91,7 +91,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ip_output.c,v 1.243 2015/07/14 08:44:59 ozaki-r Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ip_output.c,v 1.244 2015/07/17 02:21:08 ozaki-r Exp $"); #include "opt_inet.h" #include "opt_ipsec.h" @@ -201,12 +201,17 @@ klock_if_output(struct ifnet * const ifp */ int ip_hresolv_output(struct ifnet * const ifp0, struct mbuf * const m, - const struct sockaddr * const dst, struct rtentry *rt0) + const struct sockaddr * const dst, struct rtentry *rt00) { int error = 0; struct ifnet *ifp = ifp0; - struct rtentry *rt; + struct rtentry *rt, *rt0, *gwrt; + +#define RTFREE_IF_NEEDED(_rt) \ + if ((_rt) != NULL && (_rt) != rt00) \ + rtfree((_rt)); + rt0 = rt00; retry: if (!ip_hresolv_needed(ifp)) { rt = rt0; @@ -232,10 +237,8 @@ retry: goto bad; } rt0 = rt; - rt->rt_refcnt--; if (rt->rt_ifp != ifp) { ifp = rt->rt_ifp; - rt0 = rt; goto retry; } } @@ -243,24 +246,27 @@ retry: if ((rt->rt_flags & RTF_GATEWAY) == 0) goto out; - rt = rt->rt_gwroute; + gwrt = rt_get_gwroute(rt); + RTFREE_IF_NEEDED(rt); + rt = gwrt; if (rt == NULL || (rt->rt_flags & RTF_UP) == 0) { if (rt != NULL) { - rtfree(rt); + RTFREE_IF_NEEDED(rt); rt = rt0; } if (rt == NULL) { error = EHOSTUNREACH; goto bad; } - rt = rt->rt_gwroute = rtalloc1(rt->rt_gateway, 1); + gwrt = rt->rt_gwroute = rtalloc1(rt->rt_gateway, 1); + RTFREE_IF_NEEDED(rt); + rt = gwrt; if (rt == NULL) { error = EHOSTUNREACH; goto bad; } /* the "G" test below also prevents rt == rt0 */ if ((rt->rt_flags & RTF_GATEWAY) != 0 || rt->rt_ifp != ifp) { - rt->rt_refcnt--; rt0->rt_gwroute = NULL; error = EHOSTUNREACH; goto bad; @@ -300,12 +306,18 @@ out: } #endif - return klock_if_output(ifp, m, dst, rt); + error = klock_if_output(ifp, m, dst, rt); + goto exit; + bad: if (m != NULL) m_freem(m); +exit: + RTFREE_IF_NEEDED(rt); return error; + +#undef RTFREE_IF_NEEDED } /* Index: src/sys/netinet6/icmp6.c diff -u src/sys/netinet6/icmp6.c:1.170 src/sys/netinet6/icmp6.c:1.171 --- src/sys/netinet6/icmp6.c:1.170 Tue Nov 25 19:51:17 2014 +++ src/sys/netinet6/icmp6.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: icmp6.c,v 1.170 2014/11/25 19:51:17 christos Exp $ */ +/* $NetBSD: icmp6.c,v 1.171 2015/07/17 02:21:08 ozaki-r Exp $ */ /* $KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $ */ /* @@ -62,7 +62,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: icmp6.c,v 1.170 2014/11/25 19:51:17 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: icmp6.c,v 1.171 2015/07/17 02:21:08 ozaki-r Exp $"); #include "opt_inet.h" #include "opt_ipsec.h" @@ -2472,8 +2472,10 @@ icmp6_redirect_output(struct mbuf *m0, s len = sizeof(*nd_opt) + ifp->if_addrlen; len = (len + 7) & ~7; /* round by 8 */ /* safety check */ - if (len + (p - (u_char *)ip6) > maxlen) + if (len + (p - (u_char *)ip6) > maxlen) { + rtfree(rt); goto nolladdropt; + } if (!(rt_nexthop->rt_flags & RTF_GATEWAY) && (rt_nexthop->rt_flags & RTF_LLINFO) && (rt_nexthop->rt_gateway->sa_family == AF_LINK) && @@ -2486,6 +2488,7 @@ icmp6_redirect_output(struct mbuf *m0, s memcpy(lladdr, CLLADDR(sdl), ifp->if_addrlen); p += len; } + rtfree(rt); } nolladdropt:; Index: src/sys/netinet6/nd6.c diff -u src/sys/netinet6/nd6.c:1.164 src/sys/netinet6/nd6.c:1.165 --- src/sys/netinet6/nd6.c:1.164 Wed Jul 15 09:20:18 2015 +++ src/sys/netinet6/nd6.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: nd6.c,v 1.164 2015/07/15 09:20:18 ozaki-r Exp $ */ +/* $NetBSD: nd6.c,v 1.165 2015/07/17 02:21:08 ozaki-r Exp $ */ /* $KAME: nd6.c,v 1.279 2002/06/08 11:16:51 itojun Exp $ */ /* @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.164 2015/07/15 09:20:18 ozaki-r Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.165 2015/07/17 02:21:08 ozaki-r Exp $"); #include "opt_net_mpsafe.h" @@ -913,7 +913,6 @@ nd6_lookup1(const struct in6_addr *addr6 } } else return NULL; - rt->rt_refcnt--; /* * Check for a cloning route to match the address. @@ -980,6 +979,7 @@ int nd6_is_addr_neighbor(const struct sockaddr_in6 *addr, struct ifnet *ifp) { struct nd_prefix *pr; + struct rtentry *rt; /* * A link-local address is always a neighbor. @@ -1035,8 +1035,11 @@ nd6_is_addr_neighbor(const struct sockad * Even if the address matches none of our addresses, it might match * a cloning route or be in the neighbor cache. */ - if (nd6_lookup1(&addr->sin6_addr, 0, ifp, 1) != NULL) + rt = nd6_lookup1(&addr->sin6_addr, 0, ifp, 1); + if (rt != NULL) { + rtfree(rt); return 1; + } return 0; } @@ -1053,7 +1056,7 @@ nd6_free(struct rtentry *rt, int gc) struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo, *next; struct in6_addr in6 = satocsin6(rt_getkey(rt))->sin6_addr; struct nd_defrouter *dr; - struct rtentry *oldrt; + int error; /* * we used to have pfctlinput(PRC_HOSTDEAD) here. @@ -1146,14 +1149,10 @@ nd6_free(struct rtentry *rt, int gc) * caches, and disable the route entry not to be used in already * cached routes. */ - oldrt = NULL; - rtrequest(RTM_DELETE, rt_getkey(rt), NULL, rt_mask(rt), 0, &oldrt); - if (oldrt) { - rt_newmsg(RTM_DELETE, oldrt); /* tell user process */ - if (oldrt->rt_refcnt <= 0) { - oldrt->rt_refcnt++; - rtfree(oldrt); - } + error = rtrequest_newmsg(RTM_DELETE, rt_getkey(rt), NULL, + rt_mask(rt), 0); + if (error != 0) { + /* XXX need error message? */; } return next; @@ -1165,9 +1164,10 @@ nd6_free(struct rtentry *rt, int gc) * XXX cost-effective methods? */ void -nd6_nud_hint(struct rtentry *rt) +nd6_nud_hint(struct rtentry *rt0) { struct llinfo_nd6 *ln; + struct rtentry *rt = rt0; if (rt == NULL) return; @@ -1177,12 +1177,12 @@ nd6_nud_hint(struct rtentry *rt) !rt->rt_llinfo || !rt->rt_gateway || rt->rt_gateway->sa_family != AF_LINK) { /* This is not a host route. */ - return; + goto exit; } ln = (struct llinfo_nd6 *)rt->rt_llinfo; if (ln->ln_state < ND6_LLINFO_REACHABLE) - return; + goto exit; /* * if we get upper-layer reachability confirmation many times, @@ -1190,13 +1190,17 @@ nd6_nud_hint(struct rtentry *rt) */ ln->ln_byhint++; if (ln->ln_byhint > nd6_maxnudhint) - return; + goto exit; ln->ln_state = ND6_LLINFO_REACHABLE; if (!ND6_LLINFO_PERMANENT(ln)) { nd6_llinfo_settimer(ln, (long)ND_IFINFO(rt->rt_ifp)->reachable * hz); } +exit: + if (rt != rt0) + rtfree(rt); + return; } void @@ -1818,8 +1822,16 @@ nd6_ioctl(u_long cmd, void *data, struct return error; s = splsoftnet(); - if ((rt = nd6_lookup(&nb_addr, 0, ifp)) == NULL || - (ln = (struct llinfo_nd6 *)rt->rt_llinfo) == NULL) { + rt = nd6_lookup(&nb_addr, 0, ifp); + if (rt == NULL) { + error = EINVAL; + splx(s); + break; + } + + ln = (struct llinfo_nd6 *)rt->rt_llinfo; + rtfree(rt); + if (ln == NULL) { error = EINVAL; splx(s); break; @@ -1866,7 +1878,7 @@ nd6_llinfo_release_pkts(struct llinfo_nd * Create neighbor cache entry and cache link-layer address, * on reception of inbound ND6 packets. (RS/RA/NS/redirect) */ -struct rtentry * +void nd6_cache_lladdr( struct ifnet *ifp, struct in6_addr *from, @@ -1891,7 +1903,7 @@ nd6_cache_lladdr( /* nothing must be updated for unspecified address */ if (IN6_IS_ADDR_UNSPECIFIED(from)) - return NULL; + return; /* * Validation about ifp->if_addrlen and lladdrlen must be done in @@ -1915,17 +1927,20 @@ nd6_cache_lladdr( is_newentry = 1; } else { /* do nothing if static ndp is set */ - if (rt->rt_flags & RTF_STATIC) - return NULL; + if (rt->rt_flags & RTF_STATIC) { + rtfree(rt); + return; + } is_newentry = 0; } if (rt == NULL) - return NULL; + return; if ((rt->rt_flags & (RTF_GATEWAY | RTF_LLINFO)) != RTF_LLINFO) { fail: (void)nd6_free(rt, 0); - return NULL; + rtfree(rt); + return; } ln = (struct llinfo_nd6 *)rt->rt_llinfo; if (ln == NULL) @@ -2094,7 +2109,7 @@ fail: nd6_accepts_rtadv(ndi)) defrouter_select(); - return rt; + rtfree(rt); } static void @@ -2128,14 +2143,20 @@ nd6_slowtimo(void *ignored_arg) #define senderr(e) { error = (e); goto bad;} int nd6_output(struct ifnet *ifp, struct ifnet *origifp, struct mbuf *m0, - const struct sockaddr_in6 *dst, struct rtentry *rt0) + const struct sockaddr_in6 *dst, struct rtentry *rt00) { struct mbuf *m = m0; - struct rtentry *rt = rt0; + struct rtentry *rt, *rt0; struct sockaddr_in6 *gw6 = NULL; struct llinfo_nd6 *ln = NULL; int error = 0; +#define RTFREE_IF_NEEDED(_rt) \ + if ((_rt) != NULL && (_rt) != rt00) \ + rtfree((_rt)); + + rt = rt0 = rt00; + if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr)) goto sendpkt; @@ -2148,7 +2169,6 @@ nd6_output(struct ifnet *ifp, struct ifn if (rt) { if ((rt->rt_flags & RTF_UP) == 0) { if ((rt0 = rt = rtalloc1(sin6tocsa(dst), 1)) != NULL) { - rt->rt_refcnt--; if (rt->rt_ifp != ifp) senderr(EHOSTUNREACH); } else @@ -2156,6 +2176,7 @@ nd6_output(struct ifnet *ifp, struct ifn } if (rt->rt_flags & RTF_GATEWAY) { + struct rtentry *gwrt; gw6 = (struct sockaddr_in6 *)rt->rt_gateway; /* @@ -2179,18 +2200,25 @@ nd6_output(struct ifnet *ifp, struct ifn goto sendpkt; } - if (rt->rt_gwroute == NULL) + gwrt = rt_get_gwroute(rt); + if (gwrt == NULL) goto lookup; - if (((rt = rt->rt_gwroute)->rt_flags & RTF_UP) == 0) { - rtfree(rt); rt = rt0; + + RTFREE_IF_NEEDED(rt); + rt = gwrt; + if ((rt->rt_flags & RTF_UP) == 0) { + rtfree(rt); + rt = rt0; lookup: - rt->rt_gwroute = rtalloc1(rt->rt_gateway, 1); - if ((rt = rt->rt_gwroute) == NULL) + gwrt = rt->rt_gwroute = + rtalloc1(rt->rt_gateway, 1); + rtfree(rt); + rt = gwrt; + if (rt == NULL) senderr(EHOSTUNREACH); /* the "G" test below also prevents rt == rt0 */ if ((rt->rt_flags & RTF_GATEWAY) || (rt->rt_ifp != ifp)) { - rt->rt_refcnt--; rt0->rt_gwroute = NULL; senderr(EHOSTUNREACH); } @@ -2214,9 +2242,15 @@ nd6_output(struct ifnet *ifp, struct ifn * 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)) != NULL) - ln = (struct llinfo_nd6 *)rt->rt_llinfo; + if (nd6_is_addr_neighbor(dst, ifp)) { + if (rt != NULL && rt != rt00) + rtfree(rt); + + RTFREE_IF_NEEDED(rt); + rt = nd6_lookup(&dst->sin6_addr, 1, ifp); + if (rt != NULL) + ln = (struct llinfo_nd6 *)rt->rt_llinfo; + } } if (ln == NULL || rt == NULL) { if ((ifp->if_flags & IFF_POINTOPOINT) == 0 && @@ -2308,7 +2342,8 @@ nd6_output(struct ifnet *ifp, struct ifn (long)ND_IFINFO(ifp)->retrans * hz / 1000); nd6_ns_output(ifp, NULL, &dst->sin6_addr, ln, 0); } - return 0; + error = 0; + goto exit; sendpkt: /* discard the packet if IPv6 operation is disabled on the interface */ @@ -2327,12 +2362,16 @@ nd6_output(struct ifnet *ifp, struct ifn #ifndef NET_MPSAFE KERNEL_UNLOCK_ONE(NULL); #endif - return error; + goto exit; bad: if (m != NULL) m_freem(m); + exit: + RTFREE_IF_NEEDED(rt); + return error; +#undef RTFREE_IF_NEEDED } #undef senderr Index: src/sys/netinet6/nd6.h diff -u src/sys/netinet6/nd6.h:1.65 src/sys/netinet6/nd6.h:1.66 --- src/sys/netinet6/nd6.h:1.65 Wed Jul 15 09:20:18 2015 +++ src/sys/netinet6/nd6.h Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: nd6.h,v 1.65 2015/07/15 09:20:18 ozaki-r Exp $ */ +/* $NetBSD: nd6.h,v 1.66 2015/07/17 02:21:08 ozaki-r Exp $ */ /* $KAME: nd6.h,v 1.95 2002/06/08 11:31:06 itojun Exp $ */ /* @@ -422,7 +422,7 @@ int nd6_resolve(struct ifnet *, struct r struct mbuf *, struct sockaddr *, u_char *); void nd6_rtrequest(int, struct rtentry *, const struct rt_addrinfo *); int nd6_ioctl(u_long, void *, struct ifnet *); -struct rtentry *nd6_cache_lladdr(struct ifnet *, struct in6_addr *, +void nd6_cache_lladdr(struct ifnet *, struct in6_addr *, char *, int, int, int); int nd6_output(struct ifnet *, struct ifnet *, struct mbuf *, const struct sockaddr_in6 *, struct rtentry *); Index: src/sys/netinet6/nd6_nbr.c diff -u src/sys/netinet6/nd6_nbr.c:1.108 src/sys/netinet6/nd6_nbr.c:1.109 --- src/sys/netinet6/nd6_nbr.c:1.108 Mon Apr 27 10:14:44 2015 +++ src/sys/netinet6/nd6_nbr.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: nd6_nbr.c,v 1.108 2015/04/27 10:14:44 ozaki-r Exp $ */ +/* $NetBSD: nd6_nbr.c,v 1.109 2015/07/17 02:21:08 ozaki-r Exp $ */ /* $KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $ */ /* @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.108 2015/04/27 10:14:44 ozaki-r Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.109 2015/07/17 02:21:08 ozaki-r Exp $"); #include "opt_inet.h" @@ -562,7 +562,7 @@ nd6_na_input(struct mbuf *m, int off, in 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; struct sockaddr_in6 ssin6; @@ -826,6 +826,8 @@ nd6_na_input(struct mbuf *m, int off, in freeit: m_freem(m); + if (rt != NULL) + rtfree(rt); return; bad: Index: src/sys/netinet6/nd6_rtr.c diff -u src/sys/netinet6/nd6_rtr.c:1.100 src/sys/netinet6/nd6_rtr.c:1.101 --- src/sys/netinet6/nd6_rtr.c:1.100 Tue Jun 30 06:42:06 2015 +++ src/sys/netinet6/nd6_rtr.c Fri Jul 17 02:21:08 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: nd6_rtr.c,v 1.100 2015/06/30 06:42:06 ozaki-r Exp $ */ +/* $NetBSD: nd6_rtr.c,v 1.101 2015/07/17 02:21:08 ozaki-r Exp $ */ /* $KAME: nd6_rtr.c,v 1.95 2001/02/07 08:09:47 itojun Exp $ */ /* @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: nd6_rtr.c,v 1.100 2015/06/30 06:42:06 ozaki-r Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nd6_rtr.c,v 1.101 2015/07/17 02:21:08 ozaki-r Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -105,6 +105,23 @@ int nd6_numroutes = 0; #define RTPREF_RESERVED (-2) #define RTPREF_INVALID (-3) /* internal */ +static inline bool +nd6_is_llinfo_probreach(struct nd_defrouter *dr) +{ + struct rtentry *rt = NULL; + struct llinfo_nd6 *ln = NULL; + + rt = nd6_lookup(&dr->rtaddr, 0, dr->ifp); + if (rt == NULL) + return false; + ln = (struct llinfo_nd6 *)rt->rt_llinfo; + rtfree(rt); + if (ln == NULL || !ND6_IS_LLINFO_PROBREACH(ln)) + return false; + + return true; +} + /* * Receive Router Solicitation Message - just for routers. * Router solicitation/advertisement is mostly managed by a userland program @@ -427,7 +444,6 @@ defrouter_addreq(struct nd_defrouter *ne struct sockaddr_in6 sin6; struct sockaddr sa; } def, mask, gate; - struct rtentry *newrt = NULL; int s; int error; @@ -444,15 +460,12 @@ defrouter_addreq(struct nd_defrouter *ne #endif s = splsoftnet(); - error = rtrequest(RTM_ADD, &def.sa, &gate.sa, &mask.sa, - RTF_GATEWAY, &newrt); - if (newrt) { - rt_newmsg(RTM_ADD, newrt); /* tell user process */ - newrt->rt_refcnt--; + error = rtrequest_newmsg(RTM_ADD, &def.sa, &gate.sa, &mask.sa, + RTF_GATEWAY); + if (error == 0) { nd6_numroutes++; - } - if (error == 0) newdr->installed = 1; + } splx(s); return; } @@ -539,7 +552,7 @@ defrouter_delreq(struct nd_defrouter *dr struct sockaddr_in6 sin6; struct sockaddr sa; } def, mask, gw; - struct rtentry *oldrt = NULL; + int error; #ifdef DIAGNOSTIC if (dr == NULL) @@ -558,19 +571,10 @@ defrouter_delreq(struct nd_defrouter *dr gw.sin6.sin6_scope_id = 0; /* XXX */ #endif - rtrequest(RTM_DELETE, &def.sa, &gw.sa, &mask.sa, RTF_GATEWAY, &oldrt); - if (oldrt) { - rt_newmsg(RTM_DELETE, oldrt); - if (oldrt->rt_refcnt <= 0) { - /* - * XXX: borrowed from the RTM_DELETE case of - * rtrequest(). - */ - oldrt->rt_refcnt++; - rtfree(oldrt); - } + error = rtrequest_newmsg(RTM_DELETE, &def.sa, &gw.sa, &mask.sa, + RTF_GATEWAY); + if (error == 0) nd6_numroutes--; - } dr->installed = 0; } @@ -620,8 +624,6 @@ defrouter_select(void) struct nd_ifinfo *ndi; int s = splsoftnet(); struct nd_defrouter *dr, *selected_dr = NULL, *installed_dr = NULL; - struct rtentry *rt = NULL; - struct llinfo_nd6 *ln = NULL; /* * This function should be called only when acting as an autoconfigured @@ -658,11 +660,8 @@ defrouter_select(void) continue; if (selected_dr == NULL && - (rt = nd6_lookup(&dr->rtaddr, 0, dr->ifp)) != NULL && - (ln = (struct llinfo_nd6 *)rt->rt_llinfo) != NULL && - ND6_IS_LLINFO_PROBREACH(ln)) { + nd6_is_llinfo_probreach(dr)) selected_dr = dr; - } if (dr->installed && !installed_dr) installed_dr = dr; @@ -686,9 +685,7 @@ defrouter_select(void) else selected_dr = TAILQ_NEXT(installed_dr, dr_entry); } else if (installed_dr && - (rt = nd6_lookup(&installed_dr->rtaddr, 0, installed_dr->ifp)) && - (ln = (struct llinfo_nd6 *)rt->rt_llinfo) && - ND6_IS_LLINFO_PROBREACH(ln) && + nd6_is_llinfo_probreach(installed_dr) && rtpref(selected_dr) <= rtpref(installed_dr)) { selected_dr = installed_dr; } @@ -1391,17 +1388,12 @@ static struct nd_pfxrouter * find_pfxlist_reachable_router(struct nd_prefix *pr) { struct nd_pfxrouter *pfxrtr; - struct rtentry *rt; - struct llinfo_nd6 *ln; for (pfxrtr = LIST_FIRST(&pr->ndpr_advrtrs); pfxrtr; pfxrtr = LIST_NEXT(pfxrtr, pfr_entry)) { if (pfxrtr->router->ifp->if_flags & IFF_UP && pfxrtr->router->ifp->if_link_state != LINK_STATE_DOWN && - (rt = nd6_lookup(&pfxrtr->router->rtaddr, 0, - pfxrtr->router->ifp)) && - (ln = (struct llinfo_nd6 *)rt->rt_llinfo) && - ND6_IS_LLINFO_PROBREACH(ln)) + nd6_is_llinfo_probreach(pfxrtr->router)) break; /* found */ } @@ -1611,7 +1603,6 @@ nd6_prefix_onlink(struct nd_prefix *pr) struct nd_prefix *opr; u_long rtflags; int error = 0; - struct rtentry *rt = NULL; /* sanity check */ if ((pr->ndpr_stateflags & NDPRF_ONLINK) != 0) { @@ -1689,13 +1680,10 @@ nd6_prefix_onlink(struct nd_prefix *pr) */ rtflags &= ~RTF_CLONING; } - error = rtrequest(RTM_ADD, (struct sockaddr *)&pr->ndpr_prefix, - ifa->ifa_addr, (struct sockaddr *)&mask6, rtflags, &rt); + error = rtrequest_newmsg(RTM_ADD, (struct sockaddr *)&pr->ndpr_prefix, + ifa->ifa_addr, (struct sockaddr *)&mask6, rtflags); if (error == 0) { - if (rt != NULL) { /* this should be non NULL, though */ - rt_newmsg(RTM_ADD, rt); - nd6_numroutes++; - } + nd6_numroutes++; pr->ndpr_stateflags |= NDPRF_ONLINK; } else { nd6log((LOG_ERR, "nd6_prefix_onlink: failed to add route for a" @@ -1707,9 +1695,6 @@ nd6_prefix_onlink(struct nd_prefix *pr) ip6_sprintf(&mask6.sin6_addr), rtflags, error)); } - if (rt != NULL) - rt->rt_refcnt--; - return (error); } @@ -1720,7 +1705,6 @@ nd6_prefix_offlink(struct nd_prefix *pr) struct ifnet *ifp = pr->ndpr_ifp; struct nd_prefix *opr; struct sockaddr_in6 sa6, mask6; - struct rtentry *rt = NULL; /* sanity check */ if ((pr->ndpr_stateflags & NDPRF_ONLINK) == 0) { @@ -1732,16 +1716,11 @@ nd6_prefix_offlink(struct nd_prefix *pr) sockaddr_in6_init(&sa6, &pr->ndpr_prefix.sin6_addr, 0, 0, 0); sockaddr_in6_init(&mask6, &pr->ndpr_mask, 0, 0, 0); - error = rtrequest(RTM_DELETE, (struct sockaddr *)&sa6, NULL, - (struct sockaddr *)&mask6, 0, &rt); + error = rtrequest_newmsg(RTM_DELETE, (struct sockaddr *)&sa6, NULL, + (struct sockaddr *)&mask6, 0); if (error == 0) { pr->ndpr_stateflags &= ~NDPRF_ONLINK; - - /* report the route deletion to the routing socket. */ - if (rt != NULL) { - rt_newmsg(RTM_DELETE, rt); - nd6_numroutes--; - } + nd6_numroutes--; /* * There might be the same prefix on another interface, @@ -1789,15 +1768,7 @@ nd6_prefix_offlink(struct nd_prefix *pr) error)); } - if (rt != NULL) { - if (rt->rt_refcnt <= 0) { - /* XXX: we should free the entry ourselves. */ - rt->rt_refcnt++; - rtfree(rt); - } - } - - return (error); + return error; } static struct in6_ifaddr *