⬢ DragonFlyBSD Kernel Audit
DF-5847 / fix.diff
← back to finding ↓ download raw
--- 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);
 	}
 }