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: 3deff76095
("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>
This commit is contained in:
parent
9850791d38
commit
988bd27de2
@ -709,14 +709,13 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
|
|||||||
struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
|
struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
|
||||||
struct hif_device_usb *hif_dev = rx_buf->hif_dev;
|
struct hif_device_usb *hif_dev = rx_buf->hif_dev;
|
||||||
struct sk_buff *skb = rx_buf->skb;
|
struct sk_buff *skb = rx_buf->skb;
|
||||||
struct sk_buff *nskb;
|
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
if (!skb)
|
if (!skb)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
if (!hif_dev)
|
if (!hif_dev)
|
||||||
goto free;
|
goto free_skb;
|
||||||
|
|
||||||
switch (urb->status) {
|
switch (urb->status) {
|
||||||
case 0:
|
case 0:
|
||||||
@ -725,7 +724,7 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
|
|||||||
case -ECONNRESET:
|
case -ECONNRESET:
|
||||||
case -ENODEV:
|
case -ENODEV:
|
||||||
case -ESHUTDOWN:
|
case -ESHUTDOWN:
|
||||||
goto free;
|
goto free_skb;
|
||||||
default:
|
default:
|
||||||
skb_reset_tail_pointer(skb);
|
skb_reset_tail_pointer(skb);
|
||||||
skb_trim(skb, 0);
|
skb_trim(skb, 0);
|
||||||
@ -736,25 +735,27 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
|
|||||||
if (likely(urb->actual_length != 0)) {
|
if (likely(urb->actual_length != 0)) {
|
||||||
skb_put(skb, urb->actual_length);
|
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,
|
ath9k_htc_rx_msg(hif_dev->htc_handle, skb,
|
||||||
skb->len, USB_REG_IN_PIPE);
|
skb->len, USB_REG_IN_PIPE);
|
||||||
|
|
||||||
|
skb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);
|
||||||
nskb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);
|
if (!skb) {
|
||||||
if (!nskb) {
|
|
||||||
dev_err(&hif_dev->udev->dev,
|
dev_err(&hif_dev->udev->dev,
|
||||||
"ath9k_htc: REG_IN memory allocation failure\n");
|
"ath9k_htc: REG_IN memory allocation failure\n");
|
||||||
urb->context = NULL;
|
goto free_rx_buf;
|
||||||
return;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
rx_buf->skb = nskb;
|
rx_buf->skb = skb;
|
||||||
|
|
||||||
usb_fill_int_urb(urb, hif_dev->udev,
|
usb_fill_int_urb(urb, hif_dev->udev,
|
||||||
usb_rcvintpipe(hif_dev->udev,
|
usb_rcvintpipe(hif_dev->udev,
|
||||||
USB_REG_IN_PIPE),
|
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);
|
ath9k_hif_usb_reg_in_cb, rx_buf, 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -763,12 +764,13 @@ resubmit:
|
|||||||
ret = usb_submit_urb(urb, GFP_ATOMIC);
|
ret = usb_submit_urb(urb, GFP_ATOMIC);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
usb_unanchor_urb(urb);
|
usb_unanchor_urb(urb);
|
||||||
goto free;
|
goto free_skb;
|
||||||
}
|
}
|
||||||
|
|
||||||
return;
|
return;
|
||||||
free:
|
free_skb:
|
||||||
kfree_skb(skb);
|
kfree_skb(skb);
|
||||||
|
free_rx_buf:
|
||||||
kfree(rx_buf);
|
kfree(rx_buf);
|
||||||
urb->context = NULL;
|
urb->context = NULL;
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user