Author: markj Date: Thu Feb 25 20:12:05 2016 New Revision: 296063 URL: https://svnweb.freebsd.org/changeset/base/296063
Log: Lock the NDP default router list and count defrouter references. This addresses a number of race conditions that can cause crashes as a result of unsynchronized access to the list. PR: 206904 Tested by: Larry Rosenman <[email protected]>, Kevin Bowling <[email protected]> MFC after: 2 months Differential Revision: https://reviews.freebsd.org/D5315 Modified: head/sys/netinet6/nd6.c head/sys/netinet6/nd6.h head/sys/netinet6/nd6_nbr.c head/sys/netinet6/nd6_rtr.c Modified: head/sys/netinet6/nd6.c ============================================================================== --- head/sys/netinet6/nd6.c Thu Feb 25 20:02:42 2016 (r296062) +++ head/sys/netinet6/nd6.c Thu Feb 25 20:12:05 2016 (r296063) @@ -115,6 +115,7 @@ static eventhandler_tag lle_event_eh, if VNET_DEFINE(struct nd_drhead, nd_defrouter); VNET_DEFINE(struct nd_prhead, nd_prefix); +VNET_DEFINE(struct rwlock, nd6_lock); VNET_DEFINE(int, nd6_recalc_reachtm_interval) = ND6_RECALC_REACHTM_INTERVAL; #define V_nd6_recalc_reachtm_interval VNET(nd6_recalc_reachtm_interval) @@ -205,6 +206,8 @@ void nd6_init(void) { + rw_init(&V_nd6_lock, "nd6"); + LIST_INIT(&V_nd_prefix); /* initialization of the default router list */ @@ -235,6 +238,7 @@ nd6_destroy() EVENTHANDLER_DEREGISTER(lle_event, lle_event_eh); EVENTHANDLER_DEREGISTER(iflladdr_event, iflladdr_event_eh); } + rw_destroy(&V_nd6_lock); } #endif @@ -884,6 +888,7 @@ void nd6_timer(void *arg) { CURVNET_SET((struct vnet *) arg); + struct nd_drhead drq; struct nd_defrouter *dr, *ndr; struct nd_prefix *pr, *npr; struct in6_ifaddr *ia6, *nia6; @@ -891,10 +896,18 @@ nd6_timer(void *arg) callout_reset(&V_nd6_timer_ch, V_nd6_prune * hz, nd6_timer, curvnet); + TAILQ_INIT(&drq); + /* expire default router list */ - TAILQ_FOREACH_SAFE(dr, &V_nd_defrouter, dr_entry, ndr) { + ND6_WLOCK(); + TAILQ_FOREACH_SAFE(dr, &V_nd_defrouter, dr_entry, ndr) if (dr->expire && dr->expire < time_uptime) - defrtrlist_del(dr); + defrouter_unlink(dr, &drq); + ND6_WUNLOCK(); + + while ((dr = TAILQ_FIRST(&drq)) != NULL) { + TAILQ_REMOVE(&drq, dr, dr_entry); + defrouter_del(dr); } /* @@ -1089,29 +1102,37 @@ regen_tmpaddr(struct in6_ifaddr *ia6) void nd6_purge(struct ifnet *ifp) { + struct nd_drhead drq; struct nd_defrouter *dr, *ndr; struct nd_prefix *pr, *npr; + TAILQ_INIT(&drq); + /* * Nuke default router list entries toward ifp. * We defer removal of default router list entries that is installed * in the routing table, in order to keep additional side effects as * small as possible. */ + ND6_WLOCK(); TAILQ_FOREACH_SAFE(dr, &V_nd_defrouter, dr_entry, ndr) { if (dr->installed) continue; - if (dr->ifp == ifp) - defrtrlist_del(dr); + defrouter_unlink(dr, &drq); } TAILQ_FOREACH_SAFE(dr, &V_nd_defrouter, dr_entry, ndr) { if (!dr->installed) continue; - if (dr->ifp == ifp) - defrtrlist_del(dr); + defrouter_unlink(dr, &drq); + } + ND6_WUNLOCK(); + + while ((dr = TAILQ_FIRST(&drq)) != NULL) { + TAILQ_REMOVE(&drq, dr, dr_entry); + defrouter_del(dr); } /* Nuke prefix list entries toward ifp */ @@ -1357,8 +1378,8 @@ nd6_free(struct llentry *ln, int gc) /* cancel timer */ nd6_llinfo_settimer_locked(ln, -1); + dr = NULL; ifp = ln->lle_tbl->llt_ifp; - if (ND_IFINFO(ifp)->flags & ND6_IFF_ACCEPT_RTADV) { dr = defrouter_lookup(&ln->r_l3addr.addr6, ifp); @@ -1385,6 +1406,7 @@ nd6_free(struct llentry *ln, int gc) LLE_REMREF(ln); LLE_WUNLOCK(ln); + defrouter_rele(dr); return; } @@ -1465,6 +1487,8 @@ nd6_free(struct llentry *ln, int gc) IF_AFDATA_UNLOCK(ifp); llentry_free(ln); + if (dr != NULL) + defrouter_rele(dr); } static int @@ -1525,12 +1549,13 @@ nd6_rtrequest(int req, struct rtentry *r /* * check for default route */ - if (IN6_ARE_ADDR_EQUAL(&in6addr_any, - &SIN6(rt_key(rt))->sin6_addr)) { - + if (IN6_ARE_ADDR_EQUAL(&in6addr_any, + &SIN6(rt_key(rt))->sin6_addr)) { dr = defrouter_lookup(&gateway->sin6_addr, ifp); - if (dr != NULL) + if (dr != NULL) { dr->installed = 0; + defrouter_rele(dr); + } } break; } @@ -1718,12 +1743,22 @@ nd6_ioctl(u_long cmd, caddr_t data, stru case SIOCSRTRFLUSH_IN6: { /* flush all the default routers */ - struct nd_defrouter *dr, *next; + struct nd_drhead drq; + struct nd_defrouter *dr; + + TAILQ_INIT(&drq); defrouter_reset(); - TAILQ_FOREACH_SAFE(dr, &V_nd_defrouter, dr_entry, next) { - defrtrlist_del(dr); + + ND6_WLOCK(); + while ((dr = TAILQ_FIRST(&V_nd_defrouter)) != NULL) + defrouter_unlink(dr, &drq); + ND6_WUNLOCK(); + while ((dr = TAILQ_FIRST(&drq)) != NULL) { + TAILQ_REMOVE(&drq, dr, dr_entry); + defrouter_del(dr); } + defrouter_select(); break; } @@ -2535,30 +2570,33 @@ nd6_sysctl_drlist(SYSCTL_HANDLER_ARGS) struct nd_defrouter *dr; int error; - if (req->newptr) + if (req->newptr != NULL) return (EPERM); + error = sysctl_wire_old_buffer(req, 0); + if (error != 0) + return (error); + bzero(&d, sizeof(d)); d.rtaddr.sin6_family = AF_INET6; d.rtaddr.sin6_len = sizeof(d.rtaddr); - /* - * XXX locking - */ + ND6_RLOCK(); TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) { d.rtaddr.sin6_addr = dr->rtaddr; error = sa6_recoverscope(&d.rtaddr); if (error != 0) - return (error); + break; d.flags = dr->raflags; d.rtlifetime = dr->rtlifetime; d.expire = dr->expire + (time_second - time_uptime); d.if_index = dr->ifp->if_index; error = SYSCTL_OUT(req, &d, sizeof(d)); if (error != 0) - return (error); + break; } - return (0); + ND6_RUNLOCK(); + return (error); } static int Modified: head/sys/netinet6/nd6.h ============================================================================== --- head/sys/netinet6/nd6.h Thu Feb 25 20:02:42 2016 (r296062) +++ head/sys/netinet6/nd6.h Thu Feb 25 20:12:05 2016 (r296063) @@ -240,6 +240,7 @@ struct nd_defrouter { u_long expire; struct ifnet *ifp; int installed; /* is installed into kernel routing table */ + u_int refcnt; }; struct nd_prefixctl { @@ -339,6 +340,19 @@ VNET_DECLARE(int, nd6_onlink_ns_rfc4861) #define V_nd6_debug VNET(nd6_debug) #define V_nd6_onlink_ns_rfc4861 VNET(nd6_onlink_ns_rfc4861) +/* Lock for the prefix and default router lists. */ +VNET_DECLARE(struct rwlock, nd6_lock); +#define V_nd6_lock VNET(nd6_lock) + +#define ND6_RLOCK() rw_rlock(&V_nd6_lock) +#define ND6_RUNLOCK() rw_runlock(&V_nd6_lock) +#define ND6_WLOCK() rw_wlock(&V_nd6_lock) +#define ND6_WUNLOCK() rw_wunlock(&V_nd6_lock) +#define ND6_WLOCK_ASSERT() rw_assert(&V_nd6_lock, RA_WLOCKED) +#define ND6_RLOCK_ASSERT() rw_assert(&V_nd6_lock, RA_RLOCKED) +#define ND6_LOCK_ASSERT() rw_assert(&V_nd6_lock, RA_LOCKED) +#define ND6_UNLOCK_ASSERT() rw_assert(&V_nd6_lock, RA_UNLOCKED) + #define nd6log(x) do { if (V_nd6_debug) log x; } while (/*CONSTCOND*/ 0) VNET_DECLARE(struct callout, nd6_timer_ch); @@ -443,12 +457,17 @@ void nd6_rs_input(struct mbuf *, int, in void nd6_ra_input(struct mbuf *, int, int); void defrouter_reset(void); void defrouter_select(void); -void defrtrlist_del(struct nd_defrouter *); +void defrouter_ref(struct nd_defrouter *); +void defrouter_rele(struct nd_defrouter *); +void defrouter_remove(struct nd_defrouter *); +void defrouter_unlink(struct nd_defrouter *, struct nd_drhead *); +void defrouter_del(struct nd_defrouter *); void prelist_remove(struct nd_prefix *); int nd6_prelist_add(struct nd_prefixctl *, struct nd_defrouter *, struct nd_prefix **); void pfxlist_onlink_check(void); struct nd_defrouter *defrouter_lookup(struct in6_addr *, struct ifnet *); +struct nd_defrouter *defrouter_lookup_locked(struct in6_addr *, struct ifnet *); struct nd_prefix *nd6_prefix_lookup(struct nd_prefixctl *); void rt6_flush(struct in6_addr *, struct ifnet *); int nd6_setdefaultiface(int); Modified: head/sys/netinet6/nd6_nbr.c ============================================================================== --- head/sys/netinet6/nd6_nbr.c Thu Feb 25 20:02:42 2016 (r296062) +++ head/sys/netinet6/nd6_nbr.c Thu Feb 25 20:12:05 2016 (r296063) @@ -858,25 +858,28 @@ nd6_na_input(struct mbuf *m, int off, in * update the Destination Cache entries. */ struct nd_defrouter *dr; - struct in6_addr *in6; struct ifnet *nd6_ifp; - in6 = &ln->r_l3addr.addr6; - nd6_ifp = lltable_get_ifp(ln->lle_tbl); - dr = defrouter_lookup(in6, nd6_ifp); - if (dr) - defrtrlist_del(dr); - else if (ND_IFINFO(nd6_ifp)->flags & - ND6_IFF_ACCEPT_RTADV) { - /* - * Even if the neighbor is not in the default - * router list, the neighbor may be used - * as a next hop for some destinations - * (e.g. redirect case). So we must - * call rt6_flush explicitly. - */ - rt6_flush(&ip6->ip6_src, ifp); + ND6_WLOCK(); + dr = defrouter_lookup_locked(&ln->r_l3addr.addr6, + nd6_ifp); + if (dr != NULL) { + /* releases the ND lock */ + defrouter_remove(dr); + dr = NULL; + } else { + ND6_WUNLOCK(); + if ((ND_IFINFO(nd6_ifp)->flags & ND6_IFF_ACCEPT_RTADV) != 0) { + /* + * Even if the neighbor is not in the default + * router list, the neighbor may be used + * as a next hop for some destinations + * (e.g. redirect case). So we must + * call rt6_flush explicitly. + */ + rt6_flush(&ip6->ip6_src, ifp); + } } } ln->ln_router = is_router; Modified: head/sys/netinet6/nd6_rtr.c ============================================================================== --- head/sys/netinet6/nd6_rtr.c Thu Feb 25 20:02:42 2016 (r296062) +++ head/sys/netinet6/nd6_rtr.c Thu Feb 25 20:12:05 2016 (r296063) @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); #include <sys/systm.h> #include <sys/malloc.h> #include <sys/mbuf.h> +#include <sys/refcount.h> #include <sys/socket.h> #include <sys/sockio.h> #include <sys/time.h> @@ -220,6 +221,8 @@ nd6_ra_input(struct mbuf *m, int off, in struct nd_defrouter *dr; char ip6bufs[INET6_ADDRSTRLEN], ip6bufd[INET6_ADDRSTRLEN]; + dr = NULL; + /* * We only accept RAs only when the per-interface flag * ND6_IFF_ACCEPT_RTADV is on the receiving interface. @@ -369,6 +372,10 @@ nd6_ra_input(struct mbuf *m, int off, in (void)prelist_update(&pr, dr, m, mcast); } } + if (dr != NULL) { + defrouter_rele(dr); + dr = NULL; + } /* * MTU @@ -446,10 +453,6 @@ nd6_ra_input(struct mbuf *m, int off, in m_freem(m); } -/* - * default router list proccessing sub routines - */ - /* tell the change to user processes watching the routing socket. */ static void nd6_rtmsg(int cmd, struct rtentry *rt) @@ -478,6 +481,10 @@ nd6_rtmsg(int cmd, struct rtentry *rt) ifa_free(ifa); } +/* + * default router list proccessing sub routines + */ + static void defrouter_addreq(struct nd_defrouter *new) { @@ -506,16 +513,43 @@ defrouter_addreq(struct nd_defrouter *ne } struct nd_defrouter * -defrouter_lookup(struct in6_addr *addr, struct ifnet *ifp) +defrouter_lookup_locked(struct in6_addr *addr, struct ifnet *ifp) { struct nd_defrouter *dr; - TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) { - if (dr->ifp == ifp && IN6_ARE_ADDR_EQUAL(addr, &dr->rtaddr)) + ND6_LOCK_ASSERT(); + TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) + if (dr->ifp == ifp && IN6_ARE_ADDR_EQUAL(addr, &dr->rtaddr)) { + defrouter_ref(dr); return (dr); - } + } + return (NULL); +} - return (NULL); /* search failed */ +struct nd_defrouter * +defrouter_lookup(struct in6_addr *addr, struct ifnet *ifp) +{ + struct nd_defrouter *dr; + + ND6_RLOCK(); + dr = defrouter_lookup_locked(addr, ifp); + ND6_RUNLOCK(); + return (dr); +} + +void +defrouter_ref(struct nd_defrouter *dr) +{ + + refcount_acquire(&dr->refcnt); +} + +void +defrouter_rele(struct nd_defrouter *dr) +{ + + if (refcount_release(&dr->refcnt)) + free(dr, M_IP6NDP); } /* @@ -550,15 +584,41 @@ defrouter_delreq(struct nd_defrouter *dr } /* - * remove all default routes from default router list + * Remove all default routes from default router list. */ void defrouter_reset(void) { - struct nd_defrouter *dr; + struct nd_defrouter *dr, **dra; + int count, i; + + count = i = 0; + /* + * We can't delete routes with the ND lock held, so make a copy of the + * current default router list and use that when deleting routes. + */ + ND6_RLOCK(); TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) - defrouter_delreq(dr); + count++; + ND6_RUNLOCK(); + + dra = malloc(count * sizeof(*dra), M_TEMP, M_WAITOK | M_ZERO); + + ND6_RLOCK(); + TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) { + if (i == count) + break; + defrouter_ref(dr); + dra[i++] = dr; + } + ND6_RUNLOCK(); + + for (i = 0; i < count && dra[i] != NULL; i++) { + defrouter_delreq(dra[i]); + defrouter_rele(dra[i]); + } + free(dra, M_TEMP); /* * XXX should we also nuke any default routers in the kernel, by @@ -566,12 +626,49 @@ defrouter_reset(void) */ } +/* + * Remove a router from the global list and free it. + * + * The ND lock must be held and is released before returning. The caller must + * hold a reference on the router object. + */ void -defrtrlist_del(struct nd_defrouter *dr) +defrouter_remove(struct nd_defrouter *dr) +{ + + ND6_WLOCK_ASSERT(); + KASSERT(dr->refcnt >= 2, ("unexpected refcount 0x%x", dr->refcnt)); + + defrouter_unlink(dr, NULL); + ND6_WUNLOCK(); + defrouter_del(dr); + defrouter_rele(dr); +} + +/* + * Remove a router from the global list and optionally stash it in a + * caller-supplied queue. + * + * The ND lock must be held. + */ +void +defrouter_unlink(struct nd_defrouter *dr, struct nd_drhead *drq) +{ + + ND6_WLOCK_ASSERT(); + TAILQ_REMOVE(&V_nd_defrouter, dr, dr_entry); + if (drq != NULL) + TAILQ_INSERT_TAIL(drq, dr, dr_entry); +} + +void +defrouter_del(struct nd_defrouter *dr) { struct nd_defrouter *deldr = NULL; struct nd_prefix *pr; + ND6_UNLOCK_ASSERT(); + /* * Flush all the routing table entries that use the router * as a next hop. @@ -583,7 +680,6 @@ defrtrlist_del(struct nd_defrouter *dr) deldr = dr; defrouter_delreq(dr); } - TAILQ_REMOVE(&V_nd_defrouter, dr, dr_entry); /* * Also delete all the pointers to the router in each prefix lists. @@ -603,7 +699,10 @@ defrtrlist_del(struct nd_defrouter *dr) if (deldr) defrouter_select(); - free(dr, M_IP6NDP); + /* + * Release the list reference. + */ + defrouter_rele(dr); } /* @@ -630,27 +729,32 @@ defrtrlist_del(struct nd_defrouter *dr) void defrouter_select(void) { - struct nd_defrouter *dr, *selected_dr = NULL, *installed_dr = NULL; + struct nd_defrouter *dr, *selected_dr, *installed_dr; struct llentry *ln = NULL; + ND6_RLOCK(); /* * Let's handle easy case (3) first: * If default router list is empty, there's nothing to be done. */ - if (TAILQ_EMPTY(&V_nd_defrouter)) + if (TAILQ_EMPTY(&V_nd_defrouter)) { + ND6_RUNLOCK(); return; + } /* * Search for a (probably) reachable router from the list. * We just pick up the first reachable one (if any), assuming that * the ordering rule of the list described in defrtrlist_update(). */ + selected_dr = installed_dr = NULL; TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) { IF_AFDATA_RLOCK(dr->ifp); if (selected_dr == NULL && (ln = nd6_lookup(&dr->rtaddr, 0, dr->ifp)) && ND6_IS_LLINFO_PROBREACH(ln)) { selected_dr = dr; + defrouter_ref(selected_dr); } IF_AFDATA_RUNLOCK(dr->ifp); if (ln != NULL) { @@ -658,12 +762,15 @@ defrouter_select(void) ln = NULL; } - if (dr->installed && installed_dr == NULL) - installed_dr = dr; - else if (dr->installed && installed_dr) { - /* this should not happen. warn for diagnosis. */ - log(LOG_ERR, "defrouter_select: more than one router" - " is installed\n"); + if (dr->installed) { + if (installed_dr == NULL) { + installed_dr = dr; + defrouter_ref(installed_dr); + } else { + /* this should not happen. warn for diagnosis. */ + log(LOG_ERR, + "defrouter_select: more than one router is installed\n"); + } } } /* @@ -675,21 +782,25 @@ defrouter_select(void) * or when the new one has a really higher preference value. */ if (selected_dr == NULL) { - if (installed_dr == NULL || !TAILQ_NEXT(installed_dr, dr_entry)) + if (installed_dr == NULL || + TAILQ_NEXT(installed_dr, dr_entry) == NULL) selected_dr = TAILQ_FIRST(&V_nd_defrouter); else selected_dr = TAILQ_NEXT(installed_dr, dr_entry); - } else if (installed_dr) { + defrouter_ref(selected_dr); + } else if (installed_dr != NULL) { IF_AFDATA_RLOCK(installed_dr->ifp); if ((ln = nd6_lookup(&installed_dr->rtaddr, 0, installed_dr->ifp)) && ND6_IS_LLINFO_PROBREACH(ln) && rtpref(selected_dr) <= rtpref(installed_dr)) { + defrouter_rele(selected_dr); selected_dr = installed_dr; } IF_AFDATA_RUNLOCK(installed_dr->ifp); if (ln != NULL) LLE_RUNLOCK(ln); } + ND6_RUNLOCK(); /* * If the selected router is different than the installed one, @@ -697,10 +808,13 @@ defrouter_select(void) * Note that the selected router is never NULL here. */ if (installed_dr != selected_dr) { - if (installed_dr) + if (installed_dr != NULL) { defrouter_delreq(installed_dr); + defrouter_rele(installed_dr); + } defrouter_addreq(selected_dr); } + defrouter_rele(selected_dr); } /* @@ -736,10 +850,11 @@ defrtrlist_update(struct nd_defrouter *n struct nd_defrouter *dr, *n; int oldpref; - if ((dr = defrouter_lookup(&new->rtaddr, new->ifp)) != NULL) { - /* entry exists */ + ND6_WLOCK(); + if ((dr = defrouter_lookup_locked(&new->rtaddr, new->ifp)) != NULL) { if (new->rtlifetime == 0) { - defrtrlist_del(dr); + /* releases the ND lock */ + defrouter_remove(dr); return (NULL); } @@ -755,8 +870,10 @@ defrtrlist_update(struct nd_defrouter *n * to sort the entries. Also make sure the selected * router is still installed in the kernel. */ - if (dr->installed && rtpref(new) == oldpref) + if (dr->installed && rtpref(new) == oldpref) { + ND6_WUNLOCK(); return (dr); + } /* * The preferred router may have changed, so relocate this @@ -768,13 +885,19 @@ defrtrlist_update(struct nd_defrouter *n } /* entry does not exist */ - if (new->rtlifetime == 0) + if (new->rtlifetime == 0) { + ND6_WUNLOCK(); return (NULL); + } n = malloc(sizeof(*n), M_IP6NDP, M_NOWAIT | M_ZERO); - if (n == NULL) + if (n == NULL) { + ND6_WUNLOCK(); return (NULL); + } memcpy(n, new, sizeof(*n)); + /* Initialize with an extra reference for the caller. */ + refcount_init(&n->refcnt, 2); insert: /* @@ -789,10 +912,11 @@ insert: if (rtpref(n) > rtpref(dr)) break; } - if (dr) + if (dr != NULL) TAILQ_INSERT_BEFORE(dr, n, dr_entry); else TAILQ_INSERT_TAIL(&V_nd_defrouter, n, dr_entry); + ND6_WUNLOCK(); defrouter_select(); @@ -821,6 +945,7 @@ pfxrtr_add(struct nd_prefix *pr, struct if (new == NULL) return; new->router = dr; + defrouter_ref(dr); LIST_INSERT_HEAD(&pr->ndpr_advrtrs, new, pfr_entry); @@ -830,7 +955,9 @@ pfxrtr_add(struct nd_prefix *pr, struct static void pfxrtr_del(struct nd_pfxrouter *pfr) { + LIST_REMOVE(pfr, pfr_entry); + defrouter_rele(pfr->router); free(pfr, M_IP6NDP); } @@ -1345,6 +1472,7 @@ pfxlist_onlink_check() * that does not advertise any prefixes. */ if (pr == NULL) { + ND6_RLOCK(); TAILQ_FOREACH(dr, &V_nd_defrouter, dr_entry) { struct nd_prefix *pr0; @@ -1355,6 +1483,7 @@ pfxlist_onlink_check() if (pfxrtr != NULL) break; } + ND6_RUNLOCK(); } if (pr != NULL || (!TAILQ_EMPTY(&V_nd_defrouter) && pfxrtr == NULL)) { /* _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "[email protected]"
