On Mon, Jun 7, 2021 at 5:58 AM Dongliang Mu <[email protected]> wrote:
>
> Hi Dmitry,
>
> I saw you have tested one patch [1] for "memory leak in
> ath9k_hif_usb_firmware_cb". And it does not trigger any bugs on the
> patched version. This is great. However, I find there are other
> similar code patterns in the same file below. Shall we fix other sites
> as well in the same patch?
Hi Dongliang,
Sure, if there are more bugs, it would be good to fix them.
It's always a good idea to check for similar code around when a bug is fixed.
> static void ath9k_hif_usb_dealloc_tx_urbs(struct hif_device_usb *hif_dev) [2]
>
> list_for_each_entry_safe(tx_buf, tx_buf_tmp,
> &hif_dev->tx.tx_buf, list) {
> usb_get_urb(tx_buf->urb);
> spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
> usb_kill_urb(tx_buf->urb);
> list_del(&tx_buf->list);
> usb_free_urb(tx_buf->urb);
> kfree(tx_buf->buf);
> kfree(tx_buf);
> spin_lock_irqsave(&hif_dev->tx.tx_lock, flags);
> }
>
> list_for_each_entry_safe(tx_buf, tx_buf_tmp,
> &hif_dev->tx.tx_pending, list) {
> usb_get_urb(tx_buf->urb);
> spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
> usb_kill_urb(tx_buf->urb);
> list_del(&tx_buf->list);
> usb_free_urb(tx_buf->urb);
> kfree(tx_buf->buf);
> kfree(tx_buf);
> spin_lock_irqsave(&hif_dev->tx.tx_lock, flags);
> }
>
> static void hif_usb_stop(void *hif_handle) [3]
>
> list_for_each_entry_safe(tx_buf, tx_buf_tmp,
> &hif_dev->tx.tx_pending, list) {
> usb_get_urb(tx_buf->urb);
> spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
> usb_kill_urb(tx_buf->urb);
> list_del(&tx_buf->list);
> usb_free_urb(tx_buf->urb);
> kfree(tx_buf->buf);
> kfree(tx_buf);
> spin_lock_irqsave(&hif_dev->tx.tx_lock, flags);
> }
>
> [1] https://syzkaller.appspot.com/text?tag=Patch&x=14107bbdd00000
> [2] https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/ath/ath9k/hif_usb.c#L769
> [3] https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/ath/ath9k/hif_usb.c#L439
>
> --
> My best regards to you.
>
> No System Is Safe!
> Dongliang Mu