This is a note to let you know that I've just added the patch titled
wifi: ath9k: hif_usb: Fix use-after-free in ath9k_hif_usb_reg_in_cb()
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
wifi-ath9k-hif_usb-fix-use-after-free-in-ath9k_hif_u.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit c6b35c6e01dff660364943f4fed4e0bd6101e967
Author: Fedor Pchelkin <pchelkin@xxxxxxxxx>
Date: Sat Oct 8 14:49:17 2022 +0300
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@xxxxxxxxx>
Signed-off-by: Alexey Khoroshilov <khoroshilov@xxxxxxxxx>
Acked-by: Toke Høiland-Jørgensen <toke@xxxxxxx>
Signed-off-by: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
Link: https://lore.kernel.org/r/20221008114917.21404-1-pchelkin@xxxxxxxxx
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 519cc8fd3299..719cb53d8b4d 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -707,14 +707,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:
@@ -723,7 +722,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);
@@ -734,25 +733,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);
}
@@ -761,12 +762,13 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
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;
}
