[PATCH v1] wifi: mt76: mt792x: fix memory leak in USB TX path

[PATCH v1] wifi: mt76: mt792x: fix memory leak in USB TX path

[Eason Lai]

When tx_prepare_skb() returns an error in the USB TX path, the
skb is not freed, leading to a memory leak. This can occur when
zero-length frames (such as WNM NULL frames) are dropped to prevent
potential hardware TX hangs.

Fix this by properly releasing the skb with ieee80211_tx_status_ext()
when tx_prepare_skb() fails.

Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
Signed-off-by: Eason Lai <eason.lai@mediatek.com>
---
 drivers/net/wireless/mediatek/mt76/usb.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index a9af3aa6b80a..e266509a7873 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -893,6 +893,9 @@ mt76u_tx_queue_skb(struct mt76_phy *phy, struct mt76_queue *q,
 		   enum mt76_txq_id qid, struct sk_buff *skb,
 		   struct mt76_wcid *wcid, struct ieee80211_sta *sta)
 {
+	struct ieee80211_tx_status status = {
+		.sta = sta,
+	};
 	struct mt76_tx_info tx_info = {
 		.skb = skb,
 	};
@@ -906,11 +909,11 @@ mt76u_tx_queue_skb(struct mt76_phy *phy, struct mt76_queue *q,
 	skb->prev = skb->next = NULL;
 	err = dev->drv->tx_prepare_skb(dev, NULL, qid, wcid, sta, &tx_info);
 	if (err < 0)
-		return err;
+		goto err_free_skb;
 
 	err = mt76u_tx_setup_buffers(dev, tx_info.skb, q->entry[idx].urb);
 	if (err < 0)
-		return err;
+		goto err_free_skb;
 
 	mt76u_fill_bulk_urb(dev, USB_DIR_OUT, q->ep, q->entry[idx].urb,
 			    mt76u_complete_tx, &q->entry[idx]);
@@ -921,6 +924,13 @@ mt76u_tx_queue_skb(struct mt76_phy *phy, struct mt76_queue *q,
 	q->queued++;
 
 	return idx;
+
+err_free_skb:
+	status.skb = tx_info.skb;
+	spin_lock_bh(&dev->rx_lock);
+	ieee80211_tx_status_ext(dev->hw, &status);
+	spin_unlock_bh(&dev->rx_lock);
+	return err;
 }
 
 static void mt76u_tx_kick(struct mt76_dev *dev, struct mt76_queue *q)
-- 
2.45.2

Re: [PATCH v1] wifi: mt76: mt792x: fix memory leak in USB TX path

[Devin Wittmayer]

> 	err = mt76u_tx_setup_buffers(dev, tx_info.skb, q->entry[idx].urb);
> 	if (err < 0)
> -		return err;
> +		goto err_free_skb;

The first hunk looks right, that is what mt76s_tx_queue_skb already does.

The second one I think is a problem. By then tx_prepare_skb has succeeded, so
the skb can already be in wcid->pktid, and nothing takes it out before the
free. mt76_tx_status_skb_get walks that idr on every tx status event and reads
the cb off each entry, so this looks like it turns the leak into a
use-after-free. mt7925 asks for tx status every quarter second per station, so
there is usually an entry there.

mt76_tx_complete_skb would cover it instead. It marks the skb DMA_DONE and
leaves it in the idr for the existing timeout to reap.

Re: [PATCH v1] wifi: mt76: mt792x: fix memory leak in USB TX path

[Eason Lai (賴易聖)]

On Thu, 2026-08-06 at 15:29 -0700, Devin Wittmayer wrote:
> The second one I think is a problem. By then tx_prepare_skb has
> succeeded, so
> the skb can already be in wcid->pktid, and nothing takes it out
> before the
> free. mt76_tx_status_skb_get walks that idr on every tx status event
> and reads
> the cb off each entry, so this looks like it turns the leak into a
> use-after-free. mt7925 asks for tx status every quarter second per
> station, so
> there is usually an entry there.
> 
> mt76_tx_complete_skb would cover it instead. It marks the skb
> DMA_DONE and
> leaves it in the idr for the existing timeout to reap.

Yes, you are right. The second hunk will cause a use-after-free.
I will use mt76_tx_complete_skb() instead of freeing the SKB directly
in v2.

Thanks,
Eason

Re: [PATCH v1] wifi: mt76: mt792x: fix memory leak in USB TX path

[Devin Wittmayer]

Following up on the first hunk. Looking at that path again, there is a failure
behind it I did not account for on 08-06.

mt76_skb_adjust_pad() calls skb_pad(), which frees the skb on failure, and
today mt76u_tx_queue_skb() correctly returns without touching it. Routing that
exit to err_free_skb: frees it a second time. Special casing does not help,
since skb_cow_head() on the same exit also returns -ENOMEM and is safe.

The prerequisite, in mt76_skb_adjust_pad():

	-	if (skb_pad(last, pad))
	+	if (__skb_pad(last, pad, false))

It reaches mt7925 and SDIO too, so probably its own patch ahead of yours.

Worth flagging as well: mt76s_tx_queue_skb is not a model here, it has the same
two exits and the same missing free.

I have mt7921u here if you want a v2 tested.

Devin

Re: [PATCH v1] wifi: mt76: mt792x: fix memory leak in USB TX path

[Eason Lai (賴易聖)]

On Sun, 2026-08-16 at 18:41 -0700, Devin Wittmayer wrote:
> 
> mt76_skb_adjust_pad() calls skb_pad(), which frees the skb on
> failure, and
> today mt76u_tx_queue_skb() correctly returns without touching it.
> Routing that
> exit to err_free_skb: frees it a second time. Special casing does not
> help,
> since skb_cow_head() on the same exit also returns -ENOMEM and is
> safe.
> 
> The prerequisite, in mt76_skb_adjust_pad():
> 
>         -       if (skb_pad(last, pad))
>         +       if (__skb_pad(last, pad, false))
> 
> It reaches mt7925 and SDIO too, so probably its own patch ahead of
> yours.
> 

I have submitted another commit to address the UAF issue in
mt76_skb_adjust_pad()
https://patchwork.kernel.org/project/linux-wireless/patch/20260817090029.167552-1-eason.lai@mediatek.com/

> Worth flagging as well: mt76s_tx_queue_skb is not a model here, it
> has the same
> two exits and the same missing free.
> 
> I have mt7921u here if you want a v2 tested.
> 
> Devin

If you have time, please help test the v2 on mt7921u.
https://patchwork.kernel.org/project/linux-wireless/patch/20260817090157.168089-1-eason.lai@mediatek.com/

Thanks,
Eason