[PATCH] wifi: ath9k: validate WMI response length before copying

[PATCH] wifi: ath9k: validate WMI response length before copying

[Pengpeng Hou]

ath9k_wmi_rsp_callback() removes the WMI command header from the skb
and then copies wmi->cmd_rsp_len bytes into the waiting response
buffer.  The response length is expected by the waiting command path,
while the skb length comes from the device response.  A short response
can therefore be read past the end of the skb data.

Record malformed short responses in the WMI state and return the error
to the waiting command path after completion.  This avoids both the
out-of-bounds read and the false-success case where callers would
consume stale response data.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/net/wireless/ath/ath9k/wmi.c | 16 ++++++++++++++--
 drivers/net/wireless/ath/ath9k/wmi.h |  1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -206,8 +206,16 @@
 {
 	skb_pull(skb, sizeof(struct wmi_cmd_hdr));
 
-	if (wmi->cmd_rsp_buf != NULL && wmi->cmd_rsp_len != 0)
+	if (wmi->cmd_rsp_buf && wmi->cmd_rsp_len) {
+		if (skb->len < wmi->cmd_rsp_len) {
+			wmi->cmd_rsp_status = -EMSGSIZE;
+			goto complete;
+		}
 		memcpy(wmi->cmd_rsp_buf, skb->data, wmi->cmd_rsp_len);
+	}
+	wmi->cmd_rsp_status = 0;
+
+complete:
 
 	complete(&wmi->cmd_wait);
 }
@@ -300,6 +305,7 @@
 	/* record the rsp buffer and length */
 	wmi->cmd_rsp_buf = rsp_buf;
 	wmi->cmd_rsp_len = rsp_len;
+	wmi->cmd_rsp_status = 0;
 
 	wmi->last_seq_id = wmi->tx_seq_id;
 	spin_unlock_irqrestore(&wmi->wmi_lock, flags);
@@ -356,9 +362,13 @@
 		return -ETIMEDOUT;
 	}
 
+	spin_lock_irqsave(&wmi->wmi_lock, flags);
+	ret = wmi->cmd_rsp_status;
+	spin_unlock_irqrestore(&wmi->wmi_lock, flags);
+
 	mutex_unlock(&wmi->op_mutex);
 
-	return 0;
+	return ret;
 
 out:
 	ath_dbg(common, WMI, "WMI failure for: %s\n", wmi_cmd_to_name(cmd_id));
--- a/drivers/net/wireless/ath/ath9k/wmi.h
+++ b/drivers/net/wireless/ath/ath9k/wmi.h
@@ -157,6 +157,7 @@
 	u16 tx_seq_id;
 	u8 *cmd_rsp_buf;
 	u32 cmd_rsp_len;
+	int cmd_rsp_status;
 	bool stopped;
 
 	struct list_head pending_tx_events;

Re: [PATCH] wifi: ath9k: validate WMI response length before copying

[Toke Høiland-Jørgensen]

Pengpeng Hou <pengpeng@iscas.ac.cn> writes:

> ath9k_wmi_rsp_callback() removes the WMI command header from the skb
> and then copies wmi->cmd_rsp_len bytes into the waiting response
> buffer.  The response length is expected by the waiting command path,
> while the skb length comes from the device response.  A short response
> can therefore be read past the end of the skb data.
>
> Record malformed short responses in the WMI state and return the error
> to the waiting command path after completion.  This avoids both the
> out-of-bounds read and the false-success case where callers would
> consume stale response data.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>