DragonFlyBSD Kernel Audit
DF-0513 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/net/ip6fw/ip6_fw.c b/sys/net/ip6fw/ip6_fw.c
--- a/sys/net/ip6fw/ip6_fw.c
+++ b/sys/net/ip6fw/ip6_fw.c
@@ -100,6 +100,20 @@
 
 MALLOC_DEFINE(M_IP6FW, "Ip6Fw/Ip6Acct", "Ip6Fw/Ip6Acct chain's");
 
+#include <sys/thread.h>
+#include <sys/msgport.h>
+#include <sys/globaldata.h>
+
+/*
+ * Serializes ip6_fw_chain walks (ip6_fw_chk) against chain mutations
+ * (add/del/flush/zero/get).  Previously only crit_enter/exit guarded
+ * the mutators, which masks interrupts on the CURRENT CPU only and
+ * does not serialize against other CPUs running ip6_fw_chk -- leading
+ * to UAF when a freed rule is dereferenced by a concurrent walker.
+ * See DF-0513.
+ */
+static struct lwkt_token ip6_fw_token = LWKT_TOKEN_INITIALIZER(ip6_fw_token);
+
 static int fw6_debug = 1;
 #ifdef IPV6FIREWALL_VERBOSE
 static int fw6_verbose = 1;
@@ -512,6 +526,8 @@
 #endif
 
 	*cookie = 0;
+
+	lwkt_gettoken(&ip6_fw_token);
 	/*
 	 * Go down the chain, looking for enlightment
 	 * #ifdef IP6FW_DIVERT_RESTART
@@ -834,6 +850,7 @@
 		m_freem(*m);
 		*m = NULL;
 	}
+	lwkt_reltoken(&ip6_fw_token);
 	return(0);
 }
 
@@ -853,16 +870,19 @@
 	ftmp->fw_bcnt = 0L;
 	fwc->rule = ftmp;
 
+	lwkt_gettoken(&ip6_fw_token);
 	crit_enter();
 
 	if (!chainptr->lh_first) {
 		LIST_INSERT_HEAD(chainptr, fwc, chain);
 		crit_exit();
+		lwkt_reltoken(&ip6_fw_token);
 		return(0);
         } else if (ftmp->fw_number == (u_short)-1) {
 		if (fwc)  kfree(fwc, M_IP6FW);
 		if (ftmp) kfree(ftmp, M_IP6FW);
 		crit_exit();
+		lwkt_reltoken(&ip6_fw_token);
 		dprintf(("%s bad rule number\n", err_prefix));
 		return (EINVAL);
         }
@@ -895,6 +915,7 @@
 	}
 
 	crit_exit();
+	lwkt_reltoken(&ip6_fw_token);
 	return (0);
 }
 
@@ -903,6 +924,7 @@
 {
 	struct ip6_fw_chain *fcp;
 
+	lwkt_gettoken(&ip6_fw_token);
 	crit_enter();
 
 	fcp = chainptr->lh_first;
@@ -910,15 +932,17 @@
 		for (; fcp; fcp = fcp->chain.le_next) {
 			if (fcp->rule->fw_number == number) {
 				LIST_REMOVE(fcp, chain);
-				crit_exit();
 				kfree(fcp->rule, M_IP6FW);
 				kfree(fcp, M_IP6FW);
+				crit_exit();
+				lwkt_reltoken(&ip6_fw_token);
 				return 0;
 			}
 		}
 	}
 
 	crit_exit();
+	lwkt_reltoken(&ip6_fw_token);
 	return (EINVAL);
 }
 
@@ -941,6 +965,7 @@
 	 *	same number, so we don't stop after finding the first
 	 *	match if zeroing a specific entry.
 	 */
+	lwkt_gettoken(&ip6_fw_token);
 	crit_enter();
 	for (fcp = ip6_fw_chain.lh_first; fcp; fcp = fcp->chain.le_next)
 		if (!frwl || frwl->fw_number == fcp->rule->fw_number) {
@@ -948,6 +973,7 @@
 			fcp->rule->timestamp = 0;
 		}
 	crit_exit();
+	lwkt_reltoken(&ip6_fw_token);
 
 	if (fw6_verbose) {
 		if (frwl)
@@ -1103,6 +1129,7 @@
 		*mm = m = m_get(M_WAITOK, MT_DATA); /* XXX */
 		if (sizeof *(fcp->rule) > MLEN)
 			MCLGET(m, M_WAITOK);
+		lwkt_gettoken(&ip6_fw_token);
 		for (; fcp; fcp = fcp->chain.le_next) {
 			bcopy(fcp->rule, m->m_data, sizeof *(fcp->rule));
 			m->m_len = sizeof *(fcp->rule);
@@ -1112,6 +1139,7 @@
 				MCLGET(m, M_WAITOK);
 			m->m_len = 0;
 		}
+		lwkt_reltoken(&ip6_fw_token);
 		return (0);
 	}
 	m = *mm;
@@ -1124,15 +1152,17 @@
 		return(EPERM);
 	}
 	if (stage == IPV6_FW_FLUSH) {
+		lwkt_gettoken(&ip6_fw_token);
 		while (ip6_fw_chain.lh_first != NULL &&
 		    ip6_fw_chain.lh_first->rule->fw_number != (u_short)-1) {
 			struct ip6_fw_chain *fcp = ip6_fw_chain.lh_first;
 			crit_enter();
 			LIST_REMOVE(ip6_fw_chain.lh_first, chain);
-			crit_exit();
 			kfree(fcp->rule, M_IP6FW);
 			kfree(fcp, M_IP6FW);
+			crit_exit();
 		}
+		lwkt_reltoken(&ip6_fw_token);
 		if (m) {
 			m_freem(m);
 			*mm = NULL;