Re: KASAN: use-after-free Write in ath9k_htc_rx_msg

[Qiujun Huang <anenbupt@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 58 bytes --]

#syz test: https://github.com/google/kasan.git usb-fuzzer

[-- Attachment #2: 0001-ath9k-fix-use-after-free-read-in-htc_connect_service.patch --]
[-- Type: application/octet-stream, Size: 4880 bytes --]

From 61a77bb21792ba2323299880ac4a07c6fe0b94a0 Mon Sep 17 00:00:00 2001
From: Qiujun Huang <hqjagain@gmail.com>
Date: Tue, 31 Mar 2020 20:18:56 +0800
Subject: [PATCH] ath9k: fix use-after-free read in htc_connect_service

(1)The skb is consumed by htc_send_epid.
(2)free wmi later after urb has been killed.
(3)check the endpoint_id.

Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c      |  4 +++-
 drivers/net/wireless/ath/ath9k/htc_drv_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/htc_hst.c      |  7 ++++---
 drivers/net/wireless/ath/ath9k/wmi.c          | 15 ++++++++++++++-
 drivers/net/wireless/ath/ath9k/wmi.h          |  2 ++
 5 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index dd0c32379375..275607a74841 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -1341,8 +1341,10 @@ static void ath9k_hif_usb_disconnect(struct usb_interface *interface)
 
 	if (hif_dev->flags & HIF_USB_READY) {
 		ath9k_htc_hw_deinit(hif_dev->htc_handle, unplugged);
-		ath9k_htc_hw_free(hif_dev->htc_handle);
 		ath9k_hif_usb_dev_deinit(hif_dev);
+		smp_mb();
+		ath9k_destoy_wmi(hif_dev->htc_handle->drv_priv);
+		ath9k_htc_hw_free(hif_dev->htc_handle);
 	}
 
 	usb_set_intfdata(interface, NULL);
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index d961095ab01f..d1d0ed6e653c 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -982,7 +982,7 @@ void ath9k_htc_disconnect_device(struct htc_target *htc_handle, bool hotunplug)
 			htc_handle->drv_priv->ah->ah_flags |= AH_UNPLUGGED;
 
 		ath9k_deinit_device(htc_handle->drv_priv);
-		ath9k_deinit_wmi(htc_handle->drv_priv);
+		ath9k_stop_wmi(htc_handle->drv_priv);
 		ieee80211_free_hw(htc_handle->drv_priv->hw);
 	}
 }
diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index d091c8ebdcf0..86cf118c9e5f 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -115,6 +115,10 @@ static void htc_process_conn_rsp(struct htc_target *target,
 		epid = svc_rspmsg->endpoint_id;
 		service_id = be16_to_cpu(svc_rspmsg->service_id);
 		max_msglen = be16_to_cpu(svc_rspmsg->max_msg_len);
+
+		if (epid >= ENDPOINT_MAX)
+			return;
+
 		endpoint = &target->endpoint[epid];
 
 		for (tepid = (ENDPOINT_MAX - 1); tepid > ENDPOINT0; tepid--) {
@@ -170,7 +174,6 @@ static int htc_config_pipe_credits(struct htc_target *target)
 	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
 	if (!time_left) {
 		dev_err(target->dev, "HTC credit config timeout\n");
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
@@ -206,7 +209,6 @@ static int htc_setup_complete(struct htc_target *target)
 	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
 	if (!time_left) {
 		dev_err(target->dev, "HTC start timeout\n");
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
@@ -279,7 +281,6 @@ 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);
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
index cdc146091194..615e6e32f687 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -123,6 +123,20 @@ void ath9k_deinit_wmi(struct ath9k_htc_priv *priv)
 	kfree(priv->wmi);
 }
 
+void ath9k_stop_wmi(struct ath9k_htc_priv *priv)
+{
+	struct wmi *wmi = priv->wmi;
+	
+	mutex_lock(&wmi->op_mutex);
+	wmi->stopped = true;
+	mutex_unlock(&wmi->op_mutex);
+}
+
+void ath9k_destoy_wmi(struct ath9k_htc_priv *priv)
+{
+	kfree(priv->wmi);
+}
+
 void ath9k_wmi_event_drain(struct ath9k_htc_priv *priv)
 {
 	unsigned long flags;
@@ -336,7 +350,6 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 		ath_dbg(common, WMI, "Timeout waiting for WMI command: %s\n",
 			wmi_cmd_to_name(cmd_id));
 		mutex_unlock(&wmi->op_mutex);
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
diff --git a/drivers/net/wireless/ath/ath9k/wmi.h b/drivers/net/wireless/ath/ath9k/wmi.h
index 380175d5ecd7..c3e278377365 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.h
+++ b/drivers/net/wireless/ath/ath9k/wmi.h
@@ -189,6 +189,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 void ath9k_wmi_event_tasklet(unsigned long data);
 void ath9k_fatal_work(struct work_struct *work);
 void ath9k_wmi_event_drain(struct ath9k_htc_priv *priv);
+void ath9k_stop_wmi(struct ath9k_htc_priv *priv);
+void ath9k_destoy_wmi(struct ath9k_htc_priv *priv);
 
 #define WMI_CMD(_wmi_cmd)						\
 	do {								\
-- 
2.17.1