⬢ DragonFlyBSD Kernel Audit
DF-3443 / fix.diff
← back to finding ↓ download raw
--- a/sys/net/altq/altq_priq.c
+++ b/sys/net/altq/altq_priq.c
@@ -162,6 +162,7 @@
 priq_add_queue(struct pf_altq *a)
 {
 	struct priq_if *pif;
+	struct ifnet *ifp;
 	struct ifaltq *ifq;
 	int error;
 
@@ -174,12 +175,30 @@
 	/* XXX not MP safe */
 	if ((pif = a->altq_disc) == NULL)
 		return (EINVAL);
+
+	ifnet_lock();
+
+	/*
+	 * Revalidate the interface before using the ifq pointer captured
+	 * by priq_add_altq().  A pf altq transaction spans multiple ioctls
+	 * and nothing purges altq state when the underlying interface is
+	 * destroyed in between; using the stale pointer would lock and
+	 * unlock the lwkt serializer embedded in the freed ifnet's
+	 * altq_subq array (use-after-free write into the slab free list).
+	 */
+	if ((ifp = ifunit(a->ifname)) == NULL ||
+	    pif->pif_ifq != &ifp->if_snd) {
+		ifnet_unlock();
+		return (ENODEV);
+	}
 	ifq = pif->pif_ifq;
 
 	PRIQ_LOCK(ifq);
 	error = priq_add_queue_locked(a, pif);
 	PRIQ_UNLOCK(ifq);
 
+	ifnet_unlock();
+
 	return error;
 }
 
@@ -198,18 +217,29 @@
 priq_remove_queue(struct pf_altq *a)
 {
 	struct priq_if *pif;
+	struct ifnet *ifp;
 	struct ifaltq *ifq;
 	int error;
 
 	/* XXX not MF safe */
 	if ((pif = a->altq_disc) == NULL)
 		return (EINVAL);
+
+	ifnet_lock();
+
+	if ((ifp = ifunit(a->ifname)) == NULL ||
+	    pif->pif_ifq != &ifp->if_snd) {
+		ifnet_unlock();
+		return (ENODEV);
+	}
 	ifq = pif->pif_ifq;
 
 	PRIQ_LOCK(ifq);
 	error = priq_remove_queue_locked(a, pif);
 	PRIQ_UNLOCK(ifq);
 
+	ifnet_unlock();
+
 	return error;
 }