Date: Wed, 1 Oct 2025 02:24:17 GMT From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: f7aad20db592 - main - ath: fix ath_buf leak if ath_tx_tag_crypto() returns an error Message-ID: <202510010224.5912OHFM092989@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by adrian: URL: https://cgit.FreeBSD.org/src/commit/?id=f7aad20db59210a4411559d07976a48809b3b0a7 commit f7aad20db59210a4411559d07976a48809b3b0a7 Author: Adrian Chadd <adrian@FreeBSD.org> AuthorDate: 2025-10-01 02:21:01 +0000 Commit: Adrian Chadd <adrian@FreeBSD.org> CommitDate: 2025-10-01 02:21:01 +0000 ath: fix ath_buf leak if ath_tx_tag_crypto() returns an error If ath_tx_tag_crypto() returns an error, then ath_tx_normal_setup() should consume the mbuf and return an error, so the caller knows to free the ath_buf. But it wasn't. This fixes issues I've seen locally where a an AP VAP constantly hits error conditions (due to other RF/PHY/MAC chipset issues which I haven't yet figured out) and encryption fails because the station goes away whilst something's being queued. Locally tested: * AR9380, AP mode (2G HT20, 5G HT20, 5G HT40) --- sys/dev/ath/if_ath_tx.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sys/dev/ath/if_ath_tx.c b/sys/dev/ath/if_ath_tx.c index deadd63c3d18..9ac591c14943 100644 --- a/sys/dev/ath/if_ath_tx.c +++ b/sys/dev/ath/if_ath_tx.c @@ -971,6 +971,12 @@ ath_legacy_xmit_handoff(struct ath_softc *sc, struct ath_txq *txq, ath_tx_handoff_hw(sc, txq, bf); } +/* + * Setup a frame for encryption. + * + * If this fails, then an non-zero error is returned. The mbuf + * must be freed by the caller. + */ static int ath_tx_tag_crypto(struct ath_softc *sc, struct ieee80211_node *ni, struct mbuf *m0, int iswep, int isfrag, int *hdrlen, int *pktlen, @@ -1547,6 +1553,10 @@ ath_tx_xmit_normal(struct ath_softc *sc, struct ath_txq *txq, * * Note that this may cause the mbuf to be reallocated, so * m0 may not be valid. + * + * If there's a problem then the mbuf is freed and an error + * is returned. The ath_buf then needs to be freed by the + * caller. */ static int ath_tx_normal_setup(struct ath_softc *sc, struct ieee80211_node *ni, @@ -2073,9 +2083,8 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni, /* This also sets up the DMA map; crypto; frame parameters, etc */ r = ath_tx_normal_setup(sc, ni, bf, m0, txq); - if (r != 0) - goto done; + return (r); /* At this point m0 could have changed! */ m0 = bf->bf_m; @@ -2132,7 +2141,6 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni, ath_tx_leak_count_update(sc, tid, bf); ath_tx_xmit_normal(sc, txq, bf); #endif -done: return 0; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202510010224.5912OHFM092989>