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@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 03fb92a432ea ("ath9k: hif_usb: fix race condition between usb_get_urb() and usb_kill_anchored_urbs()")
> Signed-off-by: Dongliang Mu <mudongliangabcd@xxxxxxxxx>
> ---
> 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@xxxxxxxxx
Profile: http://www.google.com/profiles/julian.calaby/
