[PATCH] wifi: nxpwifi: detach sync command buffer on interrupted wait

Linmao Li <[email protected]>
Newsgroups org.kernel.vger.linux-wireless,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
nxpwifi synchronous commands keep the caller-provided data buffer in
cmd_node->data_buf. Several callers pass stack-allocated objects there,
for example nxpwifi_get_chan_type() and the timeshare_coex debugfs
handlers.

If wait_event_interruptible_timeout() is interrupted or times out, the
caller can return and release that stack object while the command is still
current. nxpwifi_cancel_all_pending_cmd() deliberately keeps the current
command because a response may still arrive. A late firmware response can
then write through cmd_node->data_buf into the stale stack address.

After cancelling pending commands, detach the caller-owned buffer from the
still-current command under nxpwifi_cmd_lock.

Unlike the host command response path, several command response callbacks
do not tolerate a NULL data buffer. Most of them ignore it or check it
already, but nxpwifi_ret_sta_get_chan_info(),
nxpwifi_ret_sta_hs_wakeup_reason() and nxpwifi_ret_sta_robust_coex()
dereference it unconditionally, so let them discard a detached response.
No caller passes a NULL buffer to these commands today, so this only
affects the newly introduced detached state.

nxpwifi was derived from mwifiex before commit ef06882c7d8a ("wifi:
mwifiex: Detach sync cmd buffer on interrupted wait") and retains the same
lifetime bug. Apply the equivalent buffer detachment here.

Fixes: 73b01e57ed3e ("wifi: nxp: add nxpwifi driver for IW61x")
Signed-off-by: Linmao Li <[email protected]>
---
 drivers/net/wireless/nxp/nxpwifi/sta_cfg.c | 12 ++++++++++++
 drivers/net/wireless/nxp/nxpwifi/sta_cmd.c | 10 ++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/wireless/nxp/nxpwifi/sta_cfg.c b/drivers/net/wireless/nxp/nxpwifi/sta_cfg.c
index 502c96dc40163..702fa1531da1e 100644
--- a/drivers/net/wireless/nxp/nxpwifi/sta_cfg.c
+++ b/drivers/net/wireless/nxp/nxpwifi/sta_cfg.c
@@ -45,6 +45,18 @@ int nxpwifi_wait_queue_complete(struct nxpwifi_adapter *adapter,
 		nxpwifi_dbg(adapter, ERROR, "cmd_wait_q terminated: %d\n",
 			    status);
 		nxpwifi_cancel_all_pending_cmd(adapter);
+
+		/*
+		 * The response path writes through cmd_node->data_buf. The
+		 * caller can release a stack-allocated data_buf after an
+		 * interrupted wait while a late response is still pending.
+		 * Detach it from the current command before returning.
+		 */
+		spin_lock_bh(&adapter->nxpwifi_cmd_lock);
+		if (adapter->curr_cmd == cmd_queued)
+			adapter->curr_cmd->data_buf = NULL;
+		spin_unlock_bh(&adapter->nxpwifi_cmd_lock);
+
 		return status;
 	}
 
diff --git a/drivers/net/wireless/nxp/nxpwifi/sta_cmd.c b/drivers/net/wireless/nxp/nxpwifi/sta_cmd.c
index 0b140e84916f3..5e8ffd306b31f 100644
--- a/drivers/net/wireless/nxp/nxpwifi/sta_cmd.c
+++ b/drivers/net/wireless/nxp/nxpwifi/sta_cmd.c
@@ -2133,6 +2133,9 @@ nxpwifi_ret_sta_robust_coex(struct nxpwifi_private *priv,
 	u16 action = le16_to_cpu(coex->action);
 	u32 mode;
 
+	if (!is_timeshare)
+		return 0;
+
 	coex_tlv = (struct nxpwifi_ie_types_robust_coex
 		    *)((u8 *)coex + sizeof(struct host_cmd_ds_robust_coex));
 	if (action == HOST_ACT_GEN_GET) {
@@ -2679,6 +2682,10 @@ nxpwifi_ret_sta_hs_wakeup_reason(struct nxpwifi_private *priv,
 {
 	struct host_cmd_ds_wakeup_reason *wakeup_reason =
 		(struct host_cmd_ds_wakeup_reason *)data_buf;
+
+	if (!wakeup_reason)
+		return 0;
+
 	wakeup_reason->wakeup_reason =
 		resp->params.hs_wakeup_reason.wakeup_reason;
 
@@ -2771,6 +2778,9 @@ nxpwifi_ret_sta_get_chan_info(struct nxpwifi_private *priv,
 		(struct nxpwifi_channel_band *)data_buf;
 	struct host_cmd_tlv_channel_band *tlv_band_channel;
 
+	if (!channel_band)
+		return 0;
+
 	tlv_band_channel =
 	(struct host_cmd_tlv_channel_band *)sta_cfg_cmd->tlv_buffer;
 	memcpy(&channel_band->band_config, &tlv_band_channel->band_config,

base-commit: 3652b49adac266a3d27cb41cdfdb7d8790fc3633
-- 
2.25.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.