[PATCH v3 1/4] mwifiex: remove unnecessary wakeup interrupt number sanity check

[PATCH v3 1/4] mwifiex: remove unnecessary wakeup interrupt number sanity check

[Xinming Hu]

From: Xinming Hu <huxm@marvell.com>

If wakeup interrupt handler is called, we know that the wakeup
interrupt number is valid, there is no need to check it.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Cathy Luo <cluo@marvell.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---
v2: modify description(Dimtry)
v3: same as v2
---
 drivers/net/wireless/marvell/mwifiex/main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 0dfbac8..98fd491 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1513,11 +1513,9 @@ static irqreturn_t mwifiex_irq_wakeup_handler(int irq, void *priv)
 {
 	struct mwifiex_adapter *adapter = priv;
 
-	if (adapter->irq_wakeup >= 0) {
-		dev_dbg(adapter->dev, "%s: wake by wifi", __func__);
-		adapter->wake_by_wifi = true;
-		disable_irq_nosync(irq);
-	}
+	dev_dbg(adapter->dev, "%s: wake by wifi", __func__);
+	adapter->wake_by_wifi = true;
+	disable_irq_nosync(irq);
 
 	/* Notify PM core we are wakeup source */
 	pm_wakeup_event(adapter->dev, 0);
-- 
1.8.1.4

[PATCH v3 2/4] mwifiex: fall back mwifiex_dbg to pr_info when adapter->dev not set

[Xinming Hu]

From: Xinming Hu <huxm@marvell.com>

mwifiex_dbg will do nothing before adapter->dev get assigned. several logs
lost in this case. it can be avoided by fall back to pr_info.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
---
v2: enhance adapter->dev null case.(Brain)
v3: use va_list in pr_info.(Dmitry)
---
 drivers/net/wireless/marvell/mwifiex/main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 98fd491..16cd14e 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1750,7 +1750,7 @@ void _mwifiex_dbg(const struct mwifiex_adapter *adapter, int mask,
 	struct va_format vaf;
 	va_list args;
 
-	if (!adapter->dev || !(adapter->debug_mask & mask))
+	if (!(adapter->debug_mask & mask))
 		return;
 
 	va_start(args, fmt);
@@ -1758,7 +1758,10 @@ void _mwifiex_dbg(const struct mwifiex_adapter *adapter, int mask,
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	dev_info(adapter->dev, "%pV", &vaf);
+	if (adapter->dev)
+		dev_info(adapter->dev, "%pV", &vaf);
+	else
+		pr_info("%pV", &vaf);
 
 	va_end(args);
 }
-- 
1.8.1.4

[PATCH v3 3/4] mwifiex: pcie: correct scratch register name

[Xinming Hu]

From: Xinming Hu <huxm@marvell.com>

This patch correct pcie scratch register name, to keep the same with
chipset side definition.

Signed-off-by: Xinming Hu <huxm@marvell.com>
---
v2: code clean and prepare for next patch 4/4
v3: same as v2
---
 drivers/net/wireless/marvell/mwifiex/pcie.c |  4 ++--
 drivers/net/wireless/marvell/mwifiex/pcie.h | 13 +++++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index f45ab12..a07cb0a 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2494,8 +2494,8 @@ static int mwifiex_pcie_host_to_card(struct mwifiex_adapter *adapter, u8 type,
 	struct pcie_service_card *card = adapter->card;
 	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
 	int pcie_scratch_reg[] = {PCIE_SCRATCH_12_REG,
-				  PCIE_SCRATCH_13_REG,
-				  PCIE_SCRATCH_14_REG};
+				  PCIE_SCRATCH_14_REG,
+				  PCIE_SCRATCH_15_REG};
 
 	if (!p)
 		return 0;
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h b/drivers/net/wireless/marvell/mwifiex/pcie.h
index 00e8ee5..7e2450c 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
@@ -77,8 +77,9 @@
 #define PCIE_SCRATCH_10_REG				0xCE8
 #define PCIE_SCRATCH_11_REG				0xCEC
 #define PCIE_SCRATCH_12_REG				0xCF0
-#define PCIE_SCRATCH_13_REG				0xCF8
-#define PCIE_SCRATCH_14_REG				0xCFC
+#define PCIE_SCRATCH_13_REG				0xCF4
+#define PCIE_SCRATCH_14_REG				0xCF8
+#define PCIE_SCRATCH_15_REG				0xCFC
 #define PCIE_RD_DATA_PTR_Q0_Q1                          0xC08C
 #define PCIE_WR_DATA_PTR_Q0_Q1                          0xC05C
 
@@ -217,8 +218,8 @@ struct mwifiex_pcie_card_reg {
 	.ring_tx_start_ptr = MWIFIEX_BD_FLAG_TX_START_PTR,
 	.pfu_enabled = 1,
 	.sleep_cookie = 0,
-	.fw_dump_ctrl = 0xcf4,
-	.fw_dump_start = 0xcf8,
+	.fw_dump_ctrl = PCIE_SCRATCH_13_REG,
+	.fw_dump_start = PCIE_SCRATCH_14_REG,
 	.fw_dump_end = 0xcff,
 	.fw_dump_host_ready = 0xee,
 	.fw_dump_read_done = 0xfe,
@@ -254,8 +255,8 @@ struct mwifiex_pcie_card_reg {
 	.ring_tx_start_ptr = MWIFIEX_BD_FLAG_TX_START_PTR,
 	.pfu_enabled = 1,
 	.sleep_cookie = 0,
-	.fw_dump_ctrl = 0xcf4,
-	.fw_dump_start = 0xcf8,
+	.fw_dump_ctrl = PCIE_SCRATCH_13_REG,
+	.fw_dump_start = PCIE_SCRATCH_14_REG,
 	.fw_dump_end = 0xcff,
 	.fw_dump_host_ready = 0xcc,
 	.fw_dump_read_done = 0xdd,
-- 
1.8.1.4

[PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Xinming Hu]

From: Xinming Hu <huxm@marvell.com>

A seperate wifi-only firmware was download during pcie function level reset.
It is in fact the tail part of wifi/bt combo firmware. Per Brian's and
Dmitry's suggestion, this patch extract the wifi part from combo firmware.

After that, we can discard the redudant image in linux-firmware repo.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
Signed-off-by: Cathy Luo <cluo@marvell.com>
---
v2: extract wifi part from combo firmware(Dimtry and Brain)
    add more description(Kalle)
v3: same as v2
---
 drivers/net/wireless/marvell/mwifiex/fw.h   | 18 +++++++
 drivers/net/wireless/marvell/mwifiex/pcie.c | 83 ++++++++++++++++++++++++++---
 drivers/net/wireless/marvell/mwifiex/pcie.h |  2 +
 3 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 0b68374..6cf9ab9 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -43,6 +43,24 @@ struct tx_packet_hdr {
 	struct rfc_1042_hdr rfc1042_hdr;
 } __packed;
 
+struct mwifiex_fw_header {
+	__le32 dnld_cmd;
+	__le32 base_addr;
+	__le32 data_length;
+	__le32 crc;
+} __packed;
+
+struct mwifiex_fw_data {
+	struct mwifiex_fw_header header;
+	__le32 seq_num;
+	u8 data[1];
+} __packed;
+
+#define MWIFIEX_FW_DNLD_CMD_1 0x1
+#define MWIFIEX_FW_DNLD_CMD_5 0x5
+#define MWIFIEX_FW_DNLD_CMD_6 0x6
+#define MWIFIEX_FW_DNLD_CMD_7 0x7
+
 #define B_SUPPORTED_RATES               5
 #define G_SUPPORTED_RATES               9
 #define BG_SUPPORTED_RATES              13
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index a07cb0a..ebf00d9 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -1956,6 +1956,63 @@ static int mwifiex_pcie_event_complete(struct mwifiex_adapter *adapter,
 	return ret;
 }
 
+/* Extract wifi part from wifi-bt combo firmware image.
+ */
+
+static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter,
+				   u8 *firmware, u32 firmware_len) {
+	struct mwifiex_fw_data fwdata;
+	u32 offset = 0, data_len, dnld_cmd;
+	int ret = 0;
+	bool cmd7_before = false;
+
+	while (1) {
+		if (offset + sizeof(fwdata.header) >= firmware_len) {
+			mwifiex_dbg(adapter, ERROR,
+				    "extract wifi-only firmware failure!");
+			ret = -1;
+			goto done;
+		}
+
+		memcpy(&fwdata.header, firmware + offset,
+		       sizeof(fwdata.header));
+		dnld_cmd = le32_to_cpu(fwdata.header.dnld_cmd);
+		data_len = le32_to_cpu(fwdata.header.data_length);
+
+		switch (dnld_cmd) {
+		case MWIFIEX_FW_DNLD_CMD_1:
+			if (!cmd7_before) {
+				mwifiex_dbg(adapter, ERROR,
+					    "no cmd7 before cmd1!");
+				ret = -1;
+				goto done;
+			}
+			offset += data_len + sizeof(fwdata.header);
+			break;
+		case MWIFIEX_FW_DNLD_CMD_5:
+			offset += data_len + sizeof(fwdata.header);
+			break;
+		case MWIFIEX_FW_DNLD_CMD_6:
+			offset += data_len + sizeof(fwdata.header);
+			ret = offset;
+			goto done;
+		case MWIFIEX_FW_DNLD_CMD_7:
+			if (!cmd7_before)
+				cmd7_before = true;
+			offset += sizeof(fwdata.header);
+			break;
+		default:
+			mwifiex_dbg(adapter, ERROR, "unknown dnld_cmd %d\n",
+				    dnld_cmd);
+			ret = -1;
+			goto done;
+		}
+	}
+
+done:
+	return ret;
+}
+
 /*
  * This function downloads the firmware to the card.
  *
@@ -1971,7 +2028,7 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
 	u32 firmware_len = fw->fw_len;
 	u32 offset = 0;
 	struct sk_buff *skb;
-	u32 txlen, tx_blocks = 0, tries, len;
+	u32 txlen, tx_blocks = 0, tries, len, val;
 	u32 block_retry_cnt = 0;
 	struct pcie_service_card *card = adapter->card;
 	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
@@ -1998,6 +2055,24 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
 		goto done;
 	}
 
+	ret = mwifiex_read_reg(adapter, PCIE_SCRATCH_13_REG, &val);
+	if (ret) {
+		mwifiex_dbg(adapter, FATAL, "Failed to read scratch register 13\n");
+		goto done;
+	}
+
+	/* PCIE FLR case: extract wifi part from combo firmware*/
+	if (val == MWIFIEX_PCIE_FLR_HAPPENS) {
+		ret = mwifiex_extract_wifi_fw(adapter, firmware, firmware_len);
+		if (ret < 0) {
+			mwifiex_dbg(adapter, ERROR, "Failed to extract wifi fw\n");
+			goto done;
+		}
+		offset = ret;
+		mwifiex_dbg(adapter, MSG,
+			    "info: dnld wifi firmware from %d bytes\n", offset);
+	}
+
 	/* Perform firmware data transfer */
 	do {
 		u32 ireg_intr = 0;
@@ -3060,12 +3135,6 @@ static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
 	struct pci_dev *pdev = card->dev;
 	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
 
-	/* Bluetooth is not on pcie interface. Download Wifi only firmware
-	 * during pcie FLR, so that bluetooth part of firmware which is
-	 * already running doesn't get affected.
-	 */
-	strcpy(adapter->fw_name, PCIE8997_DEFAULT_WIFIFW_NAME);
-
 	/* tx_buf_size might be changed to 3584 by firmware during
 	 * data transfer, we should reset it to default size.
 	 */
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h b/drivers/net/wireless/marvell/mwifiex/pcie.h
index 7e2450c..54aecda 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
@@ -120,6 +120,8 @@
 #define MWIFIEX_SLEEP_COOKIE_SIZE			4
 #define MWIFIEX_MAX_DELAY_COUNT				100
 
+#define MWIFIEX_PCIE_FLR_HAPPENS 0xFEDCBABA
+
 struct mwifiex_pcie_card_reg {
 	u16 cmd_addr_lo;
 	u16 cmd_addr_hi;
-- 
1.8.1.4

Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Brian Norris]

Hi,

On Mon, Apr 10, 2017 at 09:09:34AM +0000, Xinming Hu wrote:
> From: Xinming Hu <huxm@marvell.com>
> 
> A seperate wifi-only firmware was download during pcie function level reset.
> It is in fact the tail part of wifi/bt combo firmware. Per Brian's and
> Dmitry's suggestion, this patch extract the wifi part from combo firmware.
> 
> After that, we can discard the redudant image in linux-firmware repo.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> ---
> v2: extract wifi part from combo firmware(Dimtry and Brain)
>     add more description(Kalle)
> v3: same as v2
> ---
>  drivers/net/wireless/marvell/mwifiex/fw.h   | 18 +++++++
>  drivers/net/wireless/marvell/mwifiex/pcie.c | 83 ++++++++++++++++++++++++++---
>  drivers/net/wireless/marvell/mwifiex/pcie.h |  2 +
>  3 files changed, 96 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> index 0b68374..6cf9ab9 100644
> --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> @@ -43,6 +43,24 @@ struct tx_packet_hdr {
>  	struct rfc_1042_hdr rfc1042_hdr;
>  } __packed;
>  
> +struct mwifiex_fw_header {
> +	__le32 dnld_cmd;
> +	__le32 base_addr;
> +	__le32 data_length;
> +	__le32 crc;
> +} __packed;
> +
> +struct mwifiex_fw_data {
> +	struct mwifiex_fw_header header;
> +	__le32 seq_num;
> +	u8 data[1];
> +} __packed;
> +
> +#define MWIFIEX_FW_DNLD_CMD_1 0x1
> +#define MWIFIEX_FW_DNLD_CMD_5 0x5
> +#define MWIFIEX_FW_DNLD_CMD_6 0x6
> +#define MWIFIEX_FW_DNLD_CMD_7 0x7
> +
>  #define B_SUPPORTED_RATES               5
>  #define G_SUPPORTED_RATES               9
>  #define BG_SUPPORTED_RATES              13
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index a07cb0a..ebf00d9 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -1956,6 +1956,63 @@ static int mwifiex_pcie_event_complete(struct mwifiex_adapter *adapter,
>  	return ret;
>  }
>  
> +/* Extract wifi part from wifi-bt combo firmware image.
> + */
> +
> +static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter,
> +				   u8 *firmware, u32 firmware_len) {

Can you make 'firmware' const? Also, to help below, it might work better
as 'const void *'.

> +	struct mwifiex_fw_data fwdata;
> +	u32 offset = 0, data_len, dnld_cmd;
> +	int ret = 0;
> +	bool cmd7_before = false;
> +
> +	while (1) {
> +		if (offset + sizeof(fwdata.header) >= firmware_len) {
> +			mwifiex_dbg(adapter, ERROR,
> +				    "extract wifi-only firmware failure!");
> +			ret = -1;
> +			goto done;
> +		}
> +
> +		memcpy(&fwdata.header, firmware + offset,
> +		       sizeof(fwdata.header));

Do you actually need to copy this? Can't you just keep a pointer to the
location? e.g.

	const struct mwifiex_fw_data *fwdata;
...
	fwdata = firmware + offset;

> +		dnld_cmd = le32_to_cpu(fwdata.header.dnld_cmd);
> +		data_len = le32_to_cpu(fwdata.header.data_length);
> +
> +		switch (dnld_cmd) {
> +		case MWIFIEX_FW_DNLD_CMD_1:
> +			if (!cmd7_before) {
> +				mwifiex_dbg(adapter, ERROR,
> +					    "no cmd7 before cmd1!");
> +				ret = -1;
> +				goto done;
> +			}
> +			offset += data_len + sizeof(fwdata.header);

Technically, we need an overflow check to make sure that 'data_len'
isn't some bogus value that overflows 'offset'.

> +			break;
> +		case MWIFIEX_FW_DNLD_CMD_5:
> +			offset += data_len + sizeof(fwdata.header);

Same here.

> +			break;
> +		case MWIFIEX_FW_DNLD_CMD_6:

Can we have a comment, either here or above this function, to describe
what this sequence is? Or particularly, what is this terminating
condition? "CMD_6" doesn't really tell me that this is the start of the
Wifi blob.

> +			offset += data_len + sizeof(fwdata.header);

You don't check for overflow here. Check this before returning this
(either here, or in the 'done' label).

> +			ret = offset;
> +			goto done;
> +		case MWIFIEX_FW_DNLD_CMD_7:
> +			if (!cmd7_before)

^^ This 'if' isn't really needed.

> +				cmd7_before = true;
> +			offset += sizeof(fwdata.header);
> +			break;
> +		default:
> +			mwifiex_dbg(adapter, ERROR, "unknown dnld_cmd %d\n",
> +				    dnld_cmd);
> +			ret = -1;
> +			goto done;
> +		}
> +	}
> +
> +done:
> +	return ret;
> +}
> +
>  /*
>   * This function downloads the firmware to the card.
>   *
> @@ -1971,7 +2028,7 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
>  	u32 firmware_len = fw->fw_len;
>  	u32 offset = 0;
>  	struct sk_buff *skb;
> -	u32 txlen, tx_blocks = 0, tries, len;
> +	u32 txlen, tx_blocks = 0, tries, len, val;
>  	u32 block_retry_cnt = 0;
>  	struct pcie_service_card *card = adapter->card;
>  	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
> @@ -1998,6 +2055,24 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
>  		goto done;
>  	}
>  
> +	ret = mwifiex_read_reg(adapter, PCIE_SCRATCH_13_REG, &val);
> +	if (ret) {
> +		mwifiex_dbg(adapter, FATAL, "Failed to read scratch register 13\n");
> +		goto done;
> +	}
> +
> +	/* PCIE FLR case: extract wifi part from combo firmware*/
> +	if (val == MWIFIEX_PCIE_FLR_HAPPENS) {
> +		ret = mwifiex_extract_wifi_fw(adapter, firmware, firmware_len);
> +		if (ret < 0) {
> +			mwifiex_dbg(adapter, ERROR, "Failed to extract wifi fw\n");
> +			goto done;
> +		}
> +		offset = ret;
> +		mwifiex_dbg(adapter, MSG,
> +			    "info: dnld wifi firmware from %d bytes\n", offset);
> +	}
> +
>  	/* Perform firmware data transfer */
>  	do {
>  		u32 ireg_intr = 0;
> @@ -3060,12 +3135,6 @@ static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
>  	struct pci_dev *pdev = card->dev;
>  	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
>  
> -	/* Bluetooth is not on pcie interface. Download Wifi only firmware
> -	 * during pcie FLR, so that bluetooth part of firmware which is
> -	 * already running doesn't get affected.
> -	 */
> -	strcpy(adapter->fw_name, PCIE8997_DEFAULT_WIFIFW_NAME);

Now that there's no users, delete PCIE8997_DEFAULT_WIFIFW_NAME from
pcie.h.

Brian

> -
>  	/* tx_buf_size might be changed to 3584 by firmware during
>  	 * data transfer, we should reset it to default size.
>  	 */
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h b/drivers/net/wireless/marvell/mwifiex/pcie.h
> index 7e2450c..54aecda 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.h
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
> @@ -120,6 +120,8 @@
>  #define MWIFIEX_SLEEP_COOKIE_SIZE			4
>  #define MWIFIEX_MAX_DELAY_COUNT				100
>  
> +#define MWIFIEX_PCIE_FLR_HAPPENS 0xFEDCBABA
> +
>  struct mwifiex_pcie_card_reg {
>  	u16 cmd_addr_lo;
>  	u16 cmd_addr_hi;
> -- 
> 1.8.1.4
> 

Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Brian Norris]

Hi,

One more thing, since you need to update this patchset anyway:

On Mon, Apr 10, 2017 at 09:09:34AM +0000, Xinming Hu wrote:
> From: Xinming Hu <huxm@marvell.com>
> 
> A seperate wifi-only firmware was download during pcie function level reset.

You might consider running checkpatch.pl on your submissions. It's
helpful in general, but in this case, it would actually catch a spelling
error:

s/seperate/separate/

> It is in fact the tail part of wifi/bt combo firmware. Per Brian's and
> Dmitry's suggestion, this patch extract the wifi part from combo firmware.
> 
> After that, we can discard the redudant image in linux-firmware repo.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Cathy Luo <cluo@marvell.com>

Brian

RE: Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Xinming Hu]

SGkgQnJhaW4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQnJpYW4g
Tm9ycmlzIFttYWlsdG86YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnXQ0KPiBTZW50OiAyMDE3xOo0
1MIxMcjVIDk6MzcNCj4gVG86IFhpbm1pbmcgSHUNCj4gQ2M6IExpbnV4IFdpcmVsZXNzOyBLYWxs
ZSBWYWxvOyBEbWl0cnkgVG9yb2tob3Y7IHJhamF0amFAZ29vZ2xlLmNvbTsNCj4gQW1pdGt1bWFy
IEthcndhcjsgQ2F0aHkgTHVvOyBYaW5taW5nIEh1OyBHYW5hcGF0aGkgQmhhdA0KPiBTdWJqZWN0
OiBbRVhUXSBSZTogW1BBVENIIHYzIDQvNF0gbXdpZmlleDogcGNpZTogZXh0cmFjdCB3aWZpIHBh
cnQgZnJvbSBjb21ibw0KPiBmaXJtd2FyZSBkdXJpbmcgZnVuY3Rpb24gbGV2ZWwgcmVzZXQNCj4g
DQo+IEV4dGVybmFsIEVtYWlsDQo+IA0KPiAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+IEhpLA0KPiANCj4gT24g
TW9uLCBBcHIgMTAsIDIwMTcgYXQgMDk6MDk6MzRBTSArMDAwMCwgWGlubWluZyBIdSB3cm90ZToN
Cj4gPiBGcm9tOiBYaW5taW5nIEh1IDxodXhtQG1hcnZlbGwuY29tPg0KPiA+DQo+ID4gQSBzZXBl
cmF0ZSB3aWZpLW9ubHkgZmlybXdhcmUgd2FzIGRvd25sb2FkIGR1cmluZyBwY2llIGZ1bmN0aW9u
IGxldmVsIHJlc2V0Lg0KPiA+IEl0IGlzIGluIGZhY3QgdGhlIHRhaWwgcGFydCBvZiB3aWZpL2J0
IGNvbWJvIGZpcm13YXJlLiBQZXIgQnJpYW4ncyBhbmQNCj4gPiBEbWl0cnkncyBzdWdnZXN0aW9u
LCB0aGlzIHBhdGNoIGV4dHJhY3QgdGhlIHdpZmkgcGFydCBmcm9tIGNvbWJvIGZpcm13YXJlLg0K
PiA+DQo+ID4gQWZ0ZXIgdGhhdCwgd2UgY2FuIGRpc2NhcmQgdGhlIHJlZHVkYW50IGltYWdlIGlu
IGxpbnV4LWZpcm13YXJlIHJlcG8uDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBYaW5taW5nIEh1
IDxodXhtQG1hcnZlbGwuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6IEdhbmFwYXRoaSBCaGF0IDxn
YmhhdEBtYXJ2ZWxsLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBDYXRoeSBMdW8gPGNsdW9AbWFy
dmVsbC5jb20+DQo+ID4gLS0tDQo+ID4gdjI6IGV4dHJhY3Qgd2lmaSBwYXJ0IGZyb20gY29tYm8g
ZmlybXdhcmUoRGltdHJ5IGFuZCBCcmFpbikNCj4gPiAgICAgYWRkIG1vcmUgZGVzY3JpcHRpb24o
S2FsbGUpDQo+ID4gdjM6IHNhbWUgYXMgdjINCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9uZXQvd2ly
ZWxlc3MvbWFydmVsbC9td2lmaWV4L2Z3LmggICB8IDE4ICsrKysrKysNCj4gPiAgZHJpdmVycy9u
ZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L3BjaWUuYyB8IDgzDQo+ID4gKysrKysrKysrKysr
KysrKysrKysrKysrKystLS0NCj4gPiBkcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxsL213aWZp
ZXgvcGNpZS5oIHwgIDIgKw0KPiA+ICAzIGZpbGVzIGNoYW5nZWQsIDk2IGluc2VydGlvbnMoKyks
IDcgZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxl
c3MvbWFydmVsbC9td2lmaWV4L2Z3LmgNCj4gPiBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZl
bGwvbXdpZmlleC9mdy5oDQo+ID4gaW5kZXggMGI2ODM3NC4uNmNmOWFiOSAxMDA2NDQNCj4gPiAt
LS0gYS9kcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvZncuaA0KPiA+ICsrKyBi
L2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9mdy5oDQo+ID4gQEAgLTQzLDYg
KzQzLDI0IEBAIHN0cnVjdCB0eF9wYWNrZXRfaGRyIHsNCj4gPiAgCXN0cnVjdCByZmNfMTA0Ml9o
ZHIgcmZjMTA0Ml9oZHI7DQo+ID4gIH0gX19wYWNrZWQ7DQo+ID4NCj4gPiArc3RydWN0IG13aWZp
ZXhfZndfaGVhZGVyIHsNCj4gPiArCV9fbGUzMiBkbmxkX2NtZDsNCj4gPiArCV9fbGUzMiBiYXNl
X2FkZHI7DQo+ID4gKwlfX2xlMzIgZGF0YV9sZW5ndGg7DQo+ID4gKwlfX2xlMzIgY3JjOw0KPiA+
ICt9IF9fcGFja2VkOw0KPiA+ICsNCj4gPiArc3RydWN0IG13aWZpZXhfZndfZGF0YSB7DQo+ID4g
KwlzdHJ1Y3QgbXdpZmlleF9md19oZWFkZXIgaGVhZGVyOw0KPiA+ICsJX19sZTMyIHNlcV9udW07
DQo+ID4gKwl1OCBkYXRhWzFdOw0KPiA+ICt9IF9fcGFja2VkOw0KPiA+ICsNCj4gPiArI2RlZmlu
ZSBNV0lGSUVYX0ZXX0ROTERfQ01EXzEgMHgxDQo+ID4gKyNkZWZpbmUgTVdJRklFWF9GV19ETkxE
X0NNRF81IDB4NQ0KPiA+ICsjZGVmaW5lIE1XSUZJRVhfRldfRE5MRF9DTURfNiAweDYNCj4gPiAr
I2RlZmluZSBNV0lGSUVYX0ZXX0ROTERfQ01EXzcgMHg3DQo+ID4gKw0KPiA+ICAjZGVmaW5lIEJf
U1VQUE9SVEVEX1JBVEVTICAgICAgICAgICAgICAgNQ0KPiA+ICAjZGVmaW5lIEdfU1VQUE9SVEVE
X1JBVEVTICAgICAgICAgICAgICAgOQ0KPiA+ICAjZGVmaW5lIEJHX1NVUFBPUlRFRF9SQVRFUyAg
ICAgICAgICAgICAgMTMNCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFy
dmVsbC9td2lmaWV4L3BjaWUuYw0KPiA+IGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9t
d2lmaWV4L3BjaWUuYw0KPiA+IGluZGV4IGEwN2NiMGEuLmViZjAwZDkgMTAwNjQ0DQo+ID4gLS0t
IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L3BjaWUuYw0KPiA+ICsrKyBi
L2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9wY2llLmMNCj4gPiBAQCAtMTk1
Niw2ICsxOTU2LDYzIEBAIHN0YXRpYyBpbnQgbXdpZmlleF9wY2llX2V2ZW50X2NvbXBsZXRlKHN0
cnVjdA0KPiBtd2lmaWV4X2FkYXB0ZXIgKmFkYXB0ZXIsDQo+ID4gIAlyZXR1cm4gcmV0Ow0KPiA+
ICB9DQo+ID4NCj4gPiArLyogRXh0cmFjdCB3aWZpIHBhcnQgZnJvbSB3aWZpLWJ0IGNvbWJvIGZp
cm13YXJlIGltYWdlLg0KPiA+ICsgKi8NCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgbXdpZmlleF9l
eHRyYWN0X3dpZmlfZncoc3RydWN0IG13aWZpZXhfYWRhcHRlciAqYWRhcHRlciwNCj4gPiArCQkJ
CSAgIHU4ICpmaXJtd2FyZSwgdTMyIGZpcm13YXJlX2xlbikgew0KPiANCj4gQ2FuIHlvdSBtYWtl
ICdmaXJtd2FyZScgY29uc3Q/IEFsc28sIHRvIGhlbHAgYmVsb3csIGl0IG1pZ2h0IHdvcmsgYmV0
dGVyIGFzDQo+ICdjb25zdCB2b2lkIConLg0KPiANCg0KT0ssIFRoYW5rcyBmb3IgdGhlIHJldmll
dy4NCg0KPiA+ICsJc3RydWN0IG13aWZpZXhfZndfZGF0YSBmd2RhdGE7DQo+ID4gKwl1MzIgb2Zm
c2V0ID0gMCwgZGF0YV9sZW4sIGRubGRfY21kOw0KPiA+ICsJaW50IHJldCA9IDA7DQo+ID4gKwli
b29sIGNtZDdfYmVmb3JlID0gZmFsc2U7DQo+ID4gKw0KPiA+ICsJd2hpbGUgKDEpIHsNCj4gPiAr
CQlpZiAob2Zmc2V0ICsgc2l6ZW9mKGZ3ZGF0YS5oZWFkZXIpID49IGZpcm13YXJlX2xlbikgew0K
PiA+ICsJCQltd2lmaWV4X2RiZyhhZGFwdGVyLCBFUlJPUiwNCj4gPiArCQkJCSAgICAiZXh0cmFj
dCB3aWZpLW9ubHkgZmlybXdhcmUgZmFpbHVyZSEiKTsNCj4gPiArCQkJcmV0ID0gLTE7DQo+ID4g
KwkJCWdvdG8gZG9uZTsNCj4gPiArCQl9DQo+ID4gKw0KPiA+ICsJCW1lbWNweSgmZndkYXRhLmhl
YWRlciwgZmlybXdhcmUgKyBvZmZzZXQsDQo+ID4gKwkJICAgICAgIHNpemVvZihmd2RhdGEuaGVh
ZGVyKSk7DQo+IA0KPiBEbyB5b3UgYWN0dWFsbHkgbmVlZCB0byBjb3B5IHRoaXM/IENhbid0IHlv
dSBqdXN0IGtlZXAgYSBwb2ludGVyIHRvIHRoZSBsb2NhdGlvbj8NCj4gZS5nLg0KPiANCj4gCWNv
bnN0IHN0cnVjdCBtd2lmaWV4X2Z3X2RhdGEgKmZ3ZGF0YTsNCj4gLi4uDQo+IAlmd2RhdGEgPSBm
aXJtd2FyZSArIG9mZnNldDsNCj4gDQoNCk9rLg0KDQo+ID4gKwkJZG5sZF9jbWQgPSBsZTMyX3Rv
X2NwdShmd2RhdGEuaGVhZGVyLmRubGRfY21kKTsNCj4gPiArCQlkYXRhX2xlbiA9IGxlMzJfdG9f
Y3B1KGZ3ZGF0YS5oZWFkZXIuZGF0YV9sZW5ndGgpOw0KPiA+ICsNCj4gPiArCQlzd2l0Y2ggKGRu
bGRfY21kKSB7DQo+ID4gKwkJY2FzZSBNV0lGSUVYX0ZXX0ROTERfQ01EXzE6DQo+ID4gKwkJCWlm
ICghY21kN19iZWZvcmUpIHsNCj4gPiArCQkJCW13aWZpZXhfZGJnKGFkYXB0ZXIsIEVSUk9SLA0K
PiA+ICsJCQkJCSAgICAibm8gY21kNyBiZWZvcmUgY21kMSEiKTsNCj4gPiArCQkJCXJldCA9IC0x
Ow0KPiA+ICsJCQkJZ290byBkb25lOw0KPiA+ICsJCQl9DQo+ID4gKwkJCW9mZnNldCArPSBkYXRh
X2xlbiArIHNpemVvZihmd2RhdGEuaGVhZGVyKTsNCj4gDQo+IFRlY2huaWNhbGx5LCB3ZSBuZWVk
IGFuIG92ZXJmbG93IGNoZWNrIHRvIG1ha2Ugc3VyZSB0aGF0ICdkYXRhX2xlbicNCj4gaXNuJ3Qg
c29tZSBib2d1cyB2YWx1ZSB0aGF0IG92ZXJmbG93cyAnb2Zmc2V0Jy4NCj4gDQoNClRoZXJlIGlz
IHRoZSBzYW5pdHkgY2hlY2sgZm9yIHRoZSBvZmZzZXQgYWZ0ZXIgYnlwYXNzIENNRDEvNS83IGlu
IHRoZSBzdGFydCBvZiB3aGlsZS1sb29wLCBlbmhhbmNlZCBpbiB2NCBhcywNCmlmIChvZmZzZXQg
Pj0gZmlybXdhcmVfbGVuKQ0KDQo+ID4gKwkJCWJyZWFrOw0KPiA+ICsJCWNhc2UgTVdJRklFWF9G
V19ETkxEX0NNRF81Og0KPiA+ICsJCQlvZmZzZXQgKz0gZGF0YV9sZW4gKyBzaXplb2YoZndkYXRh
LmhlYWRlcik7DQo+IA0KPiBTYW1lIGhlcmUuDQo+IA0KPiA+ICsJCQlicmVhazsNCj4gPiArCQlj
YXNlIE1XSUZJRVhfRldfRE5MRF9DTURfNjoNCj4gDQo+IENhbiB3ZSBoYXZlIGEgY29tbWVudCwg
ZWl0aGVyIGhlcmUgb3IgYWJvdmUgdGhpcyBmdW5jdGlvbiwgdG8gZGVzY3JpYmUgd2hhdA0KPiB0
aGlzIHNlcXVlbmNlIGlzPyBPciBwYXJ0aWN1bGFybHksIHdoYXQgaXMgdGhpcyB0ZXJtaW5hdGlu
ZyBjb25kaXRpb24/ICJDTURfNiINCj4gZG9lc24ndCByZWFsbHkgdGVsbCBtZSB0aGF0IHRoaXMg
aXMgdGhlIHN0YXJ0IG9mIHRoZSBXaWZpIGJsb2IuDQo+IA0KPiA+ICsJCQlvZmZzZXQgKz0gZGF0
YV9sZW4gKyBzaXplb2YoZndkYXRhLmhlYWRlcik7DQo+IA0KDQpUaGUgc2VxdWVuY2UgaGF2ZSBi
ZWVuIGFkZGVkIHRvIGZ1bmN0aW9uIGNvbW1lbnRzIGluIHY0Lg0KDQo+IFlvdSBkb24ndCBjaGVj
ayBmb3Igb3ZlcmZsb3cgaGVyZS4gQ2hlY2sgdGhpcyBiZWZvcmUgcmV0dXJuaW5nIHRoaXMgKGVp
dGhlciBoZXJlLA0KPiBvciBpbiB0aGUgJ2RvbmUnIGxhYmVsKS4NCj4gDQoNClllcywgYWRkIHNh
bml0eSBjaGVjayBmb3IgdGhlIG9mZnNldCBpbiBlbmQgb2YgQ01ENi4NCg0KPiA+ICsJCQlyZXQg
PSBvZmZzZXQ7DQo+ID4gKwkJCWdvdG8gZG9uZTsNCj4gPiArCQljYXNlIE1XSUZJRVhfRldfRE5M
RF9DTURfNzoNCj4gPiArCQkJaWYgKCFjbWQ3X2JlZm9yZSkNCj4gDQo+IF5eIFRoaXMgJ2lmJyBp
c24ndCByZWFsbHkgbmVlZGVkLg0KDQpEb25lLg0KDQo+IA0KPiA+ICsJCQkJY21kN19iZWZvcmUg
PSB0cnVlOw0KPiA+ICsJCQlvZmZzZXQgKz0gc2l6ZW9mKGZ3ZGF0YS5oZWFkZXIpOw0KPiA+ICsJ
CQlicmVhazsNCj4gPiArCQlkZWZhdWx0Og0KPiA+ICsJCQltd2lmaWV4X2RiZyhhZGFwdGVyLCBF
UlJPUiwgInVua25vd24gZG5sZF9jbWQgJWRcbiIsDQo+ID4gKwkJCQkgICAgZG5sZF9jbWQpOw0K
PiA+ICsJCQlyZXQgPSAtMTsNCj4gPiArCQkJZ290byBkb25lOw0KPiA+ICsJCX0NCj4gPiArCX0N
Cj4gPiArDQo+ID4gK2RvbmU6DQo+ID4gKwlyZXR1cm4gcmV0Ow0KPiA+ICt9DQo+ID4gKw0KPiA+
ICAvKg0KPiA+ICAgKiBUaGlzIGZ1bmN0aW9uIGRvd25sb2FkcyB0aGUgZmlybXdhcmUgdG8gdGhl
IGNhcmQuDQo+ID4gICAqDQo+ID4gQEAgLTE5NzEsNyArMjAyOCw3IEBAIHN0YXRpYyBpbnQgbXdp
ZmlleF9wcm9nX2Z3X3dfaGVscGVyKHN0cnVjdA0KPiBtd2lmaWV4X2FkYXB0ZXIgKmFkYXB0ZXIs
DQo+ID4gIAl1MzIgZmlybXdhcmVfbGVuID0gZnctPmZ3X2xlbjsNCj4gPiAgCXUzMiBvZmZzZXQg
PSAwOw0KPiA+ICAJc3RydWN0IHNrX2J1ZmYgKnNrYjsNCj4gPiAtCXUzMiB0eGxlbiwgdHhfYmxv
Y2tzID0gMCwgdHJpZXMsIGxlbjsNCj4gPiArCXUzMiB0eGxlbiwgdHhfYmxvY2tzID0gMCwgdHJp
ZXMsIGxlbiwgdmFsOw0KPiA+ICAJdTMyIGJsb2NrX3JldHJ5X2NudCA9IDA7DQo+ID4gIAlzdHJ1
Y3QgcGNpZV9zZXJ2aWNlX2NhcmQgKmNhcmQgPSBhZGFwdGVyLT5jYXJkOw0KPiA+ICAJY29uc3Qg
c3RydWN0IG13aWZpZXhfcGNpZV9jYXJkX3JlZyAqcmVnID0gY2FyZC0+cGNpZS5yZWc7IEBAIC0x
OTk4LDYNCj4gPiArMjA1NSwyNCBAQCBzdGF0aWMgaW50IG13aWZpZXhfcHJvZ19md193X2hlbHBl
cihzdHJ1Y3QgbXdpZmlleF9hZGFwdGVyDQo+ICphZGFwdGVyLA0KPiA+ICAJCWdvdG8gZG9uZTsN
Cj4gPiAgCX0NCj4gPg0KPiA+ICsJcmV0ID0gbXdpZmlleF9yZWFkX3JlZyhhZGFwdGVyLCBQQ0lF
X1NDUkFUQ0hfMTNfUkVHLCAmdmFsKTsNCj4gPiArCWlmIChyZXQpIHsNCj4gPiArCQltd2lmaWV4
X2RiZyhhZGFwdGVyLCBGQVRBTCwgIkZhaWxlZCB0byByZWFkIHNjcmF0Y2ggcmVnaXN0ZXIgMTNc
biIpOw0KPiA+ICsJCWdvdG8gZG9uZTsNCj4gPiArCX0NCj4gPiArDQo+ID4gKwkvKiBQQ0lFIEZM
UiBjYXNlOiBleHRyYWN0IHdpZmkgcGFydCBmcm9tIGNvbWJvIGZpcm13YXJlKi8NCj4gPiArCWlm
ICh2YWwgPT0gTVdJRklFWF9QQ0lFX0ZMUl9IQVBQRU5TKSB7DQo+ID4gKwkJcmV0ID0gbXdpZmll
eF9leHRyYWN0X3dpZmlfZncoYWRhcHRlciwgZmlybXdhcmUsIGZpcm13YXJlX2xlbik7DQo+ID4g
KwkJaWYgKHJldCA8IDApIHsNCj4gPiArCQkJbXdpZmlleF9kYmcoYWRhcHRlciwgRVJST1IsICJG
YWlsZWQgdG8gZXh0cmFjdCB3aWZpIGZ3XG4iKTsNCj4gPiArCQkJZ290byBkb25lOw0KPiA+ICsJ
CX0NCj4gPiArCQlvZmZzZXQgPSByZXQ7DQo+ID4gKwkJbXdpZmlleF9kYmcoYWRhcHRlciwgTVNH
LA0KPiA+ICsJCQkgICAgImluZm86IGRubGQgd2lmaSBmaXJtd2FyZSBmcm9tICVkIGJ5dGVzXG4i
LCBvZmZzZXQpOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiAgCS8qIFBlcmZvcm0gZmlybXdhcmUgZGF0
YSB0cmFuc2ZlciAqLw0KPiA+ICAJZG8gew0KPiA+ICAJCXUzMiBpcmVnX2ludHIgPSAwOw0KPiA+
IEBAIC0zMDYwLDEyICszMTM1LDYgQEAgc3RhdGljIHZvaWQgbXdpZmlleF9wY2llX3VwX2Rldihz
dHJ1Y3QNCj4gbXdpZmlleF9hZGFwdGVyICphZGFwdGVyKQ0KPiA+ICAJc3RydWN0IHBjaV9kZXYg
KnBkZXYgPSBjYXJkLT5kZXY7DQo+ID4gIAljb25zdCBzdHJ1Y3QgbXdpZmlleF9wY2llX2NhcmRf
cmVnICpyZWcgPSBjYXJkLT5wY2llLnJlZzsNCj4gPg0KPiA+IC0JLyogQmx1ZXRvb3RoIGlzIG5v
dCBvbiBwY2llIGludGVyZmFjZS4gRG93bmxvYWQgV2lmaSBvbmx5IGZpcm13YXJlDQo+ID4gLQkg
KiBkdXJpbmcgcGNpZSBGTFIsIHNvIHRoYXQgYmx1ZXRvb3RoIHBhcnQgb2YgZmlybXdhcmUgd2hp
Y2ggaXMNCj4gPiAtCSAqIGFscmVhZHkgcnVubmluZyBkb2Vzbid0IGdldCBhZmZlY3RlZC4NCj4g
PiAtCSAqLw0KPiA+IC0Jc3RyY3B5KGFkYXB0ZXItPmZ3X25hbWUsIFBDSUU4OTk3X0RFRkFVTFRf
V0lGSUZXX05BTUUpOw0KPiANCj4gTm93IHRoYXQgdGhlcmUncyBubyB1c2VycywgZGVsZXRlIFBD
SUU4OTk3X0RFRkFVTFRfV0lGSUZXX05BTUUgZnJvbQ0KPiBwY2llLmguDQoNClJlbW92ZWQgaW4g
VjQuDQoNCj4gDQo+IEJyaWFuDQo+IA0KPiA+IC0NCj4gPiAgCS8qIHR4X2J1Zl9zaXplIG1pZ2h0
IGJlIGNoYW5nZWQgdG8gMzU4NCBieSBmaXJtd2FyZSBkdXJpbmcNCj4gPiAgCSAqIGRhdGEgdHJh
bnNmZXIsIHdlIHNob3VsZCByZXNldCBpdCB0byBkZWZhdWx0IHNpemUuDQo+ID4gIAkgKi8NCj4g
PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L3BjaWUu
aA0KPiA+IGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L3BjaWUuaA0KPiA+
IGluZGV4IDdlMjQ1MGMuLjU0YWVjZGEgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9uZXQvd2ly
ZWxlc3MvbWFydmVsbC9td2lmaWV4L3BjaWUuaA0KPiA+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVs
ZXNzL21hcnZlbGwvbXdpZmlleC9wY2llLmgNCj4gPiBAQCAtMTIwLDYgKzEyMCw4IEBADQo+ID4g
ICNkZWZpbmUgTVdJRklFWF9TTEVFUF9DT09LSUVfU0laRQkJCTQNCj4gPiAgI2RlZmluZSBNV0lG
SUVYX01BWF9ERUxBWV9DT1VOVAkJCQkxMDANCj4gPg0KPiA+ICsjZGVmaW5lIE1XSUZJRVhfUENJ
RV9GTFJfSEFQUEVOUyAweEZFRENCQUJBDQo+ID4gKw0KPiA+ICBzdHJ1Y3QgbXdpZmlleF9wY2ll
X2NhcmRfcmVnIHsNCj4gPiAgCXUxNiBjbWRfYWRkcl9sbzsNCj4gPiAgCXUxNiBjbWRfYWRkcl9o
aTsNCj4gPiAtLQ0KPiA+IDEuOC4xLjQNCj4gPg0K

Re: Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Brian Norris]

Hi,

On Thu, Apr 13, 2017 at 06:47:59AM +0000, Xinming Hu wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:briannorris@chromium.org]
> > Sent: 2017年4月11日 9:37
> > To: Xinming Hu
> > Cc: Linux Wireless; Kalle Valo; Dmitry Torokhov; rajatja@google.com;
> > Amitkumar Karwar; Cathy Luo; Xinming Hu; Ganapathi Bhat
> > Subject: [EXT] Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo
> > firmware during function level reset
> > 
> > External Email
> > 

> > On Mon, Apr 10, 2017 at 09:09:34AM +0000, Xinming Hu wrote:
> > > +	while (1) {
> > > +		if (offset + sizeof(fwdata.header) >= firmware_len) {
> > > +			mwifiex_dbg(adapter, ERROR,
> > > +				    "extract wifi-only firmware failure!");
> > > +			ret = -1;
> > > +			goto done;
> > > +		}
> > > +
> > > +		memcpy(&fwdata.header, firmware + offset,
> > > +		       sizeof(fwdata.header));
> > 
> > Do you actually need to copy this? Can't you just keep a pointer to the location?
> > e.g.
> > 
> > 	const struct mwifiex_fw_data *fwdata;
> > ...
> > 	fwdata = firmware + offset;
> > 
> 
> Ok.
> 
> > > +		dnld_cmd = le32_to_cpu(fwdata.header.dnld_cmd);
> > > +		data_len = le32_to_cpu(fwdata.header.data_length);
> > > +
> > > +		switch (dnld_cmd) {
> > > +		case MWIFIEX_FW_DNLD_CMD_1:
> > > +			if (!cmd7_before) {
> > > +				mwifiex_dbg(adapter, ERROR,
> > > +					    "no cmd7 before cmd1!");
> > > +				ret = -1;
> > > +				goto done;
> > > +			}
> > > +			offset += data_len + sizeof(fwdata.header);
> > 
> > Technically, we need an overflow check to make sure that 'data_len'
> > isn't some bogus value that overflows 'offset'.
> > 
> 
> There is the sanity check for the offset after bypass CMD1/5/7 in the start of while-loop, enhanced in v4 as,
> if (offset >= firmware_len)

That's not an enhancement!! That's a *weaker* condition, and it's wrong.
If 'offset == firmware_len - 1', then we'll still be out of bounds when
we read the next header at 'offset + {1, 2, 3, ...}'.

My point was that adding 'data_len' might actually overflow the u32, not
that the bounds check ('offset + sizeof(...header) >= firmware_len') was
wrong.

For this kind of overflow check, you need to do the check here, not
after you've saved the new offset.

e.g., suppose data_len = 0xfffffff0. Then:

	offset += data_len + sizeof(fwdata.header);
=>
	offset += 0xfffffff0 + 16;
=>
	offset += 0;

and then...we have an infinite loop. Changing the bounds check at the
start of the loop does nothing to help this.

Something like this (put before the addition) is sufficient, I think:

	if (offset + data_len + sizeof(fwdata.header) < data_len)
		... error ...

Or this would actually all be slightly cleaner if you just did this
outside the 'switch':

	// Presuming you already did the check for
	//  offset + sizeof(fwdata.header) >= firmware_len
	offset += sizeof(fwdata.header);

Then inside this 'case', you have:

	if (offset + data_len < data_len)
		... error out ...
	offset += data_len;

> > > +			break;
> > > +		case MWIFIEX_FW_DNLD_CMD_5:
> > > +			offset += data_len + sizeof(fwdata.header);
> > 
> > Same here.
> > 
> > > +			break;
> > > +		case MWIFIEX_FW_DNLD_CMD_6:
> > 
> > Can we have a comment, either here or above this function, to describe what
> > this sequence is? Or particularly, what is this terminating condition? "CMD_6"
> > doesn't really tell me that this is the start of the Wifi blob.
> > 
> > > +			offset += data_len + sizeof(fwdata.header);
> > 
> 
> The sequence have been added to function comments in v4.
> 
> > You don't check for overflow here. Check this before returning this (either here,
> > or in the 'done' label).
> > 
> 
> Yes, add sanity check for the offset in end of CMD6.
> 
> > > +			ret = offset;
> > > +			goto done;
> > > +		case MWIFIEX_FW_DNLD_CMD_7:
> > > +			if (!cmd7_before)
> > 
> > ^^ This 'if' isn't really needed.
> 
> Done.
> 
> > 
> > > +				cmd7_before = true;
> > > +			offset += sizeof(fwdata.header);
> > > +			break;
> > > +		default:
> > > +			mwifiex_dbg(adapter, ERROR, "unknown dnld_cmd %d\n",
> > > +				    dnld_cmd);
> > > +			ret = -1;
> > > +			goto done;
> > > +		}
> > > +	}
> > > +
> > > +done:
> > > +	return ret;
> > > +}

