RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

Hi Brian,

> -Original Message-
> From: Ganapathi Bhat
> Sent: Tuesday, January 30, 2018 8:55 PM
> To: 'Brian Norris'
> Cc: Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> Hi Brian,
>
> > -Original Message-
> > From: Brian Norris [mailto:briannor...@chromium.org]
> > Sent: Tuesday, January 30, 2018 3:38 AM
> > To: Ganapathi Bhat
> > Cc: Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu;
> > Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> > Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid
> > USB RX stall
> >
> > Hi,
> >
> > On Sun, Jan 28, 2018 at 11:19 PM, Ganapathi Bhat 
> > wrote:
> > >> From: Ganapathi Bhat
> > >> > From: Brian Norris [mailto:briannor...@chromium.org] On Thu, Jan
> > >> > 25, 2018 at 09:59:04AM +, Ganapathi Bhat wrote:
> > >> > > > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> > >> > > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2]
> > >> > > sent in this
> > >> > series.
> > >> >
> > >> > What? Why would you introduce a bug and only fix it in the next
> patch?
> > >> With the first patch the original issue took considerably longer
> > >> time to recreate. Also it followed a different path to get
> > >> recreated(shared in commit message).
> > >> > Does that mean you should just combine the two?
> > >> Let us know if that is fine to merge them. We can modify the commit
> > >> message accordingly.
> > >> > Or reverse the order, if patch 2 doesn't cause problems on its own?
> > >> Patch 2 has a dependency on patch 1.
> > > One correction. There is no commit dependency between patch 1 and
> > 2(they can be applied in any order). But the result would be same. We
> > need both fixes. Let us know if we can combine them.
> >
> > I haven't closely looked at patch 2 yet. My only statement was that it
> > makes no sense to have 2 commits, with the second one labeled as
> > "Fixing" the first one. From your descriptions, it sounds like patch 2
> > should actually come first,
> Ok. I understand. I will reorder them and send v3(along with spinlock
> change).
I was trying to reorder the patch but found patch 2 is indeed dependent on 
patch 1. Consider the first point in commit message of patch 2:
1. USB receives an RX data interrupt, queues rx_work
This is the change added in patch 1. Earlier on receive of RX data interrupt, 
driver would queue main_work, which in turn queued rx_work. But after patch 1 
driver tries to  queue rx_work in interrupt context.

Let us please know your thoughts on this.

> > but I'm not really sure. I only looked far enough to say that I didn't
> > like patch
> > 1 as-is :)
> >
> > Brian
> Regards,
> Ganapathi

Re: ath9k will not tx packets sometimes.

[Sebastian Gottschall]

Am 30.01.2018 um 19:55 schrieb Toke Høiland-Jørgensen:

Ben Greear  writes:


I'm actually working on reworking that whole scheduler logic, and move
some of it into mac80211. Could you test this (WiP) patch and see if
that has the same problem?

It had some serious conflicts in ath10k, due to my local changes, so
I did not actually test this.

Can send you a version without the ath10k changes tomorrow if you'd like
to test - but will try to reproduce myself as well...


But, a revert of the atf patches (a6e56d749 and 63fefa050) appear to
have resolved the issue. I'll test more with these reverted, and maybe
will have time to work more on actually fixing upstream code next time
I move to a newer kernel (and/or after your pending changes get in).

Ah, that narrows it down some. Well, that is the code I'm hacking on
currently anyway, so let's see if we can't get it fixed as part of that
series :)
i have some addition information for you maybe. in the same timeframe i 
noticed a increased memory usage for ath9k devices.
maybe that helps. so i hit memory boundaries on embedded devices with 
dual interfaces and just 32 mb  ram now which wasnt the case before

is this patch worth to try from my side?

Sebastian

--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

Centrino Advanced-N 6235 - Microcode SW error detected

[Pali Rohár]

Hello,

for almost 3 years I have a problem with Intel wifi card Centrino
Advanced-N 6235. Its firmware periodically crash and in dmesg I'm seeing
a message "Microcode SW error detected".

Problem happens independently of kernel versions (I tried 3.8, 3.13,
3.18, 4.0, 4.9). Currently I'm using Debian Stretch and its 4.9 version.

This wifi card is identified as:

$ lspci -s 03:00.0 -vv
03:00.0 Network controller: Intel Corporation Centrino Advanced-N 6235 (rev 24)
Subsystem: Intel Corporation Centrino Advanced-N 6235 AGN
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- 

signature.asc
Description: PGP signature

[PATCH][next] wil6210: fix spelling mistake: "preperation"-> "preparation"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in debug error message text.

Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/ath/wil6210/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wil6210/main.c 
b/drivers/net/wireless/ath/wil6210/main.c
index 0c61a6c13991..04c8651274d9 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -715,7 +715,7 @@ static void wil_bl_prepare_halt(struct wil6210_priv *wil)
offsetof(struct bl_dedicated_registers_v0,
 boot_loader_struct_version));
if (!tmp) {
-   wil_dbg_misc(wil, "old BL, skipping halt preperation\n");
+   wil_dbg_misc(wil, "old BL, skipping halt preparation\n");
return;
}
 
-- 
2.15.1

Re: ath9k will not tx packets sometimes.

[Toke Høiland-Jørgensen]

Ben Greear  writes:

>> I'm actually working on reworking that whole scheduler logic, and move
>> some of it into mac80211. Could you test this (WiP) patch and see if
>> that has the same problem?
>
> It had some serious conflicts in ath10k, due to my local changes, so
> I did not actually test this.

Can send you a version without the ath10k changes tomorrow if you'd like
to test - but will try to reproduce myself as well...

> But, a revert of the atf patches (a6e56d749 and 63fefa050) appear to
> have resolved the issue. I'll test more with these reverted, and maybe
> will have time to work more on actually fixing upstream code next time
> I move to a newer kernel (and/or after your pending changes get in).

Ah, that narrows it down some. Well, that is the code I'm hacking on
currently anyway, so let's see if we can't get it fixed as part of that
series :)

-Toke

[PATCH] wireless: zd1211rw: remove redundant assignment of pointer 'q'

[Colin King]

From: Colin Ian King 

Pointer q is initialized and then almost immediately afterwards being
re-assigned the same value. Remove the second redundant assignment.

Cleans up clang warning:
drivers/net/wireless/zydas/zd1211rw/zd_mac.c:503:23: warning: Value
stored to 'q' during its initialization is never read

Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_mac.c 
b/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
index b785742bfd9e..b01b44a5d16e 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
@@ -509,7 +509,6 @@ void zd_mac_tx_failed(struct urb *urb)
int found = 0;
int i, position = 0;
 
-   q = >ack_wait_queue;
spin_lock_irqsave(>lock, flags);
 
