[PATCH 0/5] brcmfmac: Stability change series

[PATCH 0/5] brcmfmac: Stability change series

[Wright Feng]

This series fixes stability and connection related failures.

Prasanna Kerekoppa (1):
  brcmfmac: To fix Bss Info flag definition Bug

Raveendran Somu (1):
  brcmfmac: To fix kernel crash on out of boundary access

Wright Feng (3):
  brcmfmac: fix invliad permanent MAC address in wiphy
  brcmfmac: keep SDIO watchdog running when console_interval is non-zero
  brcmfmac: reduce maximum station interface from 2 to 1 in RSDB mode

 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c   | 8 ++++----
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c   | 3 +++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   | 6 +++++-
 5 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.25.0

[PATCH 1/5] brcmfmac: To fix kernel crash on out of boundary access

[Wright Feng]

From: Raveendran Somu <raveendran.somu@cypress.com>

To trunkcate the addtional bytes, if extra bytes been received.
Current code only have a warning and proceed without handling it.
But in one of the crash reported by DVT, these causes the
crash intermittently. So the processing is limit to the skb->len.

Signed-off-by: Raveendran Somu <raveendran.somu@cypress.com>
Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Wright Feng <wright.feng@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
index 09701262330d..531fe9be4025 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
@@ -1843,6 +1843,9 @@ void brcmf_fws_hdrpull(struct brcmf_if *ifp, s16 siglen, struct sk_buff *skb)
 
 	WARN_ON(siglen > skb->len);
 
+	if (siglen > skb->len)
+		siglen = skb->len;
+
 	if (!siglen)
 		return;
 	/* if flow control disabled, skip to packet data and leave */
-- 
2.25.0

[PATCH 2/5] brcmfmac: fix invliad permanent MAC address in wiphy

[Wright Feng]

When host driver retrieves mac addresses from dongle, driver copies memory
from drvr->mac to perm_addr. But at the moment, drvr->mac is all zero
array which causes permanent MAC address in wiphy is all zero as well.
To fix this, we set drvr->mac before setting perm_addr.

Signed-off-by: Wright Feng <wright.feng@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index dec25e415619..e3758bd86acf 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -209,8 +209,8 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 		bphy_err(drvr, "Retrieving cur_etheraddr failed, %d\n", err);
 		goto done;
 	}
-	memcpy(ifp->drvr->wiphy->perm_addr, ifp->drvr->mac, ETH_ALEN);
 	memcpy(ifp->drvr->mac, ifp->mac_addr, sizeof(ifp->drvr->mac));
+	memcpy(ifp->drvr->wiphy->perm_addr, ifp->drvr->mac, ETH_ALEN);
 
 	bus = ifp->drvr->bus_if;
 	ri = &ifp->drvr->revinfo;
-- 
2.25.0

[PATCH 3/5] brcmfmac: keep SDIO watchdog running when console_interval is non-zero

[Wright Feng]

brcmfmac host driver makes SDIO bus sleep and stops SDIO watchdog if no
pending event or data. As a result, host driver does not poll firmware
console buffer before buffer overflow, which leads to missing firmware
logs. We should not stop SDIO watchdog if console_interval is non-zero
in debug build.

