linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "ath9k: fix ath_tx_process_buffer() potential null ptr dereference"
@ 2021-02-17 21:18 Shuah Khan
  2021-02-18  6:23 ` Kalle Valo
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Shuah Khan @ 2021-02-17 21:18 UTC (permalink / raw)
  To: kvalo, davem, kuba, nbd
  Cc: Shuah Khan, ath9k-devel, linux-wireless, netdev, linux-kernel

ath_tx_process_buffer() doesn't dereference or check sta and passes it
to ath_tx_complete_aggr() and ath_tx_complete_buf().

ath_tx_complete_aggr() checks the pointer before use. No problem here.

ath_tx_complete_buf() doesn't check or dereference sta and passes it on
to ath_tx_complete(). ath_tx_complete() doesn't check or dereference sta,
but assigns it to tx_info->status.status_driver_data[0]

ath_tx_complete_buf() is called from ath_tx_complete_aggr() passing
null ieee80211_sta pointer.

There is a potential for dereference later on, if and when the
tx_info->status.status_driver_data[0]is referenced. In addition, the
rcu read lock might be released before referencing the contents.

ath_tx_complete_buf() should be fixed to check sta perhaps? Worth
looking into.

Reverting this patch because it doesn't solve the problem and introduces
memory leak by skipping buffer completion if the pointer (sta) is NULL.

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 drivers/net/wireless/ath/ath9k/xmit.c | 28 ++++++++++++---------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 735858144e3a..1d36aae3f7b6 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -711,24 +711,20 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
 		ath_tx_count_airtime(sc, sta, bf, ts, tid->tidno);
 		if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
 			tid->clear_ps_filter = true;
+	}
 
-		if (!bf_isampdu(bf)) {
-			if (!flush) {
-				info = IEEE80211_SKB_CB(bf->bf_mpdu);
-				memcpy(info->control.rates, bf->rates,
-				       sizeof(info->control.rates));
-				ath_tx_rc_status(sc, bf, ts, 1,
-						 txok ? 0 : 1, txok);
-				ath_dynack_sample_tx_ts(sc->sc_ah,
-							bf->bf_mpdu, ts, sta);
-			}
-			ath_tx_complete_buf(sc, bf, txq, bf_head, sta,
-					    ts, txok);
-		} else {
-			ath_tx_complete_aggr(sc, txq, bf, bf_head, sta,
-					     tid, ts, txok);
+	if (!bf_isampdu(bf)) {
+		if (!flush) {
+			info = IEEE80211_SKB_CB(bf->bf_mpdu);
+			memcpy(info->control.rates, bf->rates,
+			       sizeof(info->control.rates));
+			ath_tx_rc_status(sc, bf, ts, 1, txok ? 0 : 1, txok);
+			ath_dynack_sample_tx_ts(sc->sc_ah, bf->bf_mpdu, ts,
+						sta);
 		}
-	}
+		ath_tx_complete_buf(sc, bf, txq, bf_head, sta, ts, txok);
+	} else
+		ath_tx_complete_aggr(sc, txq, bf, bf_head, sta, tid, ts, txok);
 
 	if (!flush)
 		ath_txq_schedule(sc, txq);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-22  7:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-17 21:18 [PATCH] Revert "ath9k: fix ath_tx_process_buffer() potential null ptr dereference" Shuah Khan
2021-02-18  6:23 ` Kalle Valo
     [not found] ` <20210218062333.37872C43462@smtp.codeaurora.org>
2021-02-18 14:02   ` Shuah Khan
2021-02-22  7:32 ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).