2019-11-13 05:16:30

by Ganapathi Bhat

[permalink] [raw]
Subject: [PATCH 1/2] mwifiex: fix requesting zero memory for firmware dump

From: Sharvari Harisangam <[email protected]>

mwifiex_pcie_fw_dump would read firmware scratch registers, to
get the size of the dump. It does a vmalloc of memory_size + 1,
read above, to save the dump. It is possible that the value read
by memory_size scratch register be invalid, i.e 0xffffffff. This
would pass an invalid size(0) to vmalloc. To fix this check for
invalid scratch register read.

Signed-off-by: Sharvari Harisangam <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index fc1706d..483b521 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2727,6 +2727,13 @@ static void mwifiex_pcie_fw_dump(struct mwifiex_adapter *adapter)
break;
}

+ if (memory_size == 0xffffffff) {
+ mwifiex_dbg(adapter, ERROR,
+ "Invalid dump size: 0x%x, for %s\n",
+ memory_size, entry->mem_name);
+ return;
+ }
+
mwifiex_dbg(adapter, DUMP,
"%s_SIZE=0x%x\n", entry->mem_name, memory_size);
entry->mem_ptr = vmalloc(memory_size + 1);
--
1.9.1


2019-11-13 05:16:54

by Ganapathi Bhat

[permalink] [raw]
Subject: [PATCH 2/2] mwifiex: do not download TX packets if interface has not resumed

From: Sharvari Harisangam <[email protected]>

mwifiex_queue_tx_pkt would queue the TX packet to driver queues
and before return, would invoke main_work, which in turn would
download the packet to firmware. In case the card is in sleep
state, driver would wakeup the card(via interface).

During resume it is possible that, the applications send packets
even before interface has completed its resume. If driver try to
access interface register(to wakeup the card), it would lead to
invalid results.

To avoid this, don't invoke main_work, when hs_activated is set.

Signed-off-by: Sharvari Harisangam <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/main.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index a9657ae..7b5cf2c0 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -808,6 +808,11 @@ int mwifiex_queue_tx_pkt(struct mwifiex_private *priv, struct sk_buff *skb)
mwifiex_wmm_add_buf_txqueue(priv, skb);
}

+ if (priv->adapter->hs_activated) {
+ mwifiex_dbg(priv->adapter, ERROR, "hs_activated: queue TX\n");
+ return 0;
+ }
+
mwifiex_queue_main_work(priv->adapter);

return 0;
--
1.9.1

2019-11-14 15:21:00

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] mwifiex: fix requesting zero memory for firmware dump

Ganapathi Bhat <[email protected]> writes:

> From: Sharvari Harisangam <[email protected]>
>
> mwifiex_pcie_fw_dump would read firmware scratch registers, to
> get the size of the dump. It does a vmalloc of memory_size + 1,
> read above, to save the dump. It is possible that the value read
> by memory_size scratch register be invalid, i.e 0xffffffff. This
> would pass an invalid size(0) to vmalloc. To fix this check for
> invalid scratch register read.
>
> Signed-off-by: Sharvari Harisangam <[email protected]>
> Signed-off-by: Ganapathi Bhat <[email protected]>
> ---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index fc1706d..483b521 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -2727,6 +2727,13 @@ static void mwifiex_pcie_fw_dump(struct mwifiex_adapter *adapter)
> break;
> }
>
> + if (memory_size == 0xffffffff) {
> + mwifiex_dbg(adapter, ERROR,
> + "Invalid dump size: 0x%x, for %s\n",
> + memory_size, entry->mem_name);
> + return;
> + }
> +
> mwifiex_dbg(adapter, DUMP,
> "%s_SIZE=0x%x\n", entry->mem_name, memory_size);
> entry->mem_ptr = vmalloc(memory_size + 1);

So 0xfffffffe would be a valid length for vmalloc()? I doubt that :) A
proper fix would be to add a reasonable maximum for memory_size and
return if it's anything bigger than the limit. Never trust the firmware.

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