Signed-off-by: Wright Feng <wright.feng@cypress.com>
Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index ce6f15284277..50444989ae09 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3686,7 +3686,11 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 			if (bus->idlecount > bus->idletime) {
 				brcmf_dbg(SDIO, "idle\n");
 				sdio_claim_host(bus->sdiodev->func1);
-				brcmf_sdio_wd_timer(bus, false);
+#ifdef DEBUG
+				if (!BRCMF_FWCON_ON() ||
+				    bus->console_interval == 0)
+#endif
+					brcmf_sdio_wd_timer(bus, false);
 				bus->idlecount = 0;
 				brcmf_sdio_bus_sleep(bus, true, false);
 				sdio_release_host(bus->sdiodev->func1);
-- 
2.25.0

[PATCH 4/5] brcmfmac: reduce maximum station interface from 2 to 1 in RSDB mode

[Wright Feng]

The firmware state machines are not fully suitable for concurrent
station interface support, it may hit unexpected error if we have 2
different SSIDs and the roaming scenarios concurrently.
To avoid the bad user-experience if this is not fully validated, we
dis-allow user to create two concurrent station interfaces.

Signed-off-by: Wright Feng <wright.feng@cypress.com>
Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c   | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index a757abd7a599..8c1801fb59e7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6801,7 +6801,7 @@ brcmf_txrx_stypes[NUM_NL80211_IFTYPES] = {
  *	#AP <= 4, matching BI, channels = 1, 4 total
  *
  * no p2p and rsdb:
- *	#STA <= 2, #AP <= 2, channels = 2, 4 total
+ *	#STA <= 1, #AP <= 2, channels = 2, 4 total
  *
  * p2p, no mchan, and mbss:
  *
@@ -6816,7 +6816,7 @@ brcmf_txrx_stypes[NUM_NL80211_IFTYPES] = {
  *	#AP <= 4, matching BI, channels = 1, 4 total
  *
  * p2p, rsdb, and no mbss:
- *	#STA <= 2, #P2P-DEV <= 1, #{P2P-CL, P2P-GO} <= 2, AP <= 2,
+ *	#STA <= 1, #P2P-DEV <= 1, #{P2P-CL, P2P-GO} <= 2, AP <= 2,
  *	 channels = 2, 4 total
  */
 static int brcmf_setup_ifmodes(struct wiphy *wiphy, struct brcmf_if *ifp)
@@ -6857,7 +6857,7 @@ static int brcmf_setup_ifmodes(struct wiphy *wiphy, struct brcmf_if *ifp)
 		goto err;
 
 	combo[c].num_different_channels = 1 + (rsdb || (p2p && mchan));
-	c0_limits[i].max = 1 + rsdb;
+	c0_limits[i].max = 1;
 	c0_limits[i++].types = BIT(NL80211_IFTYPE_STATION);
 	if (mon_flag) {
 		c0_limits[i].max = 1;
@@ -6873,7 +6873,7 @@ static int brcmf_setup_ifmodes(struct wiphy *wiphy, struct brcmf_if *ifp)
 	if (p2p && rsdb) {
 		c0_limits[i].max = 2;
 		c0_limits[i++].types = BIT(NL80211_IFTYPE_AP);
-		combo[c].max_interfaces = 5;
+		combo[c].max_interfaces = 4;
 	} else if (p2p) {
 		combo[c].max_interfaces = i;
 	} else if (rsdb) {
-- 
2.25.0

[PATCH 5/5] brcmfmac: To fix Bss Info flag definition Bug

[Wright Feng]

From: Prasanna Kerekoppa <prasanna.kerekoppa@cypress.com>

Bss info flag definition need to be fixed from 0x2 to 0x4
This flag is for rssi info received on channel.
All Firmware branches defined as 0x4 and this is bug in brcmfmac.

Signed-off-by: Prasanna Kerekoppa <prasanna.kerekoppa@cypress.com>
Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Wright Feng <wright.feng@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index de0ef1b545c4..2e31cc10c195 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -19,7 +19,7 @@
 #define BRCMF_ARP_OL_PEER_AUTO_REPLY	0x00000008
 
 #define	BRCMF_BSS_INFO_VERSION	109 /* curr ver of brcmf_bss_info_le struct */
-#define BRCMF_BSS_RSSI_ON_CHANNEL	0x0002
+#define BRCMF_BSS_RSSI_ON_CHANNEL	0x0004
 
 #define BRCMF_STA_BRCM			0x00000001	/* Running a Broadcom driver */
 #define BRCMF_STA_WME			0x00000002	/* WMM association */
-- 
2.25.0

Re: [PATCH 1/5] brcmfmac: To fix kernel crash on out of boundary access

[Florian Fainelli]

On 6/1/2020 12:19 AM, Wright Feng wrote:
> From: Raveendran Somu <raveendran.somu@cypress.com>
> 
> To trunkcate the addtional bytes, if extra bytes been received.

typo: truncate. Missing "have been received".

> Current code only have a warning and proceed without handling it.
> But in one of the crash reported by DVT, these causes the
> crash intermittently. So the processing is limit to the skb->len.
> 
> Signed-off-by: Raveendran Somu <raveendran.somu@cypress.com>
> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
> Signed-off-by: Wright Feng <wright.feng@cypress.com>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
> index 09701262330d..531fe9be4025 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
> @@ -1843,6 +1843,9 @@ void brcmf_fws_hdrpull(struct brcmf_if *ifp, s16 siglen, struct sk_buff *skb)
>  
>  	WARN_ON(siglen > skb->len);
>  
> +	if (siglen > skb->len)
> +		siglen = skb->len;

Does it make sense to keep the WARN_ON() one live above then?

> +
>  	if (!siglen)
>  		return;
>  	/* if flow control disabled, skip to packet data and leave */
> 

-- 
Florian

Re: [PATCH 1/5] brcmfmac: To fix kernel crash on out of boundary access

[Wright Feng]

Florian Fainelli 於 6/2/2020 12:34 PM 寫道:
> 
> 
> On 6/1/2020 12:19 AM, Wright Feng wrote:
>> From: Raveendran Somu <raveendran.somu@cypress.com>
>>
>> To trunkcate the addtional bytes, if extra bytes been received.
> 
> typo: truncate. Missing "have been received".
Thanks for catching this, Raveendran will correct it in V2.
> 
>> Current code only have a warning and proceed without handling it.
>> But in one of the crash reported by DVT, these causes the
>> crash intermittently. So the processing is limit to the skb->len.
>>
>> Signed-off-by: Raveendran Somu <raveendran.somu@cypress.com>
>> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
>> Signed-off-by: Wright Feng <wright.feng@cypress.com>
>> ---
>>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
>> index 09701262330d..531fe9be4025 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
>> @@ -1843,6 +1843,9 @@ void brcmf_fws_hdrpull(struct brcmf_if *ifp, s16 siglen, struct sk_buff *skb)
>>   
>>   	WARN_ON(siglen > skb->len);
>>   
>> +	if (siglen > skb->len)
>> +		siglen = skb->len;
> 
> Does it make sense to keep the WARN_ON() one live above then?
Regarding the change, I believe it is better to have the WARN_ON as 
getting something more than expected is indicating the violation of 
protocol.So it may be better to leave the warning, instead of quietly 
correctly it, which can help in identifying the malicious operations.

If the warning is not complying with the rules, we can remove it, if not 
we suggest to have it.
>> +
>>   	if (!siglen)
>>   		return;
>>   	/* if flow control disabled, skip to packet data and leave */
>>
>