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@xxxxxxxxxxx> wrote:
>luka.gejak@xxxxxxxxx <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.
>
>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@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()
>
>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