Re: [PATCH v2 2/2] wifi: rtw88: usb: fix memory leaks on USB write failures

[Luka Gejak]

On May 7, 2026 6:28:27 PM GMT+02:00, luka.gejak@xxxxxxxxx wrote:
>From: Luka Gejak <luka.gejak@xxxxxxxxx>
>
>When rtw_usb_write_port() fails to submit a USB Request Block (URB)
>(e.g., due to device disconnect or ENOMEM), the completion callback is
>never executed.
>
>Currently, the driver ignores the return value of rtw_usb_write_port()
>in rtw_usb_write_data() and rtw_usb_tx_agg_skb(). Because these
>functions rely on the completion callback to free the socket buffers
>(skbs) and the transaction control block (txcb), a submission failure
>results in:
>1. A memory leak of the allocated skb in rtw_usb_write_data().
>2. A memory leak of the txcb structure and all aggregated skbs in
>   rtw_usb_tx_agg_skb().
>
>Fix this by checking the return value of rtw_usb_write_port(). If it
>fails, explicitly free the skb in rtw_usb_write_data(), and properly
>purge the tx_ack_queue and free the txcb in rtw_usb_tx_agg_skb().
>
>The issue was discovered in practice during device disconnect/reconnect
>scenarios and memory pressure conditions. Tested by verifying normal TX
>operation continues after the fix without regressions.
>
>Fixes: 87caeef032fc ("wifi: rtw88: Add rtw8723du chipset support")
>Cc: stable@xxxxxxxxxxxxxxx
>Tested-by: Luka Gejak <luka.gejak@xxxxxxxxx>
>Signed-off-by: Luka Gejak <luka.gejak@xxxxxxxxx>
>---
>Changes in v2:
> - Use ret = rtw_usb_write_port(...); style, and check by next line (in
>   rtw_usb_tx_agg_skb)
> - Remove unnecessary comment
> - Use ieee80211_purge_tx_queue() instead of skb_queue_purge()
> - Add testing details to commit message
>
> drivers/net/wireless/realtek/rtw88/usb.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>index 718940ebba31..1bb922cc2928 100644
>--- a/drivers/net/wireless/realtek/rtw88/usb.c
>+++ b/drivers/net/wireless/realtek/rtw88/usb.c
>@@ -399,6 +399,7 @@ static bool rtw_usb_tx_agg_skb(struct rtw_usb *rtwusb, struct sk_buff_head *list
> 	int agg_num = 0;
> 	unsigned int align_next = 0;
> 	u8 qsel;
>+	int ret;
> 
> 	if (skb_queue_empty(list))
> 		return false;
>@@ -456,7 +457,13 @@ static bool rtw_usb_tx_agg_skb(struct rtw_usb *rtwusb, struct sk_buff_head *list
> 	tx_desc = (struct rtw_tx_desc *)skb_head->data;
> 	qsel = le32_get_bits(tx_desc->w1, RTW_TX_DESC_W1_QSEL);
> 
>-	rtw_usb_write_port(rtwdev, qsel, skb_head, rtw_usb_write_port_tx_complete, txcb);
>+	ret = rtw_usb_write_port(rtwdev, qsel, skb_head,
>+			         rtw_usb_write_port_tx_complete, txcb);
>+	if (ret) {
>+		ieee80211_purge_tx_queue(rtwdev->hw, &txcb->tx_ack_queue);
>+		kfree(txcb);
>+		return false;
>+	}
> 
> 	return true;
> }
>@@ -518,8 +525,10 @@ static int rtw_usb_write_data(struct rtw_dev *rtwdev,
> 
> 	ret = rtw_usb_write_port(rtwdev, qsel, skb,
> 				 rtw_usb_write_port_complete, skb);
>-	if (unlikely(ret))
>+	if (unlikely(ret)) {
> 		rtw_err(rtwdev, "failed to do USB write, ret=%d\n", ret);
>+		dev_kfree_skb_any(skb);
>+	}
> 
> 	return ret;
> }

I accidentally sent this as part of the patch series but it is clearly
not. Resending as a standalone patch. Sorry for the noise.
Best regards,
Luka Gejak