2019-06-21 14:15:50

by Ganapathi Bhat

Subject: [PATCH] mwifiex: ignore processing invalid command response

From: Swati Kushwaha <[email protected]>

Firmware can send invalid command response, the processing of
which can attempt to modify unexpected context and cause issues.
To fix this, driver should check that the command response ID is
same as the one it downloaded, and ignore processing of invalid
response.

Signed-off-by: Swati Kushwaha <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 27 ++++++++++++++++--------
drivers/net/wireless/marvell/mwifiex/main.h | 2 +-
2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 8c35441..133b03d 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -39,10 +39,11 @@
static void
mwifiex_init_cmd_node(struct mwifiex_private *priv,
struct cmd_ctrl_node *cmd_node,
- u32 cmd_oid, void *data_buf, bool sync)
+ u32 cmd_no, void *data_buf, bool sync)
{
cmd_node->priv = priv;
- cmd_node->cmd_oid = cmd_oid;
+ cmd_node->cmd_no = cmd_no;
+
if (sync) {
cmd_node->wait_q_enabled = true;
cmd_node->cmd_wait_q_woken = false;
@@ -92,7 +93,7 @@
mwifiex_clean_cmd_node(struct mwifiex_adapter *adapter,
struct cmd_ctrl_node *cmd_node)
{
- cmd_node->cmd_oid = 0;
+ cmd_node->cmd_no = 0;
cmd_node->cmd_flag = 0;
cmd_node->data_buf = NULL;
cmd_node->wait_q_enabled = false;
@@ -201,6 +202,7 @@ static int mwifiex_dnld_cmd_to_fw(struct mwifiex_private *priv,
}

cmd_code = le16_to_cpu(host_cmd->command);
+ cmd_node->cmd_no = cmd_code;
cmd_size = le16_to_cpu(host_cmd->size);

if (adapter->hw_status == MWIFIEX_HW_STATUS_RESET &&
@@ -621,7 +623,7 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
}

/* Initialize the command node */
- mwifiex_init_cmd_node(priv, cmd_node, cmd_oid, data_buf, sync);
+ mwifiex_init_cmd_node(priv, cmd_node, cmd_no, data_buf, sync);

if (!cmd_node->cmd_skb) {
mwifiex_dbg(adapter, ERROR,
@@ -822,9 +824,6 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter)
uint16_t cmdresp_result;
unsigned long flags;

- /* Now we got response from FW, cancel the command timer */
- del_timer_sync(&adapter->cmd_timer);
-
if (!adapter->curr_cmd || !adapter->curr_cmd->resp_skb) {
resp = (struct host_cmd_ds_command *) adapter->upld_buf;
mwifiex_dbg(adapter, ERROR,
@@ -833,9 +832,20 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter)
return -1;
}

+ resp = (struct host_cmd_ds_command *)adapter->curr_cmd->resp_skb->data;
+ orig_cmdresp_no = le16_to_cpu(resp->command);
+ cmdresp_no = (orig_cmdresp_no & HostCmd_CMD_ID_MASK);
+
+ if (adapter->curr_cmd->cmd_no != cmdresp_no) {
+ mwifiex_dbg(adapter, ERROR,
+ "cmdresp error: cmd=0x%x cmd_resp=0x%x\n",
+ adapter->curr_cmd->cmd_no, cmdresp_no);
+ return -1;
+ }
+ /* Now we got response from FW, cancel the command timer */
+ del_timer_sync(&adapter->cmd_timer);
clear_bit(MWIFIEX_IS_CMD_TIMEDOUT, &adapter->work_flags);

- resp = (struct host_cmd_ds_command *) adapter->curr_cmd->resp_skb->data;
if (adapter->curr_cmd->cmd_flag & CMD_F_HOSTCMD) {
/* Copy original response back to response buffer */
struct mwifiex_ds_misc_cmd *hostcmd;
@@ -849,7 +859,6 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter)
memcpy(hostcmd->cmd, resp, size);
}
}
- orig_cmdresp_no = le16_to_cpu(resp->command);

/* Get BSS number and corresponding priv */
priv = mwifiex_get_priv_by_id(adapter,
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index b025ba1..3e442c7 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -747,7 +747,7 @@ struct mwifiex_bss_prio_tbl {
struct cmd_ctrl_node {
struct list_head list;
struct mwifiex_private *priv;
- u32 cmd_oid;
+ u32 cmd_no;
u32 cmd_flag;
struct sk_buff *cmd_skb;
struct sk_buff *resp_skb;
--
1.7.1

2019-06-25 07:08:11

by Kalle Valo

Subject: Re: [PATCH] mwifiex: ignore processing invalid command response

Ganapathi Bhat <[email protected]> wrote:

> From: Swati Kushwaha <[email protected]>
>
> Firmware can send invalid command response, the processing of
> which can attempt to modify unexpected context and cause issues.
> To fix this, driver should check that the command response ID is
> same as the one it downloaded, and ignore processing of invalid
> response.
>
> Signed-off-by: Swati Kushwaha <[email protected]>
> Signed-off-by: Ganapathi Bhat <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

74f202aaae0a mwifiex: ignore processing invalid command response

--
https://patchwork.kernel.org/patch/11010163/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches