* [PATCH] wifi: ath9k: Fix memory leak in htc_connect_service
@ 2023-05-06 5:26 Takeshi Misawa
2023-05-06 5:53 ` Kalle Valo
2023-05-13 21:12 ` Fedor Pchelkin
0 siblings, 2 replies; 3+ messages in thread
From: Takeshi Misawa @ 2023-05-06 5:26 UTC (permalink / raw)
To: linux-wireless
Cc: Kalle Valo, Sujith, Vasanthakumar Thiagarajan,
Senthil Balasubramanian, John W. Linville
Timeout occurs in htc_connect_service(), then this function returns
without freeing skb.
Fix this by going to err path.
syzbot report:
BUG: memory leak
unreferenced object 0xffff88810a980800 (size 240):
comm "kworker/1:1", pid 24, jiffies 4294947427 (age 16.220s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff83b971c6>] __alloc_skb+0x206/0x270 net/core/skbuff.c:552
[<ffffffff82eb3731>] alloc_skb include/linux/skbuff.h:1270 [inline]
[<ffffffff82eb3731>] htc_connect_service+0x121/0x230 drivers/net/wireless/ath/ath9k/htc_hst.c:259
[<ffffffff82ec03a5>] ath9k_htc_connect_svc drivers/net/wireless/ath/ath9k/htc_drv_init.c:137 [inline]
[<ffffffff82ec03a5>] ath9k_init_htc_services.constprop.0+0xe5/0x390 drivers/net/wireless/ath/ath9k/htc_drv_init.c:157
[<ffffffff82ec0747>] ath9k_htc_probe_device+0xf7/0x8a0 drivers/net/wireless/ath/ath9k/htc_drv_init.c:959
[<ffffffff82eb3ef5>] ath9k_htc_hw_init+0x35/0x60 drivers/net/wireless/ath/ath9k/htc_hst.c:521
[<ffffffff82eb68dd>] ath9k_hif_usb_firmware_cb+0xcd/0x1f0 drivers/net/wireless/ath/ath9k/hif_usb.c:1243
[<ffffffff82aa835b>] request_firmware_work_func+0x4b/0x90 drivers/base/firmware_loader/main.c:1107
[<ffffffff8129a35a>] process_one_work+0x2ba/0x5f0 kernel/workqueue.c:2289
[<ffffffff8129ac7d>] worker_thread+0x5d/0x5b0 kernel/workqueue.c:2436
[<ffffffff812a4fa9>] kthread+0x129/0x170 kernel/kthread.c:376
[<ffffffff81002dcf>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308
Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
Link: https://syzkaller.appspot.com/bug?id=fbf138952d6c1115ba7d797cf7d56f6935184e3f
Reported-and-tested-by: syzbot+b68fbebe56d8362907e8@syzkaller.appspotmail.com
Signed-off-by: Takeshi Misawa <jeantsuru.cumc.mandola@gmail.com>
---
drivers/net/wireless/ath/ath9k/htc_hst.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index ca05b07a45e6..6878da6d15b4 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -285,7 +285,8 @@ int htc_connect_service(struct htc_target *target,
if (!time_left) {
dev_err(target->dev, "Service connection timeout for: %d\n",
service_connreq->service_id);
- return -ETIMEDOUT;
+ ret = -ETIMEDOUT;
+ goto err;
}
*conn_rsp_epid = target->conn_rsp_epid;
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] wifi: ath9k: Fix memory leak in htc_connect_service 2023-05-06 5:26 [PATCH] wifi: ath9k: Fix memory leak in htc_connect_service Takeshi Misawa @ 2023-05-06 5:53 ` Kalle Valo 2023-05-13 21:12 ` Fedor Pchelkin 1 sibling, 0 replies; 3+ messages in thread From: Kalle Valo @ 2023-05-06 5:53 UTC (permalink / raw) To: Takeshi Misawa; +Cc: linux-wireless, John W. Linville Takeshi Misawa <jeantsuru.cumc.mandola@gmail.com> writes: > Timeout occurs in htc_connect_service(), then this function returns > without freeing skb. > > Fix this by going to err path. > > syzbot report: > BUG: memory leak > unreferenced object 0xffff88810a980800 (size 240): > comm "kworker/1:1", pid 24, jiffies 4294947427 (age 16.220s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [<ffffffff83b971c6>] __alloc_skb+0x206/0x270 net/core/skbuff.c:552 > [<ffffffff82eb3731>] alloc_skb include/linux/skbuff.h:1270 [inline] > [<ffffffff82eb3731>] htc_connect_service+0x121/0x230 drivers/net/wireless/ath/ath9k/htc_hst.c:259 > [<ffffffff82ec03a5>] ath9k_htc_connect_svc drivers/net/wireless/ath/ath9k/htc_drv_init.c:137 [inline] > [<ffffffff82ec03a5>] ath9k_init_htc_services.constprop.0+0xe5/0x390 drivers/net/wireless/ath/ath9k/htc_drv_init.c:157 > [<ffffffff82ec0747>] ath9k_htc_probe_device+0xf7/0x8a0 drivers/net/wireless/ath/ath9k/htc_drv_init.c:959 > [<ffffffff82eb3ef5>] ath9k_htc_hw_init+0x35/0x60 drivers/net/wireless/ath/ath9k/htc_hst.c:521 > [<ffffffff82eb68dd>] ath9k_hif_usb_firmware_cb+0xcd/0x1f0 drivers/net/wireless/ath/ath9k/hif_usb.c:1243 > [<ffffffff82aa835b>] request_firmware_work_func+0x4b/0x90 drivers/base/firmware_loader/main.c:1107 > [<ffffffff8129a35a>] process_one_work+0x2ba/0x5f0 kernel/workqueue.c:2289 > [<ffffffff8129ac7d>] worker_thread+0x5d/0x5b0 kernel/workqueue.c:2436 > [<ffffffff812a4fa9>] kthread+0x129/0x170 kernel/kthread.c:376 > [<ffffffff81002dcf>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308 > > Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") > Link: https://syzkaller.appspot.com/bug?id=fbf138952d6c1115ba7d797cf7d56f6935184e3f > Reported-and-tested-by: syzbot+b68fbebe56d8362907e8@syzkaller.appspotmail.com > Signed-off-by: Takeshi Misawa <jeantsuru.cumc.mandola@gmail.com> Then submitting a new version of the patch you should mark it as v2 and include a changelog what changed from v1, more info in the wiki link below. But no need to resubmit the patch because of this, just keep this in mind for future patches. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] wifi: ath9k: Fix memory leak in htc_connect_service 2023-05-06 5:26 [PATCH] wifi: ath9k: Fix memory leak in htc_connect_service Takeshi Misawa 2023-05-06 5:53 ` Kalle Valo @ 2023-05-13 21:12 ` Fedor Pchelkin 1 sibling, 0 replies; 3+ messages in thread From: Fedor Pchelkin @ 2023-05-13 21:12 UTC (permalink / raw) To: Takeshi Misawa, Kalle Valo Cc: linux-wireless, Toke Høiland-Jørgensen, Sujith, Vasanthakumar Thiagarajan, Senthil Balasubramanian, John W. Linville, netdev, linux-kernel, Alexey Khoroshilov, lvc-project Hi, On Sat, May 06, 2023 at 02:26:20PM +0900, Takeshi Misawa wrote: > Timeout occurs in htc_connect_service(), then this function returns > without freeing skb. > > Fix this by going to err path. This will lead to UAF [1]. If a timeout occurs, htc_connect_service() returns with error but it is possible that a belated callback will touch the already freed SKB. It is actually a callback's responsibilty to free the buffer. The callback is ath9k_htc_txcompletion_cb(). As the urb is submitted, callback should be always executed, otherwise there is an error in usb core, not ath9k. So the problem lies in somewhat another place. htc_connect_service() issues service connection requests via control ENDPOINT0. Its pipe_ids are configured in ath9k_htc_hw_alloc(): /* Assign control endpoint pipe IDs */ endpoint = &target->endpoint[ENDPOINT0]; endpoint->ul_pipeid = hif->control_ul_pipe; endpoint->dl_pipeid = hif->control_dl_pipe; ul_pipe is USB_REG_OUT_PIPE and service connection requests should go that way. However, the reproducer managed to issue the WMI_CMD and Beacon requests via USB_REG_OUT_PIPE, but the third one was issued via USB_WLAN_TX_PIPE. As it went on the wrong way, the SKB simply got lost in __hif_usb_tx() as the special conditions weren't satisfied. Somehow the ENDPOINT0 ul_pipeid has unexpectedly changed... Well, it is htc_process_conn_rsp() which received a badly constructed "service connection reply" from a bad USB device: the endpoint which attributes were to be changed pointed to ENDPOINT0 which should never happen with healthy firmware. But ENDPOINT0 pipeid attributes were changed, so the next service conn requests went on the wrong path. The fix seems to be that htc_process_conn_rsp() should immediately return if the received endpoint which is to be connected to a new service is ENDPOINT0. [1]: https://lore.kernel.org/linux-wireless/20200404041838.10426-2-hqjagain@gmail.com/ > > syzbot report: > BUG: memory leak > unreferenced object 0xffff88810a980800 (size 240): > comm "kworker/1:1", pid 24, jiffies 4294947427 (age 16.220s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [<ffffffff83b971c6>] __alloc_skb+0x206/0x270 net/core/skbuff.c:552 > [<ffffffff82eb3731>] alloc_skb include/linux/skbuff.h:1270 [inline] > [<ffffffff82eb3731>] htc_connect_service+0x121/0x230 drivers/net/wireless/ath/ath9k/htc_hst.c:259 > [<ffffffff82ec03a5>] ath9k_htc_connect_svc drivers/net/wireless/ath/ath9k/htc_drv_init.c:137 [inline] > [<ffffffff82ec03a5>] ath9k_init_htc_services.constprop.0+0xe5/0x390 drivers/net/wireless/ath/ath9k/htc_drv_init.c:157 > [<ffffffff82ec0747>] ath9k_htc_probe_device+0xf7/0x8a0 drivers/net/wireless/ath/ath9k/htc_drv_init.c:959 > [<ffffffff82eb3ef5>] ath9k_htc_hw_init+0x35/0x60 drivers/net/wireless/ath/ath9k/htc_hst.c:521 > [<ffffffff82eb68dd>] ath9k_hif_usb_firmware_cb+0xcd/0x1f0 drivers/net/wireless/ath/ath9k/hif_usb.c:1243 > [<ffffffff82aa835b>] request_firmware_work_func+0x4b/0x90 drivers/base/firmware_loader/main.c:1107 > [<ffffffff8129a35a>] process_one_work+0x2ba/0x5f0 kernel/workqueue.c:2289 > [<ffffffff8129ac7d>] worker_thread+0x5d/0x5b0 kernel/workqueue.c:2436 > [<ffffffff812a4fa9>] kthread+0x129/0x170 kernel/kthread.c:376 > [<ffffffff81002dcf>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308 > > Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") > Link: https://syzkaller.appspot.com/bug?id=fbf138952d6c1115ba7d797cf7d56f6935184e3f > Reported-and-tested-by: syzbot+b68fbebe56d8362907e8@syzkaller.appspotmail.com > Signed-off-by: Takeshi Misawa <jeantsuru.cumc.mandola@gmail.com> > --- > drivers/net/wireless/ath/ath9k/htc_hst.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c > index ca05b07a45e6..6878da6d15b4 100644 > --- a/drivers/net/wireless/ath/ath9k/htc_hst.c > +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c > @@ -285,7 +285,8 @@ int htc_connect_service(struct htc_target *target, > if (!time_left) { > dev_err(target->dev, "Service connection timeout for: %d\n", > service_connreq->service_id); > - return -ETIMEDOUT; > + ret = -ETIMEDOUT; > + goto err; > } > > *conn_rsp_epid = target->conn_rsp_epid; > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-13 21:15 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-06 5:26 [PATCH] wifi: ath9k: Fix memory leak in htc_connect_service Takeshi Misawa 2023-05-06 5:53 ` Kalle Valo 2023-05-13 21:12 ` Fedor Pchelkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).