ath10k: fix use-after-free in ath10k_wmi_cmd_send_nowait

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>

diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index e40edced1d82010b040520b8bfdd608d8a17366d..7d2fac34215050276efe3ec49c339b56a90d245d 100644 (file)
--- 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 77c6bc671f323523107ab8fc743434d685eb2753..9649bb752bbda2ae7eb90c8a0ff196f5b6d32a3f 100644 (file)
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1747,8 +1747,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;