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

[Ping-Ke Shih <pkshih@realtek.com>]

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