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

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

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

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@vger.kernel.org
Tested-by: Luka Gejak <luka.gejak@linux.dev>
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
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;
 }
-- 
2.54.0

RE: [PATCH v2] wifi: rtw88: usb: fix memory leaks on USB write failures

[Ping-Ke Shih]

luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
> From: Luka Gejak <luka.gejak@linux.dev>
> 
> 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.

Did the memory pressure condition happen? and falls into the cases you are
adding? This is main thing I want to know.

> 
> Fixes: 87caeef032fc ("wifi: rtw88: Add rtw8723du chipset support")

I don't find this commit touching the code related to this patch.

> Cc: stable@vger.kernel.org
> Tested-by: Luka Gejak <luka.gejak@linux.dev>
> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> ---
> 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()

If it falls into the case, you will see some warnings without this change.

Again, I'd like to know if OOM can happen in your test? If not, the test
you are doing will prove nothing, since your changes are executed only if OOM.

>  - Add testing details to commit message
> 

RE: [PATCH v2] wifi: rtw88: usb: fix memory leaks on USB write failures

[Luka Gejak]

On May 8, 2026 5:47:55 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
>luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
>> From: Luka Gejak <luka.gejak@linux.dev>
>> 
>> 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.
>
>Did the memory pressure condition happen? and falls into the cases you are
>adding? This is main thing I want to know.
>
>> 
>> Fixes: 87caeef032fc ("wifi: rtw88: Add rtw8723du chipset support")
>
>I don't find this commit touching the code related to this patch.
>
>> Cc: stable@vger.kernel.org
>> Tested-by: Luka Gejak <luka.gejak@linux.dev>
>> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
>> ---
>> 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()
>
>If it falls into the case, you will see some warnings without this change.
>
>Again, I'd like to know if OOM can happen in your test? If not, the test
>you are doing will prove nothing, since your changes are executed only if OOM.
>
>>  - Add testing details to commit message
>> 
>
While triggering a genuine OOM condition (-ENOMEM) during 
usb_submit_urb is admittedly difficult to force and rare in standard 
environments, my testing primarily relied on device disconnects.
When a USB adapter is abruptly unplugged, rtw_usb_write_port() 
naturally fails to submit the URB 
(returning -ENODEV, -ESHUTDOWN, etc.). When this happens, the USB 
subsystem never executes the completion callback 
(rtw_usb_write_port_tx_complete or rtw_usb_write_port_complete). 
Because the original code ignored the return value of 
rtw_usb_write_port(), it leaked the skb and txcb structures every time
a write was attempted immediately following a disconnect. Checking the
return value catches this exact submission failure and frees the 
structures on the spot.
And should I use commit that introduced USB support for Fixes tag?
Best regards,
Luka Gejak

RE: [PATCH v2] wifi: rtw88: usb: fix memory leaks on USB write failures

[Ping-Ke Shih]

Luka Gejak <luka.gejak@linux.dev> wrote:
> On May 8, 2026 5:47:55 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
> >luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
> >> From: Luka Gejak <luka.gejak@linux.dev>
> >>
> >> 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.
> >
> >Did the memory pressure condition happen? and falls into the cases you are
> >adding? This is main thing I want to know.
> >
> >>
> >> Fixes: 87caeef032fc ("wifi: rtw88: Add rtw8723du chipset support")
> >
> >I don't find this commit touching the code related to this patch.
> >
> >> Cc: stable@vger.kernel.org
> >> Tested-by: Luka Gejak <luka.gejak@linux.dev>
> >> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> >> ---
> >> 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()
> >
> >If it falls into the case, you will see some warnings without this change.
> >
> >Again, I'd like to know if OOM can happen in your test? If not, the test
> >you are doing will prove nothing, since your changes are executed only if OOM.
> >
> >>  - Add testing details to commit message
> >>
> >
> While triggering a genuine OOM condition (-ENOMEM) during
> usb_submit_urb is admittedly difficult to force and rare in standard
> environments, my testing primarily relied on device disconnects.
> When a USB adapter is abruptly unplugged, rtw_usb_write_port()
> naturally fails to submit the URB
> (returning -ENODEV, -ESHUTDOWN, etc.). 

I got the point that you did unplug the device.

> When this happens, the USB
> subsystem never executes the completion callback
> (rtw_usb_write_port_tx_complete or rtw_usb_write_port_complete).
> Because the original code ignored the return value of
> rtw_usb_write_port(), it leaked the skb and txcb structures every time
> a write was attempted immediately following a disconnect. Checking the
> return value catches this exact submission failure and frees the
> structures on the spot.
> And should I use commit that introduced USB support for Fixes tag?

I think it should be.

Ping-Ke