From: Brooke Basile <brookebas...@gmail.com>

[ 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+89bd486af9427a9fc...@syzkaller.appspotmail.com
Link: 
https://syzkaller.appspot.com/bug?id=cabffad18eb74197f84871802fd2c5117b61febf
Signed-off-by: Brooke Basile <brookebas...@gmail.com>
Signed-off-by: Kalle Valo <kv...@codeaurora.org>
Link: https://lore.kernel.org/r/20200911071427.32354-1-brookebas...@gmail.com
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c 
b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 3f563e02d17da..2ed98aaed6fb5 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -449,10 +449,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);
 }
@@ -762,27 +771,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);
 }
-- 
2.25.1

Reply via email to