DragonFlyBSD Kernel Audit
DF-0732 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/netproto/802_11/wlan_acl/ieee80211_acl.c b/sys/netproto/802_11/wlan_acl/ieee80211_acl.c
index 0000000..0000000 100644
--- a/sys/netproto/802_11/wlan_acl/ieee80211_acl.c
+++ b/sys/netproto/802_11/wlan_acl/ieee80211_acl.c
@@ -310,23 +310,37 @@
 		ireq->i_val = as->as_policy;
 		return 0;
 	case IEEE80211_MACCMD_LIST:
+		/*
+		 * DF-0732: take the ACL lock BEFORE reading as_nacls so the
+		 * size used for kmalloc and the list iterated by TAILQ_FOREACH
+		 * cannot diverge.  Previously the unlocked read here raced with
+		 * concurrent ADDMAC/DELMAC, causing either a heap OOB write
+		 * (grow) or an uninitialised-heap info leak via copyout (shrink).
+		 * ACL_LOCK is a sleepable lockmgr lock, so holding it across
+		 * kmalloc(M_INTWAIT) is safe.
+		 */
+		ACL_LOCK(as);
 		space = as->as_nacls * IEEE80211_ADDR_LEN;
 		if (ireq->i_len == 0) {
 			ireq->i_len = space;	/* return required space */
+			ACL_UNLOCK(as);
 			return 0;		/* NB: must not error */
 		}
 #if defined(__DragonFly__)
 		ap = (struct ieee80211req_maclist *) kmalloc(space,
-		    M_TEMP, M_INTWAIT);
+		    M_TEMP, M_INTWAIT | M_ZERO);
 #else
 		ap = (struct ieee80211req_maclist *) IEEE80211_MALLOC(space,
-		    M_TEMP, IEEE80211_M_NOWAIT);
+		    M_TEMP, IEEE80211_M_NOWAIT | IEEE80211_M_ZERO);
 #endif
-		if (ap == NULL)
+		if (ap == NULL) {
+			ACL_UNLOCK(as);
 			return ENOMEM;
+		}
 		i = 0;
-		ACL_LOCK(as);
 		TAILQ_FOREACH(acl, &as->as_list, acl_list) {
+			if (i >= space / IEEE80211_ADDR_LEN)	/* bound to buffer */
+				break;
 			IEEE80211_ADDR_COPY(ap[i].ml_macaddr, acl->acl_macaddr);
 			i++;
 		}