lkml.org 
[lkml]   [2020]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.8 580/633] ath9k: hif_usb: fix race condition between usb_get_urb() and usb_kill_anchored_urbs()
    Date
    From: Brooke Basile <brookebasile@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+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>
    ---
    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


    \
     
     \ /
      Last update: 2020-10-27 17:40    [W:4.109 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site