[...]

Brian

RE: Re: Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Xinming Hu]

SGkgQnJhaW4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQnJpYW4g
Tm9ycmlzIFttYWlsdG86YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnXQ0KPiBTZW50OiAyMDE35bm0
NOaciDE05pelIDE6NTANCj4gVG86IFhpbm1pbmcgSHUNCj4gQ2M6IExpbnV4IFdpcmVsZXNzOyBL
YWxsZSBWYWxvOyBEbWl0cnkgVG9yb2tob3Y7IHJhamF0amFAZ29vZ2xlLmNvbTsNCj4gQW1pdGt1
bWFyIEthcndhcjsgQ2F0aHkgTHVvOyBHYW5hcGF0aGkgQmhhdA0KPiBTdWJqZWN0OiBbRVhUXSBS
ZTogUmU6IFtQQVRDSCB2MyA0LzRdIG13aWZpZXg6IHBjaWU6IGV4dHJhY3Qgd2lmaSBwYXJ0IGZy
b20NCj4gY29tYm8gZmlybXdhcmUgZHVyaW5nIGZ1bmN0aW9uIGxldmVsIHJlc2V0DQo+IA0KPiBF
eHRlcm5hbCBFbWFpbA0KPiANCj4gLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPiBIaSwNCj4gDQo+IE9uIFRodSwg
QXByIDEzLCAyMDE3IGF0IDA2OjQ3OjU5QU0gKzAwMDAsIFhpbm1pbmcgSHUgd3JvdGU6DQo+ID4g
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gRnJvbTogQnJpYW4gTm9ycmlzIFtt
YWlsdG86YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnXQ0KPiA+ID4gU2VudDogMjAxN+W5tDTmnIgx
MeaXpSA5OjM3DQo+ID4gPiBUbzogWGlubWluZyBIdQ0KPiA+ID4gQ2M6IExpbnV4IFdpcmVsZXNz
OyBLYWxsZSBWYWxvOyBEbWl0cnkgVG9yb2tob3Y7IHJhamF0amFAZ29vZ2xlLmNvbTsNCj4gPiA+
IEFtaXRrdW1hciBLYXJ3YXI7IENhdGh5IEx1bzsgWGlubWluZyBIdTsgR2FuYXBhdGhpIEJoYXQN
Cj4gPiA+IFN1YmplY3Q6IFtFWFRdIFJlOiBbUEFUQ0ggdjMgNC80XSBtd2lmaWV4OiBwY2llOiBl
eHRyYWN0IHdpZmkgcGFydA0KPiA+ID4gZnJvbSBjb21ibyBmaXJtd2FyZSBkdXJpbmcgZnVuY3Rp
b24gbGV2ZWwgcmVzZXQNCj4gPiA+DQo+ID4gPiBFeHRlcm5hbCBFbWFpbA0KPiA+ID4NCj4gDQo+
ID4gPiBPbiBNb24sIEFwciAxMCwgMjAxNyBhdCAwOTowOTozNEFNICswMDAwLCBYaW5taW5nIEh1
IHdyb3RlOg0KPiA+ID4gPiArCXdoaWxlICgxKSB7DQo+ID4gPiA+ICsJCWlmIChvZmZzZXQgKyBz
aXplb2YoZndkYXRhLmhlYWRlcikgPj0gZmlybXdhcmVfbGVuKSB7DQo+ID4gPiA+ICsJCQltd2lm
aWV4X2RiZyhhZGFwdGVyLCBFUlJPUiwNCj4gPiA+ID4gKwkJCQkgICAgImV4dHJhY3Qgd2lmaS1v
bmx5IGZpcm13YXJlIGZhaWx1cmUhIik7DQo+ID4gPiA+ICsJCQlyZXQgPSAtMTsNCj4gPiA+ID4g
KwkJCWdvdG8gZG9uZTsNCj4gPiA+ID4gKwkJfQ0KPiA+ID4gPiArDQo+ID4gPiA+ICsJCW1lbWNw
eSgmZndkYXRhLmhlYWRlciwgZmlybXdhcmUgKyBvZmZzZXQsDQo+ID4gPiA+ICsJCSAgICAgICBz
aXplb2YoZndkYXRhLmhlYWRlcikpOw0KPiA+ID4NCj4gPiA+IERvIHlvdSBhY3R1YWxseSBuZWVk
IHRvIGNvcHkgdGhpcz8gQ2FuJ3QgeW91IGp1c3Qga2VlcCBhIHBvaW50ZXIgdG8gdGhlDQo+IGxv
Y2F0aW9uPw0KPiA+ID4gZS5nLg0KPiA+ID4NCj4gPiA+IAljb25zdCBzdHJ1Y3QgbXdpZmlleF9m
d19kYXRhICpmd2RhdGE7IC4uLg0KPiA+ID4gCWZ3ZGF0YSA9IGZpcm13YXJlICsgb2Zmc2V0Ow0K
PiA+ID4NCj4gPg0KPiA+IE9rLg0KPiA+DQo+ID4gPiA+ICsJCWRubGRfY21kID0gbGUzMl90b19j
cHUoZndkYXRhLmhlYWRlci5kbmxkX2NtZCk7DQo+ID4gPiA+ICsJCWRhdGFfbGVuID0gbGUzMl90
b19jcHUoZndkYXRhLmhlYWRlci5kYXRhX2xlbmd0aCk7DQo+ID4gPiA+ICsNCj4gPiA+ID4gKwkJ
c3dpdGNoIChkbmxkX2NtZCkgew0KPiA+ID4gPiArCQljYXNlIE1XSUZJRVhfRldfRE5MRF9DTURf
MToNCj4gPiA+ID4gKwkJCWlmICghY21kN19iZWZvcmUpIHsNCj4gPiA+ID4gKwkJCQltd2lmaWV4
X2RiZyhhZGFwdGVyLCBFUlJPUiwNCj4gPiA+ID4gKwkJCQkJICAgICJubyBjbWQ3IGJlZm9yZSBj
bWQxISIpOw0KPiA+ID4gPiArCQkJCXJldCA9IC0xOw0KPiA+ID4gPiArCQkJCWdvdG8gZG9uZTsN
Cj4gPiA+ID4gKwkJCX0NCj4gPiA+ID4gKwkJCW9mZnNldCArPSBkYXRhX2xlbiArIHNpemVvZihm
d2RhdGEuaGVhZGVyKTsNCj4gPiA+DQo+ID4gPiBUZWNobmljYWxseSwgd2UgbmVlZCBhbiBvdmVy
ZmxvdyBjaGVjayB0byBtYWtlIHN1cmUgdGhhdCAnZGF0YV9sZW4nDQo+ID4gPiBpc24ndCBzb21l
IGJvZ3VzIHZhbHVlIHRoYXQgb3ZlcmZsb3dzICdvZmZzZXQnLg0KPiA+ID4NCj4gPg0KPiA+IFRo
ZXJlIGlzIHRoZSBzYW5pdHkgY2hlY2sgZm9yIHRoZSBvZmZzZXQgYWZ0ZXIgYnlwYXNzIENNRDEv
NS83IGluIHRoZQ0KPiA+IHN0YXJ0IG9mIHdoaWxlLWxvb3AsIGVuaGFuY2VkIGluIHY0IGFzLCBp
ZiAob2Zmc2V0ID49IGZpcm13YXJlX2xlbikNCj4gDQo+IFRoYXQncyBub3QgYW4gZW5oYW5jZW1l
bnQhISBUaGF0J3MgYSAqd2Vha2VyKiBjb25kaXRpb24sIGFuZCBpdCdzIHdyb25nLg0KPiBJZiAn
b2Zmc2V0ID09IGZpcm13YXJlX2xlbiAtIDEnLCB0aGVuIHdlJ2xsIHN0aWxsIGJlIG91dCBvZiBi
b3VuZHMgd2hlbiB3ZSByZWFkDQo+IHRoZSBuZXh0IGhlYWRlciBhdCAnb2Zmc2V0ICsgezEsIDIs
IDMsIC4uLn0nLg0KPiANCj4gTXkgcG9pbnQgd2FzIHRoYXQgYWRkaW5nICdkYXRhX2xlbicgbWln
aHQgYWN0dWFsbHkgb3ZlcmZsb3cgdGhlIHUzMiwgbm90IHRoYXQNCj4gdGhlIGJvdW5kcyBjaGVj
ayAoJ29mZnNldCArIHNpemVvZiguLi5oZWFkZXIpID49IGZpcm13YXJlX2xlbicpIHdhcyB3cm9u
Zy4NCj4gDQo+IEZvciB0aGlzIGtpbmQgb2Ygb3ZlcmZsb3cgY2hlY2ssIHlvdSBuZWVkIHRvIGRv
IHRoZSBjaGVjayBoZXJlLCBub3QgYWZ0ZXIgeW91J3ZlDQo+IHNhdmVkIHRoZSBuZXcgb2Zmc2V0
Lg0KPiANCj4gZS5nLiwgc3VwcG9zZSBkYXRhX2xlbiA9IDB4ZmZmZmZmZjAuIFRoZW46DQo+IA0K
PiAJb2Zmc2V0ICs9IGRhdGFfbGVuICsgc2l6ZW9mKGZ3ZGF0YS5oZWFkZXIpOyA9Pg0KPiAJb2Zm
c2V0ICs9IDB4ZmZmZmZmZjAgKyAxNjsNCj4gPT4NCj4gCW9mZnNldCArPSAwOw0KPiANCj4gYW5k
IHRoZW4uLi53ZSBoYXZlIGFuIGluZmluaXRlIGxvb3AuIENoYW5naW5nIHRoZSBib3VuZHMgY2hl
Y2sgYXQgdGhlIHN0YXJ0IG9mDQo+IHRoZSBsb29wIGRvZXMgbm90aGluZyB0byBoZWxwIHRoaXMu
DQo+IA0KPiBTb21ldGhpbmcgbGlrZSB0aGlzIChwdXQgYmVmb3JlIHRoZSBhZGRpdGlvbikgaXMg
c3VmZmljaWVudCwgSSB0aGluazoNCj4gDQo+IAlpZiAob2Zmc2V0ICsgZGF0YV9sZW4gKyBzaXpl
b2YoZndkYXRhLmhlYWRlcikgPCBkYXRhX2xlbikNCj4gCQkuLi4gZXJyb3IgLi4uDQo+IA0KDQpU
aGFua3MgZm9yIHRoZSBleHBsYWluLg0KQWNjb3JkaW5nIHRvIHRoZSBmaXJtd2FyZSBkb3dubG9h
ZCBwcm90b2NvbCwgZXZlcnkgQ01EIHNob3VsZCBub3QgZXhjZWVkIE1XSUZJRVhfVVBMRF9TSVpF
Lg0Kd2UgY2FuIGFkZCBhIHNhbml0eSBjaGVjayAsIGxpa2UsDQppZiAoZGF0YV9sZW4gPiBNV0lG
SUVYX1VQTERfU0laRSAtIHNpemVvZihmd2RhdGEtPmhlYWRlcikpDQoJKmVycm9yKg0KDQpUaGFu
a3MNClNpbW9uDQo+IE9yIHRoaXMgd291bGQgYWN0dWFsbHkgYWxsIGJlIHNsaWdodGx5IGNsZWFu
ZXIgaWYgeW91IGp1c3QgZGlkIHRoaXMgb3V0c2lkZSB0aGUNCj4gJ3N3aXRjaCc6DQo+IA0KPiAJ
Ly8gUHJlc3VtaW5nIHlvdSBhbHJlYWR5IGRpZCB0aGUgY2hlY2sgZm9yDQo+IAkvLyAgb2Zmc2V0
ICsgc2l6ZW9mKGZ3ZGF0YS5oZWFkZXIpID49IGZpcm13YXJlX2xlbg0KPiAJb2Zmc2V0ICs9IHNp
emVvZihmd2RhdGEuaGVhZGVyKTsNCj4gDQo+IFRoZW4gaW5zaWRlIHRoaXMgJ2Nhc2UnLCB5b3Ug
aGF2ZToNCj4gDQo+IAlpZiAob2Zmc2V0ICsgZGF0YV9sZW4gPCBkYXRhX2xlbikNCj4gCQkuLi4g
ZXJyb3Igb3V0IC4uLg0KPiAJb2Zmc2V0ICs9IGRhdGFfbGVuOw0KPiANCj4gPiA+ID4gKwkJCWJy
ZWFrOw0KPiA+ID4gPiArCQljYXNlIE1XSUZJRVhfRldfRE5MRF9DTURfNToNCj4gPiA+ID4gKwkJ
CW9mZnNldCArPSBkYXRhX2xlbiArIHNpemVvZihmd2RhdGEuaGVhZGVyKTsNCj4gPiA+DQo+ID4g
PiBTYW1lIGhlcmUuDQo+ID4gPg0KPiA+ID4gPiArCQkJYnJlYWs7DQo+ID4gPiA+ICsJCWNhc2Ug
TVdJRklFWF9GV19ETkxEX0NNRF82Og0KPiA+ID4NCj4gPiA+IENhbiB3ZSBoYXZlIGEgY29tbWVu
dCwgZWl0aGVyIGhlcmUgb3IgYWJvdmUgdGhpcyBmdW5jdGlvbiwgdG8NCj4gPiA+IGRlc2NyaWJl
IHdoYXQgdGhpcyBzZXF1ZW5jZSBpcz8gT3IgcGFydGljdWxhcmx5LCB3aGF0IGlzIHRoaXMgdGVy
bWluYXRpbmcNCj4gY29uZGl0aW9uPyAiQ01EXzYiDQo+ID4gPiBkb2Vzbid0IHJlYWxseSB0ZWxs
IG1lIHRoYXQgdGhpcyBpcyB0aGUgc3RhcnQgb2YgdGhlIFdpZmkgYmxvYi4NCj4gPiA+DQo+ID4g
PiA+ICsJCQlvZmZzZXQgKz0gZGF0YV9sZW4gKyBzaXplb2YoZndkYXRhLmhlYWRlcik7DQo+ID4g
Pg0KPiA+DQo+ID4gVGhlIHNlcXVlbmNlIGhhdmUgYmVlbiBhZGRlZCB0byBmdW5jdGlvbiBjb21t
ZW50cyBpbiB2NC4NCj4gPg0KPiA+ID4gWW91IGRvbid0IGNoZWNrIGZvciBvdmVyZmxvdyBoZXJl
LiBDaGVjayB0aGlzIGJlZm9yZSByZXR1cm5pbmcgdGhpcw0KPiA+ID4gKGVpdGhlciBoZXJlLCBv
ciBpbiB0aGUgJ2RvbmUnIGxhYmVsKS4NCj4gPiA+DQo+ID4NCj4gPiBZZXMsIGFkZCBzYW5pdHkg
Y2hlY2sgZm9yIHRoZSBvZmZzZXQgaW4gZW5kIG9mIENNRDYuDQo+ID4NCj4gPiA+ID4gKwkJCXJl
dCA9IG9mZnNldDsNCj4gPiA+ID4gKwkJCWdvdG8gZG9uZTsNCj4gPiA+ID4gKwkJY2FzZSBNV0lG
SUVYX0ZXX0ROTERfQ01EXzc6DQo+ID4gPiA+ICsJCQlpZiAoIWNtZDdfYmVmb3JlKQ0KPiA+ID4N
Cj4gPiA+IF5eIFRoaXMgJ2lmJyBpc24ndCByZWFsbHkgbmVlZGVkLg0KPiA+DQo+ID4gRG9uZS4N
Cj4gPg0KPiA+ID4NCj4gPiA+ID4gKwkJCQljbWQ3X2JlZm9yZSA9IHRydWU7DQo+ID4gPiA+ICsJ
CQlvZmZzZXQgKz0gc2l6ZW9mKGZ3ZGF0YS5oZWFkZXIpOw0KPiA+ID4gPiArCQkJYnJlYWs7DQo+
ID4gPiA+ICsJCWRlZmF1bHQ6DQo+ID4gPiA+ICsJCQltd2lmaWV4X2RiZyhhZGFwdGVyLCBFUlJP
UiwgInVua25vd24gZG5sZF9jbWQgJWRcbiIsDQo+ID4gPiA+ICsJCQkJICAgIGRubGRfY21kKTsN
Cj4gPiA+ID4gKwkJCXJldCA9IC0xOw0KPiA+ID4gPiArCQkJZ290byBkb25lOw0KPiA+ID4gPiAr
CQl9DQo+ID4gPiA+ICsJfQ0KPiA+ID4gPiArDQo+ID4gPiA+ICtkb25lOg0KPiA+ID4gPiArCXJl
dHVybiByZXQ7DQo+ID4gPiA+ICt9DQo+IA0KPiBbLi4uXQ0KPiANCj4gQnJpYW4NCg==

