ath9k: hif_usb: fix race condition between usb_get_urb() and usb_kill_anchored_urbs()

[ Upstream commit 03fb92a432ea5abe5909bca1455b7e44a9380480 ]

Calls to usb_kill_anchored_urbs() after usb_kill_urb() on multiprocessor
systems create a race condition in which usb_kill_anchored_urbs() deallocates
the URB before the completer callback is called in usb_kill_urb(), resulting
in a use-after-free.
To fix this, add proper lock protection to usb_kill_urb() calls that can
possibly run concurrently with usb_kill_anchored_urbs().

Reported-by: syzbot+89bd486af9427a9fc605@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=cabffad18eb74197f84871802fd2c5117b61febf
Signed-off-by: Brooke Basile <brookebasile@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Link: https://lore.kernel.org/r/20200911071427.32354-1-brookebasile@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index e80d509bc5415e80a1c5c9f377f2b953a261bf03..ce3a7852127404d1a161b1ef6ba9fb9e4873e165 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -447,10 +447,19 @@ static void hif_usb_stop(void *hif_handle)
        spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
 
        /* The pending URBs have to be canceled. */
+       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);
        }
+       spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
 
        usb_kill_anchored_urbs(&hif_dev->mgmt_submitted);
 }
@@ -760,27 +769,37 @@ static void ath9k_hif_usb_dealloc_tx_urbs(struct hif_device_usb *hif_dev)
        struct tx_buf *tx_buf = NULL, *tx_buf_tmp = NULL;
        unsigned long flags;
 
+       spin_lock_irqsave(&hif_dev->tx.tx_lock, flags);
        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);
        }
+       spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
 
        spin_lock_irqsave(&hif_dev->tx.tx_lock, flags);
        hif_dev->tx.flags |= HIF_USB_TX_FLUSH;
        spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
 
+       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);
        }
+       spin_unlock_irqrestore(&hif_dev->tx.tx_lock, flags);
 
        usb_kill_anchored_urbs(&hif_dev->mgmt_submitted);
 }