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