Re: Re: Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Brian Norris]

Hi,

On Fri, Apr 14, 2017 at 03:28:28AM +0000, Xinming Hu wrote:
> According to the firmware download protocol, every CMD should not exceed MWIFIEX_UPLD_SIZE.
> we can add a sanity check , like,
> if (data_len > MWIFIEX_UPLD_SIZE - sizeof(fwdata->header))
> 	*error*

I was primarily interested in protecting the kernel itself. Once the
kernel starts parsing the firmware, we have to make sure a bad firmware
file won't end up with us looping infinitely, reading/writing invalid
memory, or otherwise exposing security or stability issues. I wasn't
necessarily interested in validating every requirement of the end-point
device. For example, we're not bothering checking the CRCs. I figured that
this was all the job of your Wifi card's boot ROM.

So, we *can* implement checks like this, but I'd really hope we don't
need this particular one, because your card should be taking care of
that.

Please consider reviewing my latest submission.

Regards,
Brian

RE: Re: Re: Re: [PATCH v3 4/4] mwifiex: pcie: extract wifi part from combo firmware during function level reset

[Xinming Hu]

SGkgQnJhaW4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQnJpYW4g
Tm9ycmlzIFttYWlsdG86YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnXQ0KPiBTZW50OiAyMDE3xOo0
1MIxNcjVIDA6NTYNCj4gVG86IFhpbm1pbmcgSHUNCj4gQ2M6IExpbnV4IFdpcmVsZXNzOyBLYWxs
ZSBWYWxvOyBEbWl0cnkgVG9yb2tob3Y7IHJhamF0amFAZ29vZ2xlLmNvbTsNCj4gQW1pdGt1bWFy
IEthcndhcjsgQ2F0aHkgTHVvOyBHYW5hcGF0aGkgQmhhdA0KPiBTdWJqZWN0OiBbRVhUXSBSZTog
UmU6IFJlOiBbUEFUQ0ggdjMgNC80XSBtd2lmaWV4OiBwY2llOiBleHRyYWN0IHdpZmkgcGFydCBm
cm9tDQo+IGNvbWJvIGZpcm13YXJlIGR1cmluZyBmdW5jdGlvbiBsZXZlbCByZXNldA0KPiANCj4g
RXh0ZXJuYWwgRW1haWwNCj4gDQo+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gSGksDQo+IA0KPiBPbiBGcmks
IEFwciAxNCwgMjAxNyBhdCAwMzoyODoyOEFNICswMDAwLCBYaW5taW5nIEh1IHdyb3RlOg0KPiA+
IEFjY29yZGluZyB0byB0aGUgZmlybXdhcmUgZG93bmxvYWQgcHJvdG9jb2wsIGV2ZXJ5IENNRCBz
aG91bGQgbm90IGV4Y2VlZA0KPiBNV0lGSUVYX1VQTERfU0laRS4NCj4gPiB3ZSBjYW4gYWRkIGEg
c2FuaXR5IGNoZWNrICwgbGlrZSwNCj4gPiBpZiAoZGF0YV9sZW4gPiBNV0lGSUVYX1VQTERfU0la
RSAtIHNpemVvZihmd2RhdGEtPmhlYWRlcikpDQo+ID4gCSplcnJvcioNCj4gDQo+IEkgd2FzIHBy
aW1hcmlseSBpbnRlcmVzdGVkIGluIHByb3RlY3RpbmcgdGhlIGtlcm5lbCBpdHNlbGYuIE9uY2Ug
dGhlIGtlcm5lbCBzdGFydHMNCj4gcGFyc2luZyB0aGUgZmlybXdhcmUsIHdlIGhhdmUgdG8gbWFr
ZSBzdXJlIGEgYmFkIGZpcm13YXJlIGZpbGUgd29uJ3QgZW5kIHVwDQo+IHdpdGggdXMgbG9vcGlu
ZyBpbmZpbml0ZWx5LCByZWFkaW5nL3dyaXRpbmcgaW52YWxpZCBtZW1vcnksIG9yIG90aGVyd2lz
ZQ0KPiBleHBvc2luZyBzZWN1cml0eSBvciBzdGFiaWxpdHkgaXNzdWVzLiBJIHdhc24ndCBuZWNl
c3NhcmlseSBpbnRlcmVzdGVkIGluIHZhbGlkYXRpbmcNCj4gZXZlcnkgcmVxdWlyZW1lbnQgb2Yg
dGhlIGVuZC1wb2ludCBkZXZpY2UuIEZvciBleGFtcGxlLCB3ZSdyZSBub3QgYm90aGVyaW5nDQo+
IGNoZWNraW5nIHRoZSBDUkNzLiBJIGZpZ3VyZWQgdGhhdCB0aGlzIHdhcyBhbGwgdGhlIGpvYiBv
ZiB5b3VyIFdpZmkgY2FyZCdzIGJvb3QNCj4gUk9NLg0KPiANCj4gU28sIHdlICpjYW4qIGltcGxl
bWVudCBjaGVja3MgbGlrZSB0aGlzLCBidXQgSSdkIHJlYWxseSBob3BlIHdlIGRvbid0IG5lZWQg
dGhpcw0KPiBwYXJ0aWN1bGFyIG9uZSwgYmVjYXVzZSB5b3VyIGNhcmQgc2hvdWxkIGJlIHRha2lu
ZyBjYXJlIG9mIHRoYXQuDQo+IA0KDQpHb3QgaXQsIHdlIHdpbGwga2VlcCBpbiBtaW5kIHRvIGNo
ZWNrIHRoZSBwb3NzaWJsZSBvdmVyZmxvdyBpbiBmdXR1cmUsIGVpdGhlciB1c2luZyBnZW5lcmFs
DQpwcm90ZWN0IG9yIHVuZGVyIGxpbWl0IGJ5IG91ciBkZXZpY2UgcmVxdWlyZW1lbnQuDQoNCj4g
UGxlYXNlIGNvbnNpZGVyIHJldmlld2luZyBteSBsYXRlc3Qgc3VibWlzc2lvbi4NCj4gDQoNClN1
cmUuDQoNClRoYW5rcywNClNpbW9uDQo+IFJlZ2FyZHMsDQo+IEJyaWFuDQo=