Messages in this thread | | | From | Julian Calaby <> | Date | Tue, 27 Jul 2021 17:24:44 +1000 | Subject | Re: [PATCH] ath9k: hif_usb: fix memory leak in ath9k_hif_usb_firmware_cb |
| |
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 <mudongliangabcd@gmail.com> wrote: > > 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 <mudongliangabcd@gmail.com> > --- > 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/
| |