Re: [PATCH] ath9k: hif_usb: fix memory leak in ath9k_hif_usb_firmware_cb
From: Julian Calaby <hidden>
Date: 2021-07-27 07:25:01
Also in:
lkml, netdev
Hi Dongliang, (Drive-by review, I know almost nothing about the code in question) On Fri, Jul 9, 2021 at 6:47 PM Dongliang Mu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The commit 03fb92a432ea ("ath9k: hif_usb: fix race condition between usb_get_urb() and usb_kill_anchored_urbs()") adds three usb_get_urb in ath9k_hif_usb_dealloc_tx_urbs and usb_free_urb. Fix this bug by adding corresponding usb_free_urb in ath9k_hif_usb_dealloc_tx_urbs other and hif_usb_stop. Reported-by: syzbot+6692c72009680f7c4eb2@syzkaller.appspotmail.com Fixes: 03fb92a432ea ("ath9k: hif_usb: fix race condition between usb_get_urb() and usb_kill_anchored_urbs()") Signed-off-by: Dongliang Mu <redacted> --- drivers/net/wireless/ath/ath9k/hif_usb.c | 3 +++ 1 file changed, 3 insertions(+)diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c index 860da13bfb6a..bda91ff3289b 100644 --- a/drivers/net/wireless/ath/ath9k/hif_usb.c +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c@@ -457,6 +457,7 @@ static void hif_usb_stop(void *hif_handle) usb_kill_urb(tx_buf->urb); list_del(&tx_buf->list); usb_free_urb(tx_buf->urb); + usb_free_urb(tx_buf->urb);
Ok, so if I'm reading this correctly, before the first usb_free_urb() call, we have two references to the urb at tx_buf->urb. Why? Isn't the better fix here to detangle why there's more than one reference to it and resolve it that way? This looks like a hack to fix something much more fundamentally broken. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/