DF-0417 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 | 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); |