wifi: ath9k: hif_usb: Fix use-after-free in ath9k_hif_usb_reg_in_cb()

[ Upstream commit dd95f2239fc846795fc926787c3ae0ca701c9840 ]

It is possible that skb is freed in ath9k_htc_rx_msg(), then
usb_submit_urb() fails and we try to free skb again. It causes
use-after-free bug. Moreover, if alloc_skb() fails, urb->context becomes
NULL but rx_buf is not freed and there can be a memory leak.

The patch removes unnecessary nskb and makes skb processing more clear: it
is supposed that ath9k_htc_rx_msg() either frees old skb or passes its
managing to another callback function.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 3deff76095c4 ("ath9k_htc: Increase URB count for REG_IN pipe")
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20221008114917.21404-1-pchelkin@ispras.ru
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 b6b5ce9b9b68a87e7e991bbf6524842b5017e402..6c653d46136e9c31fe86671bf0eb9ce4d9ea4b59 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -708,14 +708,13 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
        struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
        struct hif_device_usb *hif_dev = rx_buf->hif_dev;
        struct sk_buff *skb = rx_buf->skb;
-       struct sk_buff *nskb;
        int ret;
 
        if (!skb)
                return;
 
        if (!hif_dev)
-               goto free;
+               goto free_skb;
 
        switch (urb->status) {
        case 0:
@@ -724,7 +723,7 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
        case -ECONNRESET:
        case -ENODEV:
        case -ESHUTDOWN:
-               goto free;
+               goto free_skb;
        default:
                skb_reset_tail_pointer(skb);
                skb_trim(skb, 0);
@@ -735,25 +734,27 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
        if (likely(urb->actual_length != 0)) {
                skb_put(skb, urb->actual_length);
 
-               /* Process the command first */
+               /*
+                * Process the command first.
+                * skb is either freed here or passed to be
+                * managed to another callback function.
+                */
                ath9k_htc_rx_msg(hif_dev->htc_handle, skb,
                                 skb->len, USB_REG_IN_PIPE);
 
-
-               nskb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);
-               if (!nskb) {
+               skb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);
+               if (!skb) {
                        dev_err(&hif_dev->udev->dev,
                                "ath9k_htc: REG_IN memory allocation failure\n");
-                       urb->context = NULL;
-                       return;
+                       goto free_rx_buf;
                }
 
-               rx_buf->skb = nskb;
+               rx_buf->skb = skb;
 
                usb_fill_int_urb(urb, hif_dev->udev,
                                 usb_rcvintpipe(hif_dev->udev,
                                                 USB_REG_IN_PIPE),
-                                nskb->data, MAX_REG_IN_BUF_SIZE,
+                                skb->data, MAX_REG_IN_BUF_SIZE,
                                 ath9k_hif_usb_reg_in_cb, rx_buf, 1);
        }
 
@@ -762,12 +763,13 @@ resubmit:
        ret = usb_submit_urb(urb, GFP_ATOMIC);
        if (ret) {
                usb_unanchor_urb(urb);
-               goto free;
+               goto free_skb;
        }
 
        return;
-free:
+free_skb:
        kfree_skb(skb);
+free_rx_buf:
        kfree(rx_buf);
        urb->context = NULL;
 }