DF-5847 / 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 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 | --- a/sys/netinet6/nd6.c 2026-06-29 12:51:19.000000000 +0000 +++ b/sys/netinet6/nd6.c 2026-10-26 06:01:23.129508243 +0000 @@ -143,6 +143,17 @@ struct nd_prhead nd_prefix = { 0 }; struct mtx nd6_mtx = MTX_INITIALIZER("nd6"); +/* + * DF-5847: unlinked llinfo entries are tombstoned (ln_rt = NULL) and + * moved onto this deferred-free list instead of being freed in place. + * A concurrent nd6_timer walk (or ndp ioctl walk) may still hold a + * pointer to the entry as its `next`; tombstoned entries keep their + * ln_next chain intact so such walks continue correctly, and the memory + * is only released by nd6_timer_dispatch() under nd6_mtx after its walk + * has finished. + */ +static struct llinfo_nd6 *nd6_lnfreelist; + int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL; static struct sockaddr_in6 all1_sa; @@ -495,7 +506,15 @@ switch (ln->ln_state) { case ND6_LLINFO_WAITDELETE: + /* + * DF-5847: nd6_free() deletes the route and reaches + * nd6_rtrequest(), which now takes nd6_mtx; drop the + * mutex around it. The saved `next` stays safe: + * concurrent deletes only tombstone entries. + */ + mtx_unlock(&nd6_mtx); next = nd6_free(rt); + mtx_lock(&nd6_mtx); break; case ND6_LLINFO_INCOMPLETE: if (ln->ln_asked++ >= nd6_mmaxtries) { @@ -538,8 +557,11 @@ case ND6_LLINFO_STALE: /* Garbage Collection(RFC 2461 5.3) */ - if (ln->ln_expire) + if (ln->ln_expire) { + mtx_unlock(&nd6_mtx); /* DF-5847 */ next = nd6_free(rt); + mtx_lock(&nd6_mtx); + } break; case ND6_LLINFO_DELAY: @@ -566,7 +588,9 @@ &dst->sin6_addr, ln, 0); } else { rt_rtmsg(RTM_MISS, rt, rt->rt_ifp, 0); + mtx_unlock(&nd6_mtx); /* DF-5847 */ next = nd6_free(rt); + mtx_lock(&nd6_mtx); } break; } @@ -611,11 +635,22 @@ */ if (ip6_use_tempaddr && (ia6->ia6_flags & IN6_IFF_TEMPORARY)) { + mtx_unlock(&nd6_mtx); /* DF-5847 */ if (regen_tmpaddr(ia6) == 0) regen = 1; + mtx_lock(&nd6_mtx); } + /* + * DF-5847: in6_purgeaddr() tears down routes and + * reaches nd6_rtrequest(), which now takes nd6_mtx. + * The in6_ifaddr chain is only mutated on netisr0 + * (ioctls are dispatched there), so dropping nd6_mtx + * here does not expose the ia6 walk on this system. + */ + mtx_unlock(&nd6_mtx); in6_purgeaddr(&ia6->ia_ifa); + mtx_lock(&nd6_mtx); if (regen) goto addrloop; /* XXX: see below */ @@ -666,6 +701,17 @@ } } + /* + * DF-5847: release tombstoned llinfo entries now that the walk is + * over and no other nd6_mtx-holding walker can still reference them. + */ + while (nd6_lnfreelist != NULL) { + struct llinfo_nd6 *fln = nd6_lnfreelist; + + nd6_lnfreelist = fln->ln_prev; + R_Free(fln); + } + /* expire prefix list */ pr = nd_prefix.lh_first; while (pr) { @@ -1037,6 +1083,8 @@ */ /* XXX: this condition too restrictive? */ + int need_rt6_flush = 0; + if (!ip6_forwarding && (ND_IFINFO(rt->rt_ifp)->flags & ND6_IFF_ACCEPT_RTADV)) { mtx_lock(&nd6_mtx); @@ -1050,7 +1098,7 @@ * is in the Default Router List. * See a corresponding comment in nd6_na_input(). */ - rt6_flush(&in6, rt->rt_ifp); + need_rt6_flush = 1; } if (dr) { @@ -1070,12 +1118,15 @@ ln->ln_state = ND6_LLINFO_INCOMPLETE; /* - * Since defrouter_select() does not affect the - * on-link determination and MIP6 needs the check - * before the default router selection, we perform - * the check now. + * DF-5847: pfxlist_onlink_check() and + * defrouter_select() add/delete routes and reach + * nd6_rtrequest(), which now takes nd6_mtx -- run + * them with the mutex released (order otherwise as + * before: onlink check first, then router rotation). */ + mtx_unlock(&nd6_mtx); pfxlist_onlink_check(); + mtx_lock(&nd6_mtx); if (dr == TAILQ_FIRST(&nd_defrouter)) { /* @@ -1087,13 +1138,17 @@ */ TAILQ_REMOVE(&nd_defrouter, dr, dr_entry); TAILQ_INSERT_TAIL(&nd_defrouter, dr, dr_entry); - + mtx_unlock(&nd6_mtx); defrouter_select(); + mtx_lock(&nd6_mtx); } } mtx_unlock(&nd6_mtx); } + if (need_rt6_flush) + rt6_flush(&in6, rt->rt_ifp); + /* * Before deleting the entry, remember the next entry as the * return value. We need this because pfxlist_onlink_check() above @@ -1292,11 +1347,18 @@ * add with a LL address. */ R_Malloc(ln, struct llinfo_nd6 *, sizeof(*ln)); - rt->rt_llinfo = (caddr_t)ln; if (!ln) { log(LOG_DEBUG, "nd6_rtrequest: malloc failed\n"); break; } + /* + * DF-5847: the llinfo_nd6 list is walked by + * nd6_timer_dispatch() and nd6 ioctl handlers under + * nd6_mtx; serialize the publication of the new entry + * (ln_rt must be visible before the list linkage). + */ + mtx_lock(&nd6_mtx); + rt->rt_llinfo = (caddr_t)ln; nd6_inuse++; nd6_allocated++; bzero(ln, sizeof(*ln)); @@ -1323,6 +1385,7 @@ llinfo_nd6.ln_next = ln; ln->ln_prev = &llinfo_nd6; ln->ln_next->ln_prev = ln; + mtx_unlock(&nd6_mtx); /* * check if rt_key(rt) is one of my address assigned @@ -1407,15 +1470,29 @@ if (in6m) in6_delmulti(in6m); } + if (ln->ln_hold) { + m_freem(ln->ln_hold); + ln->ln_hold = NULL; + } + /* + * DF-5847: unlink under nd6_mtx, then tombstone (ln_rt = + * NULL) and defer the free. A concurrent timer/ioctl walk + * may still hold this entry as its saved `next`; the + * tombstone makes those walkers skip it (they check + * ln_rt != NULL), its ln_next stays valid so walks can + * continue past it, and nd6_timer_dispatch() releases the + * memory under nd6_mtx after its walk completes. + */ + mtx_lock(&nd6_mtx); nd6_inuse--; ln->ln_next->ln_prev = ln->ln_prev; ln->ln_prev->ln_next = ln->ln_next; - ln->ln_prev = NULL; + ln->ln_prev = nd6_lnfreelist; + nd6_lnfreelist = ln; + ln->ln_rt = NULL; /* tombstone */ rt->rt_llinfo = 0; rt->rt_flags &= ~RTF_LLINFO; - if (ln->ln_hold) - m_freem(ln->ln_hold); - R_Free(ln); + mtx_unlock(&nd6_mtx); } } |