Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 19 Apr 2007 13:13:14 GMT
From:      Sepherosa Ziehau <sephe@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 118409 for review
Message-ID:  <200704191313.l3JDDEZ6005772@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=118409

Change 118409 by sephe@sephe_zealot:sam_wifi on 2007/04/19 13:12:13

	- Fix mbuf/node leakage in drivers' raw_xmit().
	- For ural(4):
	  o  Fix node leakage in ural_start(), if ural_tx_mgt() fails.
	  o  Fix mbuf leakage in ural_tx_{mgt,data}(), if usbd_transfer()
	     fails.
	  o  In ural_tx_{mgt,data}(), set ural_tx_data.{m,ni} to NULL,
	     if usbd_transfer() fails, so they will not be freed again
	     in ural_stop().
	
	Approved by:	sam (mentor)

Affected files ...

.. //depot/projects/wifi/sys/dev/ath/if_ath.c#137 edit
.. //depot/projects/wifi/sys/dev/ral/rt2560.c#18 edit
.. //depot/projects/wifi/sys/dev/usb/if_ural.c#18 edit
.. //depot/projects/wifi/sys/dev/wi/if_wi.c#32 edit

Differences ...

==== //depot/projects/wifi/sys/dev/ath/if_ath.c#137 (text+ko) ====

@@ -6498,6 +6498,7 @@
 	struct ath_buf *bf;
 
 	if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0 || sc->sc_invalid) {
+		ieee80211_free_node(ni);
 		m_freem(m);
 		return ENETDOWN;
 	}
@@ -6514,6 +6515,7 @@
 			__func__);
 		sc->sc_stats.ast_tx_qstop++;
 		ifp->if_drv_flags |= IFF_DRV_OACTIVE;
+		ieee80211_free_node(ni);
 		m_freem(m);
 		return ENOBUFS;
 	}

==== //depot/projects/wifi/sys/dev/ral/rt2560.c#18 (text) ====

@@ -1681,8 +1681,10 @@
 
 	rate = params->ibp_rate0 & IEEE80211_RATE_VAL;
 	/* XXX validate */
-	if (rate == 0)
+	if (rate == 0) {
+		m_freem(m0);
 		return EINVAL;
+	}
 
 	error = bus_dmamap_load_mbuf_sg(sc->prioq.data_dmat, data->map, m0,
 	    segs, &nsegs, 0);
@@ -2823,11 +2825,15 @@
 	/* prevent management frames from being sent if we're not ready */
 	if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
 		RAL_UNLOCK(sc);
+		m_freem(m);
+		ieee80211_free_node(ni);
 		return ENETDOWN;
 	}
 	if (sc->prioq.queued >= RT2560_PRIO_RING_COUNT) {
 		ifp->if_drv_flags |= IFF_DRV_OACTIVE;
 		RAL_UNLOCK(sc);
+		m_freem(m);
+		ieee80211_free_node(ni);
 		return ENOBUFS;		/* XXX */
 	}
 

==== //depot/projects/wifi/sys/dev/usb/if_ural.c#18 (text+ko) ====

@@ -1246,8 +1246,12 @@
 	    ural_txeof);
 
 	error = usbd_transfer(data->xfer);
-	if (error != USBD_NORMAL_COMPLETION && error != USBD_IN_PROGRESS)
+	if (error != USBD_NORMAL_COMPLETION && error != USBD_IN_PROGRESS) {
+		m_freem(m0);
+		data->m = NULL;
+		data->ni = NULL;
 		return error;
+	}
 
 	sc->tx_queued++;
 
@@ -1270,8 +1274,10 @@
 
 	rate = params->ibp_rate0 & IEEE80211_RATE_VAL;
 	/* XXX validate */
-	if (rate == 0)
+	if (rate == 0) {
+		m_freem(m0);
 		return EINVAL;
+	}
 
 	if (bpf_peers_present(sc->sc_drvbpf)) {
 		struct ural_tx_radiotap_header *tap = &sc->sc_txtap;
@@ -1314,8 +1320,12 @@
 	    ural_txeof);
 
 	error = usbd_transfer(data->xfer);
-	if (error != USBD_NORMAL_COMPLETION && error != USBD_IN_PROGRESS)
+	if (error != USBD_NORMAL_COMPLETION && error != USBD_IN_PROGRESS) {
+		m_freem(m0);
+		data->m = NULL;
+		data->ni = NULL;
 		return error;
+	}
 
 	sc->tx_queued++;
 
@@ -1435,9 +1445,10 @@
 			if (bpf_peers_present(ic->ic_rawbpf))
 				bpf_mtap(ic->ic_rawbpf, m0);
 
-			if (ural_tx_mgt(sc, m0, ni) != 0)
+			if (ural_tx_mgt(sc, m0, ni) != 0) {
+				ieee80211_free_node(ni);
 				break;
-
+			}
 		} else {
 			if (ic->ic_state != IEEE80211_S_RUN)
 				break;
@@ -2381,10 +2392,16 @@
 	struct ural_softc *sc = ifp->if_softc;
 
 	/* prevent management frames from being sent if we're not ready */
-	if (!(ifp->if_drv_flags & IFF_DRV_RUNNING))
+	if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
+		m_freem(m);
+		ieee80211_free_node(ni);
 		return ENETDOWN;
+	}
+
 	if (sc->tx_queued >= RAL_TX_LIST_COUNT) {
 		ifp->if_drv_flags |= IFF_DRV_OACTIVE;
+		m_freem(m);
+		ieee80211_free_node(ni);
 		return EIO;
 	}
 

==== //depot/projects/wifi/sys/dev/wi/if_wi.c#32 (text+ko) ====

@@ -1116,9 +1116,6 @@
 
 			k = ieee80211_crypto_encap(ic, ni, m0);
 			if (k == NULL) {
-				if (ni != NULL)
-					ieee80211_free_node(ni);
-				m_freem(m0);
 				rc = ENOMEM;
 				goto out;
 			}
@@ -1139,16 +1136,20 @@
 	frmhdr.wi_dat_len = htole16(m0->m_pkthdr.len);
 	if (IFF_DUMPPKTS(ifp))
 		wi_dump_pkt(&frmhdr, NULL, -1);
-	if (ni != NULL)
-		ieee80211_free_node(ni);
-	rc = wi_start_tx(ifp, &frmhdr, m0);
-	if (rc)
+	if (wi_start_tx(ifp, &frmhdr, m0) < 0) {
+		m0 = NULL;
+		rc = EIO;
 		goto out;
+	}
+	m0 = NULL;
 
 	sc->sc_txnext = cur = (cur + 1) % sc->sc_ntxbuf;
 out:
 	WI_UNLOCK(sc);
 
+	if (m0 != NULL)
+		m_freem(m0);
+	ieee80211_free_node(ni);
 	return rc;
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200704191313.l3JDDEZ6005772>