diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h --- a/sys/netinet6/nd6.h +++ b/sys/netinet6/nd6.h @@ -240,6 +240,7 @@ u_long advint_expire; /* Mobile IPv6 addition */ int advints_lost; /* Mobile IPv6 addition */ struct ifnet *ifp; + int refcnt; /* reference count (list + callers + pfxrtr) */ }; struct nd_prefix { diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c --- a/sys/netinet6/nd6_rtr.c +++ b/sys/netinet6/nd6_rtr.c @@ -75,6 +75,9 @@ struct nd_defrouter *); static void pfxrtr_add (struct nd_prefix *, struct nd_defrouter *); static void pfxrtr_del (struct nd_pfxrouter *); +/* refcount helpers for default router entries (see defrtrlist_del) */ +static void nd_defrouter_hold (struct nd_defrouter *); +static void nd_defrouter_rele (struct nd_defrouter *); static struct nd_pfxrouter *find_pfxlist_reachable_router (struct nd_prefix *); static void defrouter_addifreq (struct ifnet *); @@ -346,6 +349,14 @@ } /* + * The default router pointer returned by defrtrlist_update() carried + * a reference so it stayed valid across the unlocked prefix loop + * above. Drop it now that we are done consuming it. + */ + if (dr) + nd_defrouter_rele(dr); + + /* * MTU */ if (ndopts.nd_opts_mtu && ndopts.nd_opts_mtu->nd_opt_mtu_len == 1) { @@ -537,6 +548,30 @@ kfree(dr, M_IP6NDP); } +/* + * Reference counting for default router entries. An entry has one + * reference for its membership in nd_defrouter (the "list" ref). Each + * nd_pfxrouter that points at it holds another reference, and a caller + * that obtains a pointer via defrtrlist_update() holds one for the + * duration of its unlocked use. The entry is only freed once the last + * reference is dropped, which closes the use-after-free race in + * nd6_ra_input() where defrtrlist_update() returned an unreferenced + * pointer that could be freed concurrently while the prefix-option loop + * still referenced it. + */ +static void +nd_defrouter_hold(struct nd_defrouter *dr) +{ + atomic_add_int((volatile u_int *)&dr->refcnt, 1); +} + +static void +nd_defrouter_rele(struct nd_defrouter *dr) +{ + if (atomic_fetchadd_int((volatile u_int *)&dr->refcnt, -1) == 1) + kfree(dr, M_IP6NDP); +} + void defrtrlist_del(struct nd_defrouter *dr) { @@ -573,7 +608,8 @@ if (deldr) defrouter_select(); - kfree(dr, M_IP6NDP); + /* Drop the list membership reference; frees when last ref is gone. */ + nd_defrouter_rele(dr); } /* @@ -676,6 +712,14 @@ dr->rtlifetime = new->rtlifetime; dr->expire = new->expire; } + /* + * Hand out a reference to the caller so the returned + * pointer stays valid after the lock is released. In the + * rtlifetime==0 case defrtrlist_del() already ran and dr is + * NULL, so there is nothing to reference. + */ + if (dr != NULL) + nd_defrouter_hold(dr); mtx_unlock(&nd6_mtx); return (dr); } @@ -693,6 +737,7 @@ return (NULL); } *n = *new; + n->refcnt = 1; /* list membership reference */ /* * Insert the new router at the end of the Default Router List. @@ -703,6 +748,8 @@ if (TAILQ_FIRST(&nd_defrouter) == n) defrouter_select(); mtx_unlock(&nd6_mtx); + /* Hand out a caller reference for the returned pointer. */ + nd_defrouter_hold(n); return (n); } @@ -726,6 +773,7 @@ new = kmalloc(sizeof(*new), M_IP6NDP, M_INTWAIT | M_ZERO); new->router = dr; + nd_defrouter_hold(dr); /* pfxrtr keeps a reference on the router */ LIST_INSERT_HEAD(&pr->ndpr_advrtrs, new, pfr_entry); @@ -736,6 +784,7 @@ pfxrtr_del(struct nd_pfxrouter *pfr) { LIST_REMOVE(pfr, pfr_entry); + nd_defrouter_rele(pfr->router); /* drop the pfxrtr reference */ kfree(pfr, M_IP6NDP); } @@ -838,6 +887,7 @@ for (pfr = pr->ndpr_advrtrs.lh_first; pfr; pfr = next) { next = pfr->pfr_next; + nd_defrouter_rele(pfr->router); /* drop the pfxrtr reference */ kfree(pfr, M_IP6NDP); } mtx_unlock(&nd6_mtx);