⬢ DragonFlyBSD Kernel Audit
DF-3638 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/netif/ath/ath/if_ath_tx.c
+++ b/sys/dev/netif/ath/ath/if_ath_tx.c
@@ -4566,16 +4566,26 @@ ath_tx_retry_clone(struct ath_softc *sc, struct ath_node *an,
 	/* Setup the dma for the new buffer */
 	error = ath_tx_dmasetup(sc, nbf, nbf->bf_m);
 	if (error != 0) {
 		DPRINTF(sc, ATH_DEBUG_XMIT,
 		    "%s: failed to setup dma for clone\n",
 		    __func__);
 		/*
+		 * ath_tx_dmasetup() has already freed the mbuf chain on
+		 * failure, and nbf owns the node reference moved over by
+		 * ath_buf_clone().  Drop that reference and detach both
+		 * pointers before recycling the buffer, or the node is
+		 * leaked forever and bf_m dangles at freed memory.
+		 */
+		if (nbf->bf_node != NULL) {
+			ieee80211_node_decref(nbf->bf_node);
+			nbf->bf_node = NULL;
+		}
+		nbf->bf_m = NULL;
+		/*
 		 * Put this at the head of the list, not tail;
 		 * that way it doesn't interfere with the
 		 * busy buffer logic (which uses the tail of
 		 * the list.)
 		 */
 		ATH_TXBUF_LOCK(sc);
 		ath_returnbuf_head(sc, nbf);
--- a/sys/dev/netif/ath/ath/if_ath.c
+++ b/sys/dev/netif/ath/ath/if_ath.c
@@ -3133,6 +3133,12 @@ _ath_getbuf_locked(struct ath_softc *sc, ath_buf_type_t btype)
 	bf->bf_next = NULL;	/* XXX just to make sure */
 	bf->bf_last = NULL;	/* XXX again, just to make sure */
 	bf->bf_comp = NULL;	/* XXX again, just to make sure */
+	/*
+	 * Defense in depth: never hand out a buffer with stale
+	 * mbuf/node pointers left over from a previous owner
+	 * (matches the ath_freebuf() KASSERT contract).
+	 */
+	bf->bf_m = NULL;
+	bf->bf_node = NULL;
 	bzero(&bf->bf_state, sizeof(bf->bf_state));