skb_queue_walk(q, skb) {
-- 
2.15.1

Re: ath9k will not tx packets sometimes.

[Ben Greear]

On 01/30/2018 04:07 AM, Toke Høiland-Jørgensen wrote:

Ben Greear  writes:


Maybe there is some way for the scheduler to get stuck and not
schedule anything?


It would appear so, yeah. Do you do anything special other than
associate a whole bunch of stations to trigger this? I can try to see if
I can script something that works equivalently on my setup.


Ok, and I'll give you my user-space software package to easily set up
this test case if you want to test with that.  Contact me off-list if
you want help setting this up.



I'm actually working on reworking that whole scheduler logic, and move
some of it into mac80211. Could you test this (WiP) patch and see if
that has the same problem?


It had some serious conflicts in ath10k, due to my local changes, so
I did not actually test this.

But, a revert of the atf patches (a6e56d749 and 63fefa050) appear to have 
resolved the
issue.  I'll test more with these reverted, and maybe will have time to
work more on actually fixing upstream code next time I move to a newer
kernel (and/or after your pending changes get in).

Thanks,
Ben


--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com

[PATCH 7/7] staging: wilc1000: rename Handle_Key() and Handle_ConnectTimeout()

[Ajay Singh]

Fix "Avoid camelCase" issue found by checkpatch.pl script.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index f5fdca7..5e01f6e 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1165,7 +1165,7 @@ static s32 Handle_Connect(struct wilc_vif *vif,
return result;
 }
 
-static s32 Handle_ConnectTimeout(struct wilc_vif *vif)
+static s32 handle_connect_timeout(struct wilc_vif *vif)
 {
s32 result = 0;
struct connect_info strConnectInfo;
@@ -1525,7 +1525,7 @@ static s32 Handle_RcvdGnrlAsyncInfo(struct wilc_vif *vif,
return result;
 }
 
-static int Handle_Key(struct wilc_vif *vif,
+static int handle_key(struct wilc_vif *vif,
  struct key_attr *pstrHostIFkeyAttr)
 {
s32 result = 0;
@@ -2511,7 +2511,7 @@ static void host_if_work(struct work_struct *work)
break;
 
case HOST_IF_MSG_KEY:
-   Handle_Key(msg->vif, >body.key_info);
+   handle_key(msg->vif, >body.key_info);
break;
 
case HOST_IF_MSG_CFG_PARAMS:
@@ -2578,7 +2578,7 @@ static void host_if_work(struct work_struct *work)
break;
 
case HOST_IF_MSG_CONNECT_TIMER_FIRED:
-   Handle_ConnectTimeout(msg->vif);
+   handle_connect_timeout(msg->vif);
break;
 
case HOST_IF_MSG_POWER_MGMT:
-- 
2.7.4

[PATCH 5/7] staging: wilc1000: rename pu8CurrByte variable to avoid camelCase

[Ajay Singh]

Fix "Avoid camelCase" issue reported by checkpatch.pl script.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 232 +++---
 1 file changed, 116 insertions(+), 116 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 496c72f..43f8559 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -908,7 +908,7 @@ static s32 Handle_Connect(struct wilc_vif *vif,
s32 result = 0;
struct wid wid_list[8];
u32 wid_cnt = 0, dummyval = 0;
-   u8 *pu8CurrByte = NULL;
+   u8 *cur_byte = NULL;
struct join_bss_param *ptstrJoinBssParam;
struct host_if_drv *hif_drv = vif->hif_drv;
 
@@ -1016,90 +1016,90 @@ static s32 Handle_Connect(struct wilc_vif *vif,
goto ERRORHANDLER;
}
 
-   pu8CurrByte = wid_list[wid_cnt].val;
+   cur_byte = wid_list[wid_cnt].val;
 
if (pstrHostIFconnectAttr->ssid) {
-   memcpy(pu8CurrByte, pstrHostIFconnectAttr->ssid, 
pstrHostIFconnectAttr->ssid_len);
-   pu8CurrByte[pstrHostIFconnectAttr->ssid_len] = '\0';
+   memcpy(cur_byte, pstrHostIFconnectAttr->ssid, 
pstrHostIFconnectAttr->ssid_len);
+   cur_byte[pstrHostIFconnectAttr->ssid_len] = '\0';
}
-   pu8CurrByte += MAX_SSID_LEN;
-   *(pu8CurrByte++) = INFRASTRUCTURE;
+   cur_byte += MAX_SSID_LEN;
+   *(cur_byte++) = INFRASTRUCTURE;
 
if (pstrHostIFconnectAttr->ch >= 1 && pstrHostIFconnectAttr->ch <= 14) {
-   *(pu8CurrByte++) = pstrHostIFconnectAttr->ch;
+   *(cur_byte++) = pstrHostIFconnectAttr->ch;
} else {
netdev_err(vif->ndev, "Channel out of range\n");
-   *(pu8CurrByte++) = 0xFF;
+   *(cur_byte++) = 0xFF;
}
-   *(pu8CurrByte++)  = (ptstrJoinBssParam->cap_info) & 0xFF;
-   *(pu8CurrByte++)  = ((ptstrJoinBssParam->cap_info) >> 8) & 0xFF;
+   *(cur_byte++)  = (ptstrJoinBssParam->cap_info) & 0xFF;
+   *(cur_byte++)  = ((ptstrJoinBssParam->cap_info) >> 8) & 0xFF;
 
if (pstrHostIFconnectAttr->bssid)
-   memcpy(pu8CurrByte, pstrHostIFconnectAttr->bssid, 6);
-   pu8CurrByte += 6;
+   memcpy(cur_byte, pstrHostIFconnectAttr->bssid, 6);
+   cur_byte += 6;
 
if (pstrHostIFconnectAttr->bssid)
-   memcpy(pu8CurrByte, pstrHostIFconnectAttr->bssid, 6);
-   pu8CurrByte += 6;
+   memcpy(cur_byte, pstrHostIFconnectAttr->bssid, 6);
+   cur_byte += 6;
 
-   *(pu8CurrByte++)  = (ptstrJoinBssParam->beacon_period) & 0xFF;
-   *(pu8CurrByte++)  = ((ptstrJoinBssParam->beacon_period) >> 8) & 0xFF;
-   *(pu8CurrByte++)  =  ptstrJoinBssParam->dtim_period;
+   *(cur_byte++)  = (ptstrJoinBssParam->beacon_period) & 0xFF;
+   *(cur_byte++)  = ((ptstrJoinBssParam->beacon_period) >> 8) & 0xFF;
+   *(cur_byte++)  =  ptstrJoinBssParam->dtim_period;
 
-   memcpy(pu8CurrByte, ptstrJoinBssParam->supp_rates, MAX_RATES_SUPPORTED 
+ 1);
-   pu8CurrByte += (MAX_RATES_SUPPORTED + 1);
+   memcpy(cur_byte, ptstrJoinBssParam->supp_rates, MAX_RATES_SUPPORTED + 
1);
+   cur_byte += (MAX_RATES_SUPPORTED + 1);
 
-   *(pu8CurrByte++)  =  ptstrJoinBssParam->wmm_cap;
-   *(pu8CurrByte++)  = ptstrJoinBssParam->uapsd_cap;
+   *(cur_byte++)  =  ptstrJoinBssParam->wmm_cap;
+   *(cur_byte++)  = ptstrJoinBssParam->uapsd_cap;
 
-   *(pu8CurrByte++)  = ptstrJoinBssParam->ht_capable;
+   *(cur_byte++)  = ptstrJoinBssParam->ht_capable;
hif_drv->usr_conn_req.ht_capable = ptstrJoinBssParam->ht_capable;
 
-   *(pu8CurrByte++)  =  ptstrJoinBssParam->rsn_found;
-   *(pu8CurrByte++)  =  ptstrJoinBssParam->rsn_grp_policy;
-   *(pu8CurrByte++) =  ptstrJoinBssParam->mode_802_11i;
+   *(cur_byte++)  =  ptstrJoinBssParam->rsn_found;
+   *(cur_byte++)  =  ptstrJoinBssParam->rsn_grp_policy;
+   *(cur_byte++) =  ptstrJoinBssParam->mode_802_11i;
 
-   memcpy(pu8CurrByte, ptstrJoinBssParam->rsn_pcip_policy, 
sizeof(ptstrJoinBssParam->rsn_pcip_policy));
-   pu8CurrByte += sizeof(ptstrJoinBssParam->rsn_pcip_policy);
+   memcpy(cur_byte, ptstrJoinBssParam->rsn_pcip_policy, 
sizeof(ptstrJoinBssParam->rsn_pcip_policy));
+   cur_byte += sizeof(ptstrJoinBssParam->rsn_pcip_policy);
 
-   memcpy(pu8CurrByte, ptstrJoinBssParam->rsn_auth_policy, 
sizeof(ptstrJoinBssParam->rsn_auth_policy));
-   pu8CurrByte += sizeof(ptstrJoinBssParam->rsn_auth_policy);
+   memcpy(cur_byte, ptstrJoinBssParam->rsn_auth_policy, 
sizeof(ptstrJoinBssParam->rsn_auth_policy));
+   cur_byte += sizeof(ptstrJoinBssParam->rsn_auth_policy);
 
-   memcpy(pu8CurrByte, ptstrJoinBssParam->rsn_cap, 
sizeof(ptstrJoinBssParam->rsn_cap));
-   pu8CurrByte 

[PATCH 6/7] staging: wilc1000: rename Handle_ScanDone function to avoid camelCase

[Ajay Singh]

Fix "Avoid camelCase" issue reported by checkpatch.pl.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 43f8559..f5fdca7 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -267,7 +267,7 @@ static struct wilc_vif *join_req_vif;
 
 static void *host_int_parse_join_bss_param(struct network_info *info);
 static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
-static s32 Handle_ScanDone(struct wilc_vif *vif, enum scan_event enuEvent);
+static s32 handle_scan_done(struct wilc_vif *vif, enum scan_event enuEvent);
 static void host_if_work(struct work_struct *work);
 
 /*!
@@ -847,7 +847,7 @@ static s32 handle_scan(struct wilc_vif *vif, struct 
scan_attr *scan_info)
 ERRORHANDLER:
if (result) {
del_timer(_drv->scan_timer);
-   Handle_ScanDone(vif, SCAN_EVENT_ABORTED);
+   handle_scan_done(vif, SCAN_EVENT_ABORTED);
}
 
kfree(scan_info->ch_freq_list);
@@ -863,8 +863,8 @@ static s32 handle_scan(struct wilc_vif *vif, struct 
scan_attr *scan_info)
return result;
 }
 
-static s32 Handle_ScanDone(struct wilc_vif *vif,
-  enum scan_event enuEvent)
+static s32 handle_scan_done(struct wilc_vif *vif,
+   enum scan_event enuEvent)
 {
s32 result = 0;
u8 u8abort_running_scan;
@@ -1467,7 +1467,7 @@ static s32 Handle_RcvdGnrlAsyncInfo(struct wilc_vif *vif,
 
if (hif_drv->usr_scan_req.scan_result) {
del_timer(_drv->scan_timer);
-   Handle_ScanDone(vif, SCAN_EVENT_ABORTED);
+   handle_scan_done(vif, SCAN_EVENT_ABORTED);
}
 
strDisconnectNotifInfo.reason = 0;
@@ -1515,7 +1515,7 @@ static s32 Handle_RcvdGnrlAsyncInfo(struct wilc_vif *vif,
   (hif_drv->usr_scan_req.scan_result)) {
del_timer(_drv->scan_timer);
if (hif_drv->usr_scan_req.scan_result)
-   Handle_ScanDone(vif, SCAN_EVENT_ABORTED);
+   handle_scan_done(vif, SCAN_EVENT_ABORTED);
}
}
 
@@ -2532,7 +2532,7 @@ static void host_if_work(struct work_struct *work)
if (!wilc_wlan_get_num_conn_ifcs(wilc))
wilc_chip_sleep_manually(wilc);
 
-   Handle_ScanDone(msg->vif, SCAN_EVENT_DONE);
+   handle_scan_done(msg->vif, SCAN_EVENT_DONE);
 
if (msg->vif->hif_drv->remain_on_ch_pending)
Handle_RemainOnChan(msg->vif,
@@ -2574,7 +2574,7 @@ static void host_if_work(struct work_struct *work)
break;
 
case HOST_IF_MSG_SCAN_TIMER_FIRED:
-   Handle_ScanDone(msg->vif, SCAN_EVENT_ABORTED);
+   handle_scan_done(msg->vif, SCAN_EVENT_ABORTED);
break;
 
case HOST_IF_MSG_CONNECT_TIMER_FIRED:
-- 
2.7.4

[PATCH 4/7] staging: wilc1000: rename u32WidsCount to avoid camelCase

[Ajay Singh]

Fix "Avoid camleCase" issue reported by checkpatch.pl script.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 146 +++---
 1 file changed, 73 insertions(+), 73 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 13ac482..496c72f 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -907,7 +907,7 @@ static s32 Handle_Connect(struct wilc_vif *vif,
 {
s32 result = 0;
struct wid wid_list[8];
-   u32 u32WidsCount = 0, dummyval = 0;
+   u32 wid_cnt = 0, dummyval = 0;
u8 *pu8CurrByte = NULL;
struct join_bss_param *ptstrJoinBssParam;
struct host_if_drv *hif_drv = vif->hif_drv;
@@ -952,30 +952,30 @@ static s32 Handle_Connect(struct wilc_vif *vif,
hif_drv->usr_conn_req.conn_result = pstrHostIFconnectAttr->result;
hif_drv->usr_conn_req.arg = pstrHostIFconnectAttr->arg;
 
-   wid_list[u32WidsCount].id = WID_SUCCESS_FRAME_COUNT;
-   wid_list[u32WidsCount].type = WID_INT;
-   wid_list[u32WidsCount].size = sizeof(u32);
-   wid_list[u32WidsCount].val = (s8 *)(&(dummyval));
-   u32WidsCount++;
+   wid_list[wid_cnt].id = WID_SUCCESS_FRAME_COUNT;
+   wid_list[wid_cnt].type = WID_INT;
+   wid_list[wid_cnt].size = sizeof(u32);
+   wid_list[wid_cnt].val = (s8 *)(&(dummyval));
+   wid_cnt++;
 
-   wid_list[u32WidsCount].id = WID_RECEIVED_FRAGMENT_COUNT;
-   wid_list[u32WidsCount].type = WID_INT;
-   wid_list[u32WidsCount].size = sizeof(u32);
-   wid_list[u32WidsCount].val = (s8 *)(&(dummyval));
-   u32WidsCount++;
+   wid_list[wid_cnt].id = WID_RECEIVED_FRAGMENT_COUNT;
+   wid_list[wid_cnt].type = WID_INT;
+   wid_list[wid_cnt].size = sizeof(u32);
+   wid_list[wid_cnt].val = (s8 *)(&(dummyval));
+   wid_cnt++;
 
-   wid_list[u32WidsCount].id = WID_FAILED_COUNT;
-   wid_list[u32WidsCount].type = WID_INT;
-   wid_list[u32WidsCount].size = sizeof(u32);
-   wid_list[u32WidsCount].val = (s8 *)(&(dummyval));
-   u32WidsCount++;
+   wid_list[wid_cnt].id = WID_FAILED_COUNT;
+   wid_list[wid_cnt].type = WID_INT;
+   wid_list[wid_cnt].size = sizeof(u32);
+   wid_list[wid_cnt].val = (s8 *)(&(dummyval));
+   wid_cnt++;
 
{
-   wid_list[u32WidsCount].id = WID_INFO_ELEMENT_ASSOCIATE;
-   wid_list[u32WidsCount].type = WID_BIN_DATA;
-   wid_list[u32WidsCount].val = hif_drv->usr_conn_req.ies;
-   wid_list[u32WidsCount].size = hif_drv->usr_conn_req.ies_len;
-   u32WidsCount++;
+   wid_list[wid_cnt].id = WID_INFO_ELEMENT_ASSOCIATE;
+   wid_list[wid_cnt].type = WID_BIN_DATA;
+   wid_list[wid_cnt].val = hif_drv->usr_conn_req.ies;
+   wid_list[wid_cnt].size = hif_drv->usr_conn_req.ies_len;
+   wid_cnt++;
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7)) {
info_element_size = hif_drv->usr_conn_req.ies_len;
@@ -984,39 +984,39 @@ static s32 Handle_Connect(struct wilc_vif *vif,
   info_element_size);
}
}
-   wid_list[u32WidsCount].id = (u16)WID_11I_MODE;
-   wid_list[u32WidsCount].type = WID_CHAR;
-   wid_list[u32WidsCount].size = sizeof(char);
-   wid_list[u32WidsCount].val = (s8 *)_drv->usr_conn_req.security;
-   u32WidsCount++;
+   wid_list[wid_cnt].id = (u16)WID_11I_MODE;
+   wid_list[wid_cnt].type = WID_CHAR;
+   wid_list[wid_cnt].size = sizeof(char);
+   wid_list[wid_cnt].val = (s8 *)_drv->usr_conn_req.security;
+   wid_cnt++;
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7))
mode_11i = hif_drv->usr_conn_req.security;
 
-   wid_list[u32WidsCount].id = (u16)WID_AUTH_TYPE;
-   wid_list[u32WidsCount].type = WID_CHAR;
-   wid_list[u32WidsCount].size = sizeof(char);
-   wid_list[u32WidsCount].val = (s8 *)_drv->usr_conn_req.auth_type;
-   u32WidsCount++;
+   wid_list[wid_cnt].id = (u16)WID_AUTH_TYPE;
+   wid_list[wid_cnt].type = WID_CHAR;
+   wid_list[wid_cnt].size = sizeof(char);
+   wid_list[wid_cnt].val = (s8 *)_drv->usr_conn_req.auth_type;
+   wid_cnt++;
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7))
auth_type = (u8)hif_drv->usr_conn_req.auth_type;
 
-   wid_list[u32WidsCount].id = (u16)WID_JOIN_REQ_EXTENDED;
-   wid_list[u32WidsCount].type = WID_STR;
-   wid_list[u32WidsCount].size = 112;
-   wid_list[u32WidsCount].val = kmalloc(wid_list[u32WidsCount].size, 
GFP_KERNEL);
+   wid_list[wid_cnt].id = (u16)WID_JOIN_REQ_EXTENDED;
+   wid_list[wid_cnt].type = WID_STR;
+   wid_list[wid_cnt].size = 112;
+   

[PATCH 3/7] staging: wilc1000: rename strWIDList variable to avoid camelCase

[Ajay Singh]

Fix "Avoid camelCase" issue found by checkpatch.pl script.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 176 +++---
 1 file changed, 88 insertions(+), 88 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index f0168e3..13ac482 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -906,7 +906,7 @@ static s32 Handle_Connect(struct wilc_vif *vif,
  struct connect_attr *pstrHostIFconnectAttr)
 {
s32 result = 0;
-   struct wid strWIDList[8];
+   struct wid wid_list[8];
u32 u32WidsCount = 0, dummyval = 0;
u8 *pu8CurrByte = NULL;
struct join_bss_param *ptstrJoinBssParam;
@@ -952,29 +952,29 @@ static s32 Handle_Connect(struct wilc_vif *vif,
hif_drv->usr_conn_req.conn_result = pstrHostIFconnectAttr->result;
hif_drv->usr_conn_req.arg = pstrHostIFconnectAttr->arg;
 
-   strWIDList[u32WidsCount].id = WID_SUCCESS_FRAME_COUNT;
-   strWIDList[u32WidsCount].type = WID_INT;
-   strWIDList[u32WidsCount].size = sizeof(u32);
-   strWIDList[u32WidsCount].val = (s8 *)(&(dummyval));
+   wid_list[u32WidsCount].id = WID_SUCCESS_FRAME_COUNT;
+   wid_list[u32WidsCount].type = WID_INT;
+   wid_list[u32WidsCount].size = sizeof(u32);
+   wid_list[u32WidsCount].val = (s8 *)(&(dummyval));
u32WidsCount++;
 
-   strWIDList[u32WidsCount].id = WID_RECEIVED_FRAGMENT_COUNT;
-   strWIDList[u32WidsCount].type = WID_INT;
-   strWIDList[u32WidsCount].size = sizeof(u32);
-   strWIDList[u32WidsCount].val = (s8 *)(&(dummyval));
+   wid_list[u32WidsCount].id = WID_RECEIVED_FRAGMENT_COUNT;
+   wid_list[u32WidsCount].type = WID_INT;
+   wid_list[u32WidsCount].size = sizeof(u32);
+   wid_list[u32WidsCount].val = (s8 *)(&(dummyval));
u32WidsCount++;
 
-   strWIDList[u32WidsCount].id = WID_FAILED_COUNT;
-   strWIDList[u32WidsCount].type = WID_INT;
-   strWIDList[u32WidsCount].size = sizeof(u32);
-   strWIDList[u32WidsCount].val = (s8 *)(&(dummyval));
+   wid_list[u32WidsCount].id = WID_FAILED_COUNT;
+   wid_list[u32WidsCount].type = WID_INT;
+   wid_list[u32WidsCount].size = sizeof(u32);
+   wid_list[u32WidsCount].val = (s8 *)(&(dummyval));
u32WidsCount++;
 
{
-   strWIDList[u32WidsCount].id = WID_INFO_ELEMENT_ASSOCIATE;
-   strWIDList[u32WidsCount].type = WID_BIN_DATA;
-   strWIDList[u32WidsCount].val = hif_drv->usr_conn_req.ies;
-   strWIDList[u32WidsCount].size = hif_drv->usr_conn_req.ies_len;
+   wid_list[u32WidsCount].id = WID_INFO_ELEMENT_ASSOCIATE;
+   wid_list[u32WidsCount].type = WID_BIN_DATA;
+   wid_list[u32WidsCount].val = hif_drv->usr_conn_req.ies;
+   wid_list[u32WidsCount].size = hif_drv->usr_conn_req.ies_len;
u32WidsCount++;
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7)) {
@@ -984,39 +984,39 @@ static s32 Handle_Connect(struct wilc_vif *vif,
   info_element_size);
}
}
-   strWIDList[u32WidsCount].id = (u16)WID_11I_MODE;
-   strWIDList[u32WidsCount].type = WID_CHAR;
-   strWIDList[u32WidsCount].size = sizeof(char);
-   strWIDList[u32WidsCount].val = (s8 *)_drv->usr_conn_req.security;
+   wid_list[u32WidsCount].id = (u16)WID_11I_MODE;
+   wid_list[u32WidsCount].type = WID_CHAR;
+   wid_list[u32WidsCount].size = sizeof(char);
+   wid_list[u32WidsCount].val = (s8 *)_drv->usr_conn_req.security;
u32WidsCount++;
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7))
mode_11i = hif_drv->usr_conn_req.security;
 
-   strWIDList[u32WidsCount].id = (u16)WID_AUTH_TYPE;
-   strWIDList[u32WidsCount].type = WID_CHAR;
-   strWIDList[u32WidsCount].size = sizeof(char);
-   strWIDList[u32WidsCount].val = (s8 *)_drv->usr_conn_req.auth_type;
+   wid_list[u32WidsCount].id = (u16)WID_AUTH_TYPE;
+   wid_list[u32WidsCount].type = WID_CHAR;
+   wid_list[u32WidsCount].size = sizeof(char);
+   wid_list[u32WidsCount].val = (s8 *)_drv->usr_conn_req.auth_type;
u32WidsCount++;
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7))
auth_type = (u8)hif_drv->usr_conn_req.auth_type;
 
-   strWIDList[u32WidsCount].id = (u16)WID_JOIN_REQ_EXTENDED;
-   strWIDList[u32WidsCount].type = WID_STR;
-   strWIDList[u32WidsCount].size = 112;
-   strWIDList[u32WidsCount].val = kmalloc(strWIDList[u32WidsCount].size, 
GFP_KERNEL);
+   wid_list[u32WidsCount].id = (u16)WID_JOIN_REQ_EXTENDED;
+   wid_list[u32WidsCount].type = WID_STR;
+   wid_list[u32WidsCount].size = 112;
+   

[PATCH 2/7] staging: wilc1000: rename Handle_DelAllSta() and its variable using camelCase

[Ajay Singh]

Fix "Avoid camelCase" issue reported by checkpatch.pl script.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 8f8e727..f0168e3 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2123,34 +2123,34 @@ static void Handle_AddStation(struct wilc_vif *vif,
kfree(wid.val);
 }
 
-static void Handle_DelAllSta(struct wilc_vif *vif,
-struct del_all_sta *pstrDelAllStaParam)
+static void handle_del_all_sta(struct wilc_vif *vif,
+  struct del_all_sta *param)
 {
s32 result = 0;
struct wid wid;
-   u8 *pu8CurrByte;
+   u8 *curr_byte;
u8 i;
-   u8 au8Zero_Buff[6] = {0};
+   u8 zero_buff[6] = {0};
 
wid.id = (u16)WID_DEL_ALL_STA;
wid.type = WID_STR;
-   wid.size = (pstrDelAllStaParam->assoc_sta * ETH_ALEN) + 1;
+   wid.size = (param->assoc_sta * ETH_ALEN) + 1;
 
-   wid.val = kmalloc((pstrDelAllStaParam->assoc_sta * ETH_ALEN) + 1, 
GFP_KERNEL);
+   wid.val = kmalloc((param->assoc_sta * ETH_ALEN) + 1, GFP_KERNEL);
if (!wid.val)
goto ERRORHANDLER;
 
-   pu8CurrByte = wid.val;
+   curr_byte = wid.val;
 
-   *(pu8CurrByte++) = pstrDelAllStaParam->assoc_sta;
+   *(curr_byte++) = param->assoc_sta;
 
for (i = 0; i < MAX_NUM_STA; i++) {
-   if (memcmp(pstrDelAllStaParam->del_all_sta[i], au8Zero_Buff, 
ETH_ALEN))
-   memcpy(pu8CurrByte, pstrDelAllStaParam->del_all_sta[i], 
ETH_ALEN);
+   if (memcmp(param->del_all_sta[i], zero_buff, ETH_ALEN))
+   memcpy(curr_byte, param->del_all_sta[i], ETH_ALEN);
else
continue;
 
-   pu8CurrByte += ETH_ALEN;
+   curr_byte += ETH_ALEN;
}
 
result = wilc_send_config_pkt(vif, SET_CFG, , 1,
@@ -2626,7 +2626,7 @@ static void host_if_work(struct work_struct *work)
break;
 
case HOST_IF_MSG_DEL_ALL_STA:
-   Handle_DelAllSta(msg->vif, >body.del_all_sta_info);
+   handle_del_all_sta(msg->vif, >body.del_all_sta_info);
break;
 
case HOST_IF_MSG_SET_TX_POWER:
-- 
2.7.4

[PATCH 1/7] staging: wilc1000: rename variables using camelCase in host_int_ParseJoinBssParam()

[Ajay Singh]

Fix "Avoid CamelCase:" issue reported by checkpatch.pl script.
Rename host_int_ParseJoinBssParam() & its variables using camelCase.

Signed-off-by: Ajay Singh 
Reviewed-by: Claudiu Beznea 
---
 drivers/staging/wilc1000/host_interface.c | 230 +++---
 1 file changed, 115 insertions(+), 115 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 358354b..8f8e727 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -265,7 +265,7 @@ static struct wilc_vif *join_req_vif;
 #define FLUSHED_JOIN_REQ 1
 #define FLUSHED_BYTE_POS 79
 
-static void *host_int_ParseJoinBssParam(struct network_info *ptstrNetworkInfo);
+static void *host_int_parse_join_bss_param(struct network_info *info);
 static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
 static s32 Handle_ScanDone(struct wilc_vif *vif, enum scan_event enuEvent);
 static void host_if_work(struct work_struct *work);
@@ -1288,7 +1288,7 @@ static s32 Handle_RcvdNtwrkInfo(struct wilc_vif *vif,
hif_drv->usr_scan_req.rcvd_ch_cnt++;
 
pstrNetworkInfo->new_network = true;
-   pJoinParams = 
host_int_ParseJoinBssParam(pstrNetworkInfo);
+   pJoinParams = 
host_int_parse_join_bss_param(pstrNetworkInfo);
 

hif_drv->usr_scan_req.scan_result(SCAN_EVENT_NETWORK_FOUND, pstrNetworkInfo,
  
hif_drv->usr_scan_req.arg,
@@ -3870,152 +3870,152 @@ int wilc_setup_multicast_filter(struct wilc_vif *vif, 
bool enabled,
return result;
 }
 
-static void *host_int_ParseJoinBssParam(struct network_info *ptstrNetworkInfo)
+static void *host_int_parse_join_bss_param(struct network_info *info)
 {
-   struct join_bss_param *pNewJoinBssParam = NULL;
-   u8 *pu8IEs;
-   u16 u16IEsLen;
+   struct join_bss_param *param = NULL;
+   u8 *ies;
+   u16 ies_len;
u16 index = 0;
-   u8 suppRatesNo = 0;
-   u8 extSuppRatesNo;
-   u16 jumpOffset;
-   u8 pcipherCount;
-   u8 authCount;
-   u8 pcipherTotalCount = 0;
-   u8 authTotalCount = 0;
+   u8 rates_no = 0;
+   u8 ext_rates_no;
+   u16 offset;
+   u8 pcipher_cnt;
+   u8 auth_cnt;
+   u8 pcipher_total_cnt = 0;
+   u8 auth_total_cnt = 0;
u8 i, j;
 
-   pu8IEs = ptstrNetworkInfo->ies;
-   u16IEsLen = ptstrNetworkInfo->ies_len;
-
-   pNewJoinBssParam = kzalloc(sizeof(*pNewJoinBssParam), GFP_KERNEL);
-   if (pNewJoinBssParam) {
-   pNewJoinBssParam->dtim_period = ptstrNetworkInfo->dtim_period;
-   pNewJoinBssParam->beacon_period = 
ptstrNetworkInfo->beacon_period;
-   pNewJoinBssParam->cap_info = ptstrNetworkInfo->cap_info;
-   memcpy(pNewJoinBssParam->bssid, ptstrNetworkInfo->bssid, 6);
-   memcpy((u8 *)pNewJoinBssParam->ssid, ptstrNetworkInfo->ssid,
-  ptstrNetworkInfo->ssid_len + 1);
-   pNewJoinBssParam->ssid_len = ptstrNetworkInfo->ssid_len;
-   memset(pNewJoinBssParam->rsn_pcip_policy, 0xFF, 3);
-   memset(pNewJoinBssParam->rsn_auth_policy, 0xFF, 3);
-
-   while (index < u16IEsLen) {
-   if (pu8IEs[index] == SUPP_RATES_IE) {
-   suppRatesNo = pu8IEs[index + 1];
-   pNewJoinBssParam->supp_rates[0] = suppRatesNo;
+   ies = info->ies;
+   ies_len = info->ies_len;
+
+   param = kzalloc(sizeof(*param), GFP_KERNEL);
+   if (param) {
+   param->dtim_period = info->dtim_period;
+   param->beacon_period = info->beacon_period;
+   param->cap_info = info->cap_info;
+   memcpy(param->bssid, info->bssid, 6);
+   memcpy((u8 *)param->ssid, info->ssid,
+  info->ssid_len + 1);
+   param->ssid_len = info->ssid_len;
+   memset(param->rsn_pcip_policy, 0xFF, 3);
+   memset(param->rsn_auth_policy, 0xFF, 3);
+
+   while (index < ies_len) {
+   if (ies[index] == SUPP_RATES_IE) {
+   rates_no = ies[index + 1];
+   param->supp_rates[0] = rates_no;
index += 2;
 
-   for (i = 0; i < suppRatesNo; i++)
-   pNewJoinBssParam->supp_rates[i + 1] = 
pu8IEs[index + i];
+   for (i = 0; i < rates_no; i++)
+   param->supp_rates[i + 1] = ies[index + 
i];
 
-   index += suppRatesNo;
-   } else if (pu8IEs[index] == 

[PATCH 0/7] fixes to avoid use of cameCase

[Ajay Singh]

Patch set to fix "Avoid camelCase" issues found by checkpatch.pl.

Ajay Singh (7):
  staging: wilc1000: rename variables using camelCase in
host_int_ParseJoinBssParam()
  staging: wilc1000: rename Handle_DelAllSta() and its variable using
camelCase
  staging: wilc1000: rename strWIDList variable to avoid camelCase
  staging: wilc1000: rename u32WidsCount to avoid camelCase
  staging: wilc1000: rename pu8CurrByte variable to avoid camelCase
  staging: wilc1000: rename Handle_ScanDone function to avoid camelCase
  staging: wilc1000: rename Handle_Key() and Handle_ConnectTimeout()

 drivers/staging/wilc1000/host_interface.c | 726 +++---
 1 file changed, 363 insertions(+), 363 deletions(-)

-- 
2.7.4

Re: [PATCH v2 13/14] staging: wilc1000: rename Handle_Connect() to avoid camelCase

[Ajay Singh]

On Tue, 30 Jan 2018 17:40:31 +0300
Dan Carpenter  wrote:

> On Tue, Jan 30, 2018 at 07:29:49PM +0530, Ajay Singh wrote:
> > On Tue, 30 Jan 2018 02:13:53 +0800
> > kbuild test robot  wrote:
> >   
> 
> > 
> > 
> > The patch only change variable names to avoid the camelCase, didn't modify 
> > any extra code to dereference memory.  
> 
> You are responding to a robot and I think we all understood that this
> warning was there before you renamed the variables.
> 
Thanks, got it.

> > I think, with the use of shorter variable name now memcpy() is taking 1 
> > line instead of 3 lines. So, now line 937 has different code line(as code 
> > is swifted up by few lines).So because of that new potential NULL 
> > dereference error is popped up for same existing code.  
> > The code to validate dynamically allocated memory before access, will be 
> > include in separate patch to keep it segregated from variable names 
> > changes. 
> > I will rework on this patch and resend again.  
> 
> There is no need to re-work the patch.
> 

Few of the patches for patch series are accepted and submitted to "linux-next". 
For now, will submit the a new patch series by including the remaining patches 
(changing commit subject line to avoid confusion from previous patches). 

> You are right that the NULL check should be added as a separate check.
> 
> regards,
> dan carpenter
> 


Regards,
Ajay

[PATCH] NFC: fdp: fix signed less or equal zero check in u8 max_size

[Colin King]

From: Colin Ian King 

The u8 variable max_size is being assigned a return value from the
call to nci_conn_max_data_pkt_payload_size that can return a -ve
error return. Since max_size is a u8, the -ve check for the error will
always be false. Fix this by making max_size an int type.

Detected using Coccinelle:
drivers/nfc/fdp/fdp.c:208:5-13: WARNING: Unsigned expression compared
with zero: max_size <= 0

Fixes: a06347c04c13 ("NFC: Add Intel Fields Peak NFC solution driver")
Signed-off-by: Colin Ian King 
---
 drivers/nfc/fdp/fdp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/fdp/fdp.c b/drivers/nfc/fdp/fdp.c
index d5784a47fc13..5ddc01b4913a 100644
--- a/drivers/nfc/fdp/fdp.c
+++ b/drivers/nfc/fdp/fdp.c
@@ -192,8 +192,8 @@ static int fdp_nci_send_patch(struct nci_dev *ndev, u8 
conn_id, u8 type)
const struct firmware *fw;
struct sk_buff *skb;
unsigned long len;
-   u8 max_size, payload_size;
-   int rc = 0;
+   u8 payload_size;
+   int max_size, rc = 0;
 
if ((type == NCI_PATCH_TYPE_OTP && !info->otp_patch) ||
(type == NCI_PATCH_TYPE_RAM && !info->ram_patch))
-- 
2.15.1

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

Hi Brian,

> -Original Message-
> From: Brian Norris [mailto:briannor...@chromium.org]
> Sent: Tuesday, January 30, 2018 3:38 AM
> To: Ganapathi Bhat
> Cc: Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> Hi,
>
> On Sun, Jan 28, 2018 at 11:19 PM, Ganapathi Bhat 
> wrote:
> >> From: Ganapathi Bhat
> >> > From: Brian Norris [mailto:briannor...@chromium.org] On Thu, Jan
> >> > 25, 2018 at 09:59:04AM +, Ganapathi Bhat wrote:
> >> > > > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> >> > > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2]
> >> > > sent in this
> >> > series.
> >> >
> >> > What? Why would you introduce a bug and only fix it in the next patch?
> >> With the first patch the original issue took considerably longer time
> >> to recreate. Also it followed a different path to get
> >> recreated(shared in commit message).
> >> > Does that mean you should just combine the two?
> >> Let us know if that is fine to merge them. We can modify the commit
> >> message accordingly.
> >> > Or reverse the order, if patch 2 doesn't cause problems on its own?
> >> Patch 2 has a dependency on patch 1.
> > One correction. There is no commit dependency between patch 1 and
> 2(they can be applied in any order). But the result would be same. We need
> both fixes. Let us know if we can combine them.
>
> I haven't closely looked at patch 2 yet. My only statement was that it makes
> no sense to have 2 commits, with the second one labeled as "Fixing" the first
> one. From your descriptions, it sounds like patch 2 should actually come 
> first,
Ok. I understand. I will reorder them and send v3(along with spinlock change).
> but I'm not really sure. I only looked far enough to say that I didn't like 
> patch
> 1 as-is :)
>
> Brian
Regards,
Ganapathi

Re: [PATCH v2 13/14] staging: wilc1000: rename Handle_Connect() to avoid camelCase

[Dan Carpenter]

On Tue, Jan 30, 2018 at 07:29:49PM +0530, Ajay Singh wrote:
> On Tue, 30 Jan 2018 02:13:53 +0800
> kbuild test robot  wrote:
> 

> 
> 
> The patch only change variable names to avoid the camelCase, didn't modify 
> any extra code to dereference memory.

You are responding to a robot and I think we all understood that this
warning was there before you renamed the variables.

> I think, with the use of shorter variable name now memcpy() is taking 1 line 
> instead of 3 lines. So, now line 937 has different code line(as code is 
> swifted up by few lines).So because of that new potential NULL dereference 
> error is popped up for same existing code.  
> The code to validate dynamically allocated memory before access, will be 
> include in separate patch to keep it segregated from variable names changes. 
> I will rework on this patch and resend again.

There is no need to re-work the patch.

You are right that the NULL check should be added as a separate check.

regards,
dan carpenter

Re: [PATCH v2 13/14] staging: wilc1000: rename Handle_Connect() to avoid camelCase

[Dan Carpenter]

On Tue, Jan 30, 2018 at 02:13:53AM +0800, kbuild test robot wrote:
> Hi Ajay,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on staging/staging-testing]
> [cannot apply to v4.15 next-20180126]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Ajay-Singh/fix-to-remove-unnecessary-parenthesis-typedef-and-avoid-camelCase/20180123-115314
> 
> New smatch warnings:
> drivers/staging/wilc1000/host_interface.c:937 handle_connect() error: 
> potential null dereference 'hif_drv->usr_conn_req.ssid'.  (kmalloc returns 
> null)
> 

It's not really a new warning, it's just that we renamed variables.  But
that's fine because it's a real bug and we should fix it.

What I don't understand is that this warning was introduced in patch 9
so I don't know why the script is responding to patch 13.  I would have
expect it to reply to patch 9 (because that's where the warning is
introduced) or patch 0 or patch 14 (if it's testing the whole patchset
instead of individual patches).

regards,
dan carpenter

Re: [PATCH v2 13/14] staging: wilc1000: rename Handle_Connect() to avoid camelCase

[Ajay Singh]

On Tue, 30 Jan 2018 02:13:53 +0800
kbuild test robot  wrote:

> Hi Ajay,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on staging/staging-testing]
> [cannot apply to v4.15 next-20180126]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Ajay-Singh/fix-to-remove-unnecessary-parenthesis-typedef-and-avoid-camelCase/20180123-115314
> 
> New smatch warnings:
> drivers/staging/wilc1000/host_interface.c:937 handle_connect() error: 
> potential null dereference 'hif_drv->usr_conn_req.ssid'.  (kmalloc returns 
> null)
> 
> Old smatch warnings:
> drivers/staging/wilc1000/host_interface.c:514 handle_cfg_param() warn: always 
> true condition '(cfg_param_attr->auth_timeout < 65536) => (0-u16max < 65536)'
> drivers/staging/wilc1000/host_interface.c:583 handle_cfg_param() warn: always 
> true condition '(cfg_param_attr->rts_threshold < 65536) => (0-u16max < 65536)'
> drivers/staging/wilc1000/host_interface.c:636 handle_cfg_param() warn: always 
> true condition '(cfg_param_attr->beacon_interval < 65536) => (0-u16max < 
> 65536)'
> drivers/staging/wilc1000/host_interface.c:677 handle_cfg_param() warn: always 
> true condition '(cfg_param_attr->site_survey_scan_time < 65536) => (0-u16max 
> < 65536)'
> drivers/staging/wilc1000/host_interface.c:691 handle_cfg_param() warn: always 
> true condition '(cfg_param_attr->active_scan_time < 65536) => (0-u16max < 
> 65536)'
> drivers/staging/wilc1000/host_interface.c:705 handle_cfg_param() warn: always 
> true condition '(cfg_param_attr->passive_scan_time < 65536) => (0-u16max < 
> 65536)'
> 
> vim +937 drivers/staging/wilc1000/host_interface.c
> 
> c5c77ba1 Johnny Kim   2015-05-11   903  
> e554a305 Leo Kim  2015-11-19   904  u8 wilc_connected_ssid[6] = {0};
> cd1931cf Ajay Singh   2018-01-22   905  static s32 handle_connect(struct 
> wilc_vif *vif,
> 3891285c Ajay Singh   2018-01-22   906  struct 
> connect_attr *attr)
> c5c77ba1 Johnny Kim   2015-05-11   907  {
> 31390eec Leo Kim  2015-10-19   908s32 result = 0;
> 5a99cdf9 Ajay Singh   2018-01-22   909struct wid wid_list[8];
> 7046f41b Ajay Singh   2018-01-22   910u32 wid_cnt = 0, dummyval = 0;
> 44ea7461 Ajay Singh   2018-01-22   911u8 *cur_byte = NULL;
> 5e18dd82 Ajay Singh   2018-01-22   912struct join_bss_param 
> *bss_param;
> 71130e81 Glen Lee 2015-12-21   913struct host_if_drv *hif_drv = 
> vif->hif_drv;
> c5c77ba1 Johnny Kim   2015-05-11   914  
> 3891285c Ajay Singh   2018-01-22   915if (memcmp(attr->bssid, 
> wilc_connected_ssid, ETH_ALEN) == 0) {
> 31390eec Leo Kim  2015-10-19   916result = 0;
> b92f9304 Chris Park   2016-02-22   917netdev_err(vif->ndev, 
> "Discard connect request\n");
> 31390eec Leo Kim  2015-10-19   918return result;
> c5c77ba1 Johnny Kim   2015-05-11   919}
> c5c77ba1 Johnny Kim   2015-05-11   920  
> 5e18dd82 Ajay Singh   2018-01-22   921bss_param = attr->params;
> 5e18dd82 Ajay Singh   2018-01-22   922if (!bss_param) {
> b92f9304 Chris Park   2016-02-22   923netdev_err(vif->ndev, 
> "Required BSSID not found\n");
> 31390eec Leo Kim  2015-10-19   924result = -ENOENT;
> 24db713f Leo Kim  2015-09-16   925goto ERRORHANDLER;
> c5c77ba1 Johnny Kim   2015-05-11   926}
> c5c77ba1 Johnny Kim   2015-05-11   927  
> 3891285c Ajay Singh   2018-01-22   928if (attr->bssid) {
> 788f6fc0 Chaehyun Lim 2016-02-12   929
> hif_drv->usr_conn_req.bssid = kmalloc(6, GFP_KERNEL);
> 3891285c Ajay Singh   2018-01-22   930
> memcpy(hif_drv->usr_conn_req.bssid, attr->bssid, 6);
> c5c77ba1 Johnny Kim   2015-05-11   931}
> c5c77ba1 Johnny Kim   2015-05-11   932  
> 3891285c Ajay Singh   2018-01-22   933hif_drv->usr_conn_req.ssid_len 
> = attr->ssid_len;
> 3891285c Ajay Singh   2018-01-22   934if (attr->ssid) {
> 3891285c Ajay Singh   2018-01-22   935
> hif_drv->usr_conn_req.ssid = kmalloc(attr->ssid_len + 1, GFP_KERNEL);
> 3891285c Ajay Singh   2018-01-22   936
> memcpy(hif_drv->usr_conn_req.ssid, attr->ssid, attr->ssid_len);
> 3891285c Ajay Singh   2018-01-22  @937
> hif_drv->usr_conn_req.ssid[attr->ssid_len] = '\0';
> c5c77ba1 Johnny Kim   2015-05-11   938}
> c5c77ba1 Johnny Kim   2015-05-11   939  
> 3891285c Ajay Singh   2018-01-22   940hif_drv->usr_conn_req.ies_len = 
> attr->ies_len;
> 3891285c Ajay Singh   2018-01-22   941if (attr->ies) {
> 3891285c Ajay Singh   2018-01-22   942
> hif_drv->usr_conn_req.ies = kmalloc(attr->ies_len, GFP_KERNEL);
> 3891285c Ajay Singh   2018-01-22   943
> memcpy(hif_drv->usr_conn_req.ies, attr->ies, attr->ies_len);
> 

Re: [bug report] mt76: fix transmission of encrypted management frames

[Dan Carpenter]

On Tue, Jan 30, 2018 at 03:39:56PM +0300, Dan Carpenter wrote:
> Hello Felix Fietkau,
> 
> The patch 23405236460b: "mt76: fix transmission of encrypted
> management frames" from Jan 18, 2018, leads to the following static
> checker warning:
> 
>   drivers/net/wireless/mediatek/mt76/mt76x2_tx.c:41 mt76x2_tx()
>   warn: always true condition '(wcid->hw_key_idx != -1) => (0-255 != 
> (-1))'
> 
> drivers/net/wireless/mediatek/mt76/mt76x2_tx.c
> 26  void mt76x2_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control 
> *control,
> 27   struct sk_buff *skb)
> 28  {
> 29  struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> 30  struct mt76x2_dev *dev = hw->priv;
> 31  struct ieee80211_vif *vif = info->control.vif;
> 32  struct mt76_wcid *wcid = >global_wcid;
> 33  
> 34  if (control->sta) {
> 35  struct mt76x2_sta *msta;
> 36  
> 37  msta = (struct mt76x2_sta *) control->sta->drv_priv;
> 38  wcid = >wcid;
> 39  }
> 40  
> 41  if (vif || (!info->control.hw_key && wcid->hw_key_idx != -1)) 
> {
>  ^^
> We set ->hw_key_idx to -1 but it's a u8 so it gets truncated to 0xFF.
> This should probably be a define anyway.
> 
> 42  struct mt76x2_vif *mvif;
> 43  
> 44  mvif = (struct mt76x2_vif *) vif->drv_priv;
   ^
Oh, there is another static checker warning because of the "vif" check:

drivers/net/wireless/mediatek/mt76/mt76x2_tx.c:44 mt76x2_tx()
error: we previously assumed 'vif' could be null (see line 41)

Assume "vif" is NULL, and info->control.hw_key is zero and ->hw_key_idx
is not -1.

regards,
dan carpenter

[bug report] mt76: fix transmission of encrypted management frames

[Dan Carpenter]

Hello Felix Fietkau,

The patch 23405236460b: "mt76: fix transmission of encrypted
management frames" from Jan 18, 2018, leads to the following static
checker warning:

drivers/net/wireless/mediatek/mt76/mt76x2_tx.c:41 mt76x2_tx()
warn: always true condition '(wcid->hw_key_idx != -1) => (0-255 != 
(-1))'

drivers/net/wireless/mediatek/mt76/mt76x2_tx.c
26  void mt76x2_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control 
*control,
27   struct sk_buff *skb)
28  {
29  struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
30  struct mt76x2_dev *dev = hw->priv;
31  struct ieee80211_vif *vif = info->control.vif;
32  struct mt76_wcid *wcid = >global_wcid;
33  
34  if (control->sta) {
35  struct mt76x2_sta *msta;
36  
37  msta = (struct mt76x2_sta *) control->sta->drv_priv;
38  wcid = >wcid;
39  }
40  
41  if (vif || (!info->control.hw_key && wcid->hw_key_idx != -1)) {
 ^^
We set ->hw_key_idx to -1 but it's a u8 so it gets truncated to 0xFF.
This should probably be a define anyway.

42  struct mt76x2_vif *mvif;
43  
44  mvif = (struct mt76x2_vif *) vif->drv_priv;
45  wcid = >group_wcid;
46  }
47  
48  mt76_tx(>mt76, control->sta, wcid, skb);
49  }

regards,
dan carpenter

[PATCH] cfg80211: use only 1Mbps for basic rates in mesh

[Johannes Berg]

From: Johannes Berg 

Mesh used to use the mandatory rates as basic rates, but we got
the calculation of mandatory rates wrong until some time ago.
Fix this this broke interoperability with older versions since
now more basic rates are required, and thus the MBSS isn't the
same and the network stops working.

Fix this by simply using only 1Mbps as the basic rate in 2.4GHz.
Since the changed mandatory rates only affected 2.4GHz, this is
all we need to make it work again.

Reported-and-tested-by: Matthias Schiffer 
Fixes: 1bd773c077de ("wireless: set correct mandatory rate flags")
Signed-off-by: Johannes Berg 
---
 net/wireless/mesh.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index 51aa55618ef7..b12da6ef3c12 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -170,9 +170,28 @@ int __cfg80211_join_mesh(struct cfg80211_registered_device 
*rdev,
enum nl80211_bss_scan_width scan_width;
struct ieee80211_supported_band *sband =
rdev->wiphy.bands[setup->chandef.chan->band];
-   scan_width = cfg80211_chandef_to_scan_width(>chandef);
-   setup->basic_rates = ieee80211_mandatory_rates(sband,
-  scan_width);
+
+   if (setup->chandef.chan->band == NL80211_BAND_2GHZ) {
+   int i;
+
+   /*
+* Older versions selected the mandatory rates for
+* 2.4 GHz as well, but were broken in that only
+* 1 Mbps was regarded as a mandatory rate. Keep
+* using just 1 Mbps as the default basic rate for
+* mesh to be interoperable with older versions.
+*/
+   for (i = 0; i < sband->n_bitrates; i++) {
+   if (sband->bitrates[i].bitrate == 10) {
+   setup->basic_rates = BIT(i);
+   break;
+   }
+   }
+   } else {
+   scan_width = 
cfg80211_chandef_to_scan_width(>chandef);
+   setup->basic_rates = ieee80211_mandatory_rates(sband,
+  
scan_width);
+   }
}
 
err = cfg80211_chandef_dfs_required(>wiphy,
-- 
2.15.1

Re: ath9k will not tx packets sometimes.

[Toke Høiland-Jørgensen]

Ben Greear  writes:

> Maybe there is some way for the scheduler to get stuck and not
> schedule anything?

It would appear so, yeah. Do you do anything special other than
associate a whole bunch of stations to trigger this? I can try to see if
I can script something that works equivalently on my setup.

I'm actually working on reworking that whole scheduler logic, and move
some of it into mac80211. Could you test this (WiP) patch and see if
that has the same problem?

-Toke

mac80211: Add TXQ scheduling API

From: Toke Høiland-Jørgensen 

This adds an API to mac80211 to handle scheduling of TXQs and changes the
interface between driver and mac80211 for TXQ handling as follows:

- The wake_tx_queue callback interface no longer includes the TXQ. Instead,
  the driver is expected to retrieve that from ieee80211_next_txq()

- Two new mac80211 functions are added: ieee80211_next_txq() and
  ieee80211_schedule_txq(). The former returns the next TXQ that should be
  scheduled, and is how the driver gets a queue to pull packets from. The
  latter is called internally by mac80211 to start scheduling a queue, and
  the driver is supposed to call it to re-schedule the TXQ after it is
  finished pulling packets from it (unless the queue emptied).

The ath9k and ath10k drivers are changed to use the new API.

Signed-off-by: Toke Høiland-Jørgensen 
---
 drivers/net/wireless/ath/ath10k/core.c |2 
 drivers/net/wireless/ath/ath10k/core.h |4 -
 drivers/net/wireless/ath/ath10k/mac.c  |   55 +++--
 drivers/net/wireless/ath/ath9k/ath9k.h |4 -
 drivers/net/wireless/ath/ath9k/recv.c  |2 
 drivers/net/wireless/ath/ath9k/xmit.c  |  192 +---
 include/net/mac80211.h |   38 +-
 net/mac80211/agg-tx.c  |6 +
 net/mac80211/driver-ops.h  |   12 +-
 net/mac80211/ieee80211_i.h |8 +
 net/mac80211/main.c|3 +
 net/mac80211/sta_info.c|3 -
 net/mac80211/trace.h   |   19 +--
 net/mac80211/tx.c  |   57 +-
 14 files changed, 187 insertions(+), 218 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 6d065f8d7f78..042175a5653f 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2655,9 +2655,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 
 	mutex_init(>conf_mutex);
 	spin_lock_init(>data_lock);
-	spin_lock_init(>txqs_lock);
 
-	INIT_LIST_HEAD(>txqs);
 	INIT_LIST_HEAD(>peers);
 	init_waitqueue_head(>peer_mapping_wq);
 	init_waitqueue_head(>htt.empty_tx_wq);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 631df2137e25..398cd3c57139 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -346,7 +346,6 @@ struct ath10k_peer {
 };
 
 struct ath10k_txq {
-	struct list_head list;
 	unsigned long num_fw_queued;
 	unsigned long num_push_allowed;
 };
@@ -896,10 +895,7 @@ struct ath10k {
 
 	/* protects shared structure data */
 	spinlock_t data_lock;
-	/* protects: ar->txqs, artxq->list */
-	spinlock_t txqs_lock;
 
-	struct list_head txqs;
 	struct list_head arvifs;
 	struct list_head peers;
 	struct ath10k_peer *peer_map[ATH10K_MAX_NUM_PEER_IDS];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 75726f12a31c..e5dd97da3fa1 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3832,12 +3832,10 @@ static void ath10k_mac_txq_init(struct ieee80211_txq *txq)
 		return;
 
 	artxq = (void *)txq->drv_priv;
-	INIT_LIST_HEAD(>list);
 }
 
 static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
 {
-	struct ath10k_txq *artxq;
 	struct ath10k_skb_cb *cb;
 	struct sk_buff *msdu;
 	int msdu_id;
@@ -3845,12 +3843,6 @@ static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
 	if (!txq)
 		return;
 
-	artxq = (void *)txq->drv_priv;
-	spin_lock_bh(>txqs_lock);
-	if (!list_empty(>list))
-		list_del_init(>list);
-	spin_unlock_bh(>txqs_lock);
-
 	spin_lock_bh(>htt.tx_lock);
 	idr_for_each_entry(>htt.pending_tx, msdu, msdu_id) {
 		cb = ATH10K_SKB_CB(msdu);
@@ -3980,23 +3972,17 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
 void ath10k_mac_tx_push_pending(struct ath10k *ar)
 {
 	struct ieee80211_hw *hw = ar->hw;
-	struct ieee80211_txq *txq;
-	struct ath10k_txq *artxq;
-	struct ath10k_txq *last;
+	struct ieee80211_txq *txq, *first = NULL;
 	int ret;
 	int max;
 
 	if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
 		return;
 
-	spin_lock_bh(>txqs_lock);
 	rcu_read_lock();
 
-	last = list_last_entry(>txqs, struct ath10k_txq, list);
-	while (!list_empty(>txqs)) {
-		artxq = list_first_entry(>txqs, struct ath10k_txq, list);
-		txq = 

Re: [PATCH] brcmfmac: detect & reject faked packet generated by a firmware

[Arend van Spriel]

On 1/30/2018 10:09 AM, Rafał Miłecki wrote:

From: Rafał Miłecki 

When using 4366b1 and 4366c0 chipsets with more recent firmwares
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
respectively, it is impossible to use brcmfmac with interface in AP
mode. With the AP interface bridged and multicast used, no STA will be
able to associate; the STA will be immediately disassociated when
attempting to associate.

Debugging revealed this to be caused by a "faked" packet (generated by
firmware), that is passed to the networking subsystem and then back to
the firmware. Fortunately this packet is easily identified and can be
detected and ignored as a workaround for misbehaving firmware.


I am actually wondering what this packet is. Have you checked in 
brcmf_msgbuf_process_rx_complete(). I am curious what buflen is there 
and what eth_type_trans() will do to the packet, ie. what protocol. As 
everything should be 802.3 we could/should add a length check of 14 bytes.


Regards,
Arend


Signed-off-by: Rafał Miłecki 
---
  .../wireless/broadcom/brcm80211/brcmfmac/core.c| 46 ++
  1 file changed, 46 insertions(+)

Re: [PATCH] brcmfmac: detect & reject faked packet generated by a firmware

[Arend van Spriel]

On 1/30/2018 10:09 AM, Rafał Miłecki wrote:

From: Rafał Miłecki 

When using 4366b1 and 4366c0 chipsets with more recent firmwares
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
respectively, it is impossible to use brcmfmac with interface in AP
mode. With the AP interface bridged and multicast used, no STA will be
able to associate; the STA will be immediately disassociated when
attempting to associate.

Debugging revealed this to be caused by a "faked" packet (generated by
firmware), that is passed to the networking subsystem and then back to
the firmware. Fortunately this packet is easily identified and can be
detected and ignored as a workaround for misbehaving firmware.

Signed-off-by: Rafał Miłecki 
---
  .../wireless/broadcom/brcm80211/brcmfmac/core.c| 46 ++
  1 file changed, 46 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 930e423f83a8..a98ba9bbc7fe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -323,8 +323,54 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
spin_unlock_irqrestore(>netif_stop_lock, flags);
  }

+/**
+ * brcmf_is_valid_skb - validates skb received from the hardware
+ *
+ * @skb: skb to check
+ *
+ * Sometimes firmware/hardware can generate broken packets that aren't real or
+ * valid and their skb-s shouldn't be passed up to the networking subsystem.
+ *
+ * Firmwares for 43602a1, 4366b1 and 4366c0 are known to *generate* a faked 6 B
+ * packet whenever a STA associates. The purpose of this fake packet remains
+ * unknown but it is clearly not data coming from a station. As such it
+ * shouldn't be passed to the networking subsystem.
+ *
+ * Normally such a packet would simply be ignored, but this is not the case 
with
+ * more recent 4366b1 and 4366c0 firmwares. These firmwares seem to explicitly
+ * check for this packet and will reject (disassociate) the station, making it
+ * impossible to connect to the AP at all. This can happen when using a bridged
+ * interface with multicasting. Such a scenario apparently isn't tested (or
+ * supported) by Broadcom's internal team.
+ */
+static bool brcmf_is_valid_skb(struct sk_buff *skb)
+{
+   const u8 fw_faked_packet[6] __aligned(2) = {
+   0x00, 0x01, 0xaf, 0x81, 0x01, 0x00,
+   };
+#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+   const u16 *a = (const u16 *)skb->data;
+   const u16 *b = (const u16 *)fw_faked_packet;
+#endif
+
+   if (skb->len != 6)
+   return true;
+
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+   return !!(((*(const u32 *)skb->data) ^ (*(const u32 *)fw_faked_packet)) 
|
+ ((*(const u16 *)(skb->data + 4)) ^ (*(const u16 
*)(fw_faked_packet + 4;
+#else
+   return !!((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2]));
+#endif
+}


The code above does look very much like ether_addr_equal(). Why not use 
that instead of reinventing it.



  void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
  {
+   if (!brcmf_is_valid_skb(skb)) {
+   brcmu_pkt_buf_free_skb(skb);


Maybe we should add a driver stat for this although I better have a look 
into the root cause of this.


Regards,
Arend

Re: [PATCH 1/2] wireless: set correct mandatory rate flags

[Matthias Schiffer]

On 01/30/2018 08:43 AM, Johannes Berg wrote:
> On Fri, 2018-01-26 at 23:17 +0100, Matthias Schiffer wrote:
>>
>> I propose to revert this for now (I assume it's too late for 4.15, but
>> hopefully the regression can be fixed in 4.15.1).
> 
> I really don't think we should revert this, it fixes a real bug.
> 
> We can easily switch the default though, would something like this
> help?

Working perfectly.

Tested-by: Matthias Schiffer 


> 
> diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
> index 51aa55618ef7..b12da6ef3c12 100644
> --- a/net/wireless/mesh.c
> +++ b/net/wireless/mesh.c
> @@ -170,9 +170,28 @@ int __cfg80211_join_mesh(struct 
> cfg80211_registered_device *rdev,
>   enum nl80211_bss_scan_width scan_width;
>   struct ieee80211_supported_band *sband =
>   rdev->wiphy.bands[setup->chandef.chan->band];
> - scan_width = cfg80211_chandef_to_scan_width(>chandef);
> - setup->basic_rates = ieee80211_mandatory_rates(sband,
> -scan_width);
> +
> + if (setup->chandef.chan->band == NL80211_BAND_2GHZ) {
> + int i;
> +
> + /*
> +  * Older versions selected the mandatory rates for
> +  * 2.4 GHz as well, but were broken in that only
> +  * 1 Mbps was regarded as a mandatory rate. Keep
> +  * using just 1 Mbps as the default basic rate for
> +  * mesh to be interoperable with older versions.
> +  */
> + for (i = 0; i < sband->n_bitrates; i++) {
> + if (sband->bitrates[i].bitrate == 10) {
> + setup->basic_rates = BIT(i);
> + break;
> + }
> + }
> + } else {
> + scan_width = 
> cfg80211_chandef_to_scan_width(>chandef);
> + setup->basic_rates = ieee80211_mandatory_rates(sband,
> +
> scan_width);
> + }
>   }
>  
>   err = cfg80211_chandef_dfs_required(>wiphy,
> 
> johannes
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 2/3] mac80211_hwsim: add nl_err_msg in hwsim_new_radio in netlink case

[Benjamin Beichler]

Am 30.01.2018 um 08:56 schrieb Johannes Berg:
> On Mon, 2018-01-22 at 18:04 +0100, Benjamin Beichler wrote:
>> +if(!info)
>> +pr_debug("mac80211_hwsim: radio index %d already 
>> present\n",
>> + idx);
> Can this really trigger? I think I'll edit it out - we do pass
> info==NULL in the case of module init, but in that case nothing else
> can interfere and we can't even really fail.
It should not be triggered because of already existing macs, I added it
for somehow completeness, but you can leave it out.

After having again a look on it, there could be an -ENOMEM on
rhashtable_insert. Maybe it's better to explicitly catch only -EEXIST
and print for the rest an generic error. Do you agree ?

> johannes
>

-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: benjamin.beich...@uni-rostock.de
www: http://www.imd.uni-rostock.de/




smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH] brcmfmac: detect & reject faked packet generated by a firmware

[Rafał Miłecki]

From: Rafał Miłecki 

When using 4366b1 and 4366c0 chipsets with more recent firmwares
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
respectively, it is impossible to use brcmfmac with interface in AP
mode. With the AP interface bridged and multicast used, no STA will be
able to associate; the STA will be immediately disassociated when
attempting to associate.

Debugging revealed this to be caused by a "faked" packet (generated by
firmware), that is passed to the networking subsystem and then back to
the firmware. Fortunately this packet is easily identified and can be
detected and ignored as a workaround for misbehaving firmware.

Signed-off-by: Rafał Miłecki 
---
 .../wireless/broadcom/brcm80211/brcmfmac/core.c| 46 ++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 930e423f83a8..a98ba9bbc7fe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -323,8 +323,54 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
spin_unlock_irqrestore(>netif_stop_lock, flags);
 }
 
+/**
+ * brcmf_is_valid_skb - validates skb received from the hardware
+ *
+ * @skb: skb to check
+ *
+ * Sometimes firmware/hardware can generate broken packets that aren't real or
+ * valid and their skb-s shouldn't be passed up to the networking subsystem.
+ *
+ * Firmwares for 43602a1, 4366b1 and 4366c0 are known to *generate* a faked 6 B
+ * packet whenever a STA associates. The purpose of this fake packet remains
+ * unknown but it is clearly not data coming from a station. As such it
+ * shouldn't be passed to the networking subsystem.
+ *
+ * Normally such a packet would simply be ignored, but this is not the case 
with
+ * more recent 4366b1 and 4366c0 firmwares. These firmwares seem to explicitly
+ * check for this packet and will reject (disassociate) the station, making it
+ * impossible to connect to the AP at all. This can happen when using a bridged
+ * interface with multicasting. Such a scenario apparently isn't tested (or
+ * supported) by Broadcom's internal team.
+ */
+static bool brcmf_is_valid_skb(struct sk_buff *skb)
+{
+   const u8 fw_faked_packet[6] __aligned(2) = {
+   0x00, 0x01, 0xaf, 0x81, 0x01, 0x00,
+   };
+#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+   const u16 *a = (const u16 *)skb->data;
+   const u16 *b = (const u16 *)fw_faked_packet;
+#endif
+
+   if (skb->len != 6)
+   return true;
+
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+   return !!(((*(const u32 *)skb->data) ^ (*(const u32 *)fw_faked_packet)) 
|
+ ((*(const u16 *)(skb->data + 4)) ^ (*(const u16 
*)(fw_faked_packet + 4;
+#else
+   return !!((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2]));
+#endif
+}
+
 void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
 {
+   if (!brcmf_is_valid_skb(skb)) {
+   brcmu_pkt_buf_free_skb(skb);
+   return;
+   }
+
if (skb->pkt_type == PACKET_MULTICAST)
ifp->ndev->stats.multicast++;
 
-- 
2.11.0