RE: [PATCH rtw-next 2/6] wifi: rtw89: usb: fix leak in rtw89_usb_write_port()

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

Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> When there is an attempt to write data and RTW89_FLAG_UNPLUGGED is set,
> this means device is disconnected and no urb is submitted.  Return
> appropriate error code to the caller to properly free the allocated
> resources.
> 
> Found by Linux Verification Center (linuxtesting.org).
> 
> Fixes: 2135c28be6a8 ("wifi: rtw89: Add usb.{c,h}")
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
>  drivers/net/wireless/realtek/rtw89/usb.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c
> index 3435599f4740..bc0d5e48d39b 100644
> --- a/drivers/net/wireless/realtek/rtw89/usb.c
> +++ b/drivers/net/wireless/realtek/rtw89/usb.c
> @@ -256,7 +256,7 @@ static int rtw89_usb_write_port(struct rtw89_dev *rtwdev, u8 ch_dma,
>         int ret;
> 
>         if (test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags))
> -               return 0;
> +               return -ENODEV;
> 
>         urb = usb_alloc_urb(0, GFP_ATOMIC);
>         if (!urb)
> @@ -305,8 +305,9 @@ static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch)
>                 ret = rtw89_usb_write_port(rtwdev, txch, skb->data, skb->len,
>                                            txcb);
>                 if (ret) {
> -                       rtw89_err(rtwdev, "write port txch %d failed: %d\n",
> -                                 txch, ret);
> +                       if (ret != -ENODEV)
> +                               rtw89_err(rtwdev, "write port txch %d failed: %d\n",
> +                                         txch, ret);
> 
>                         skb_dequeue(&txcb->tx_ack_queue);

By the way, during I review this function, txcb->tx_ack_queue is a
struct sk_buff_head, how about just struct sk_buff *skb? 
(I might ask Bitterblue Smith about this).

More, since skb here is from mac80211, so ieee80211_free_txskb() would be 
more suitable rather than dev_kfree_skb_any()?

>                         kfree(txcb);
> --
> 2.51.0
>