diff mbox series

[for-4.14.y,2/4] ath10k: fix use-after-free in ath10k_wmi_cmd_send_nowait

Message ID 1539163789-32338-2-git-send-email-amit.pundir@linaro.org
State New
Headers show
Series [for-4.14.y,1/4] cgroup/cpuset: remove circular dependency deadlock | expand

Commit Message

Amit Pundir Oct. 10, 2018, 9:29 a.m. UTC 
From: Carl Huang <cjhuang@codeaurora.org>


commit 9ef0f58ed7b4a55da4a64641d538e0d9e46579ac upstream.

The skb may be freed in tx completion context before
trace_ath10k_wmi_cmd is called. This can be easily captured when
KASAN(Kernel Address Sanitizer) is enabled. The fix is to move
trace_ath10k_wmi_cmd before the send operation. As the ret has no
meaning in trace_ath10k_wmi_cmd then, so remove this parameter too.

Signed-off-by: Carl Huang <cjhuang@codeaurora.org>

Tested-by: Brian Norris <briannorris@chromium.org>

Reviewed-by: Brian Norris <briannorris@chromium.org>

Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Signed-off-by: Amit Pundir <amit.pundir@linaro.org>

---
To be applied on 4.9.y and 4.4.y as well.
Build tested on 4.14.74, 4.9.131 and 4.4.159 for ARCH=arm/arm64 allmodconfig.

 drivers/net/wireless/ath/ath10k/trace.h | 12 ++++--------
 drivers/net/wireless/ath/ath10k/wmi.c   |  2 +-
 2 files changed, 5 insertions(+), 9 deletions(-)

-- 
2.7.4

Comments

Greg KH Oct. 11, 2018, 9:25 a.m. UTC | #1 
On Wed, Oct 10, 2018 at 02:59:47PM +0530, Amit Pundir wrote:
> From: Carl Huang <cjhuang@codeaurora.org>

> 

> commit 9ef0f58ed7b4a55da4a64641d538e0d9e46579ac upstream.

> 

> The skb may be freed in tx completion context before

> trace_ath10k_wmi_cmd is called. This can be easily captured when

> KASAN(Kernel Address Sanitizer) is enabled. The fix is to move

> trace_ath10k_wmi_cmd before the send operation. As the ret has no

> meaning in trace_ath10k_wmi_cmd then, so remove this parameter too.

> 

> Signed-off-by: Carl Huang <cjhuang@codeaurora.org>

> Tested-by: Brian Norris <briannorris@chromium.org>

> Reviewed-by: Brian Norris <briannorris@chromium.org>

> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>

> ---

> To be applied on 4.9.y and 4.4.y as well.

> Build tested on 4.14.74, 4.9.131 and 4.4.159 for ARCH=arm/arm64 allmodconfig.


Applied, thanks.

greg k-h
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index e0d00cef0bd8..5b974bb76e6c 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -152,10 +152,9 @@  TRACE_EVENT(ath10k_log_dbg_dump,
 );
 
 TRACE_EVENT(ath10k_wmi_cmd,
-	TP_PROTO(struct ath10k *ar, int id, const void *buf, size_t buf_len,
-		 int ret),
+	TP_PROTO(struct ath10k *ar, int id, const void *buf, size_t buf_len),
 
-	TP_ARGS(ar, id, buf, buf_len, ret),
+	TP_ARGS(ar, id, buf, buf_len),
 
 	TP_STRUCT__entry(
 		__string(device, dev_name(ar->dev))
@@ -163,7 +162,6 @@  TRACE_EVENT(ath10k_wmi_cmd,
 		__field(unsigned int, id)
 		__field(size_t, buf_len)
 		__dynamic_array(u8, buf, buf_len)
-		__field(int, ret)
 	),
 
 	TP_fast_assign(
@@ -171,17 +169,15 @@  TRACE_EVENT(ath10k_wmi_cmd,
 		__assign_str(driver, dev_driver_string(ar->dev));
 		__entry->id = id;
 		__entry->buf_len = buf_len;
-		__entry->ret = ret;
 		memcpy(__get_dynamic_array(buf), buf, buf_len);
 	),
 
 	TP_printk(
-		"%s %s id %d len %zu ret %d",
+		"%s %s id %d len %zu",
 		__get_str(driver),
 		__get_str(device),
 		__entry->id,
-		__entry->buf_len,
-		__entry->ret
+		__entry->buf_len
 	)
 );
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 38a97086708b..2ab5311659ea 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1741,8 +1741,8 @@  int ath10k_wmi_cmd_send_nowait(struct ath10k *ar, struct sk_buff *skb,
 	cmd_hdr->cmd_id = __cpu_to_le32(cmd);
 
 	memset(skb_cb, 0, sizeof(*skb_cb));
+	trace_ath10k_wmi_cmd(ar, cmd_id, skb->data, skb->len);
 	ret = ath10k_htc_send(&ar->htc, ar->wmi.eid, skb);
-	trace_ath10k_wmi_cmd(ar, cmd_id, skb->data, skb->len, ret);
 
 	if (ret)
 		goto err_pull;