Re: [mt76/mt7603/mac] Question about missing variable assignment

2019-03-03 Thread Gustavo A. R. Silva



On 3/3/19 4:05 AM, Felix Fietkau wrote:
> On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> The following piece of code in 
>> drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> is missing a variable assignment before line 1058.  Notice that there
>> is a potential execution path in which variable *i* is compared against
>> magic number 15 at line 1075 without being initialized previously
>> (this was reported by Coverity):
>>
>> 1055 out:
>> 1056 final_rate_flags = info->status.rates[final_idx].flags;
>> 1057 
>> 1058 switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
>> 1059 case MT_PHY_TYPE_CCK:
>> 1060 cck = true;
>> 1061 /* fall through */
>> 1062 case MT_PHY_TYPE_OFDM:
>> 1063 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
>> 1064 sband = &dev->mt76.sband_5g.sband;
>> 1065 else
>> 1066 sband = &dev->mt76.sband_2g.sband;
>> 1067 final_rate &= GENMASK(5, 0);
>> 1068 final_rate = mt7603_get_rate(dev, sband, final_rate, 
>> cck);
>> 1069 final_rate_flags = 0;
>> 1070 break;
>> 1071 case MT_PHY_TYPE_HT_GF:
>> 1072 case MT_PHY_TYPE_HT:
>> 1073 final_rate_flags |= IEEE80211_TX_RC_MCS;
>> 1074 final_rate &= GENMASK(5, 0);
>> 1075 if (i > 15)
>> 1076 return false;
>> 1077 break;
>> 1078 default:
>> 1079 return false;
>> 1080 }
>>
>> My guess is that such missing assignment should be something similar
>> to the one at line 566:
>>
>>  i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
>>
>> but I'm not sure what the proper arguments for macro FIELD_GET should
>> be.
>>
>> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
>>
>> What do you think?
> Thanks for reporting this. The fix is simpler than that, the check
> should be: if (final_rate > 15)
> I will send a fix.
> 

Great. Glad to help. :)

Thanks
--
Gustavo


[mt76/mt7603/mac] Question about missing variable assignment

2019-03-02 Thread Gustavo A. R. Silva
Hi all,

The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
is missing a variable assignment before line 1058.  Notice that there
is a potential execution path in which variable *i* is compared against
magic number 15 at line 1075 without being initialized previously
(this was reported by Coverity):

1055 out:
1056 final_rate_flags = info->status.rates[final_idx].flags;
1057 
1058 switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
1059 case MT_PHY_TYPE_CCK:
1060 cck = true;
1061 /* fall through */
1062 case MT_PHY_TYPE_OFDM:
1063 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
1064 sband = &dev->mt76.sband_5g.sband;
1065 else
1066 sband = &dev->mt76.sband_2g.sband;
1067 final_rate &= GENMASK(5, 0);
1068 final_rate = mt7603_get_rate(dev, sband, final_rate, cck);
1069 final_rate_flags = 0;
1070 break;
1071 case MT_PHY_TYPE_HT_GF:
1072 case MT_PHY_TYPE_HT:
1073 final_rate_flags |= IEEE80211_TX_RC_MCS;
1074 final_rate &= GENMASK(5, 0);
1075 if (i > 15)
1076 return false;
1077 break;
1078 default:
1079 return false;
1080 }

My guess is that such missing assignment should be something similar
to the one at line 566:

i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);

but I'm not sure what the proper arguments for macro FIELD_GET should
be.

This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe

What do you think?

Thanks
--
Gustavo



Re: -Wimplicit-fallthrough not working with ccache

2019-02-18 Thread Gustavo A. R. Silva
Hi Kalle,

On 2/16/19 5:21 AM, Kalle Valo wrote:
> (replying to an old thread but renaming it)
> 
> Kalle Valo  writes:
> 
>> "Gustavo A. R. Silva"  wrote:
>>
>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>> where we are expecting to fall through.
>>>
>>> Notice that in this particular case, I replaced "pass through" with
>>> a proper "fall through" comment, which is what GCC is expecting
>>> to find.
>>>
>>> Signed-off-by: Gustavo A. R. Silva 
>>> Signed-off-by: Kalle Valo 
>>
>> Patch applied to ath-next branch of ath.git, thanks.
>>
>> f1d270ae10ff ath10k: htt_tx: mark expected switch fall-throughs
> 
> Gustavo, I enabled W=1 on my ath10k build checks and it took me a while
> to figure out why GCC was warning about fall through annotations missing
> even I knew you had fixed them. Finally I figured out that the reason
> was ccache, which I need because I work with different branches and need
> to recompile the kernel quite often.
> 
> If the plan is to enable -Wimplicit-fallthrough by default in the kernel
> IMHO this might become an issue, as otherwise people using ccache start
> seeing lots of invalid warnings. Apparently CCACHE_COMMENTS=1 will fix
> that but my version of ccache doesn't support it, and how would everyone
> learn that trick anyway? Or maybe CCACHE_COMMENTS can enabled through
> kernel Makefile?
> 

Can you share with me the warning messages you get?

I just see the following warnings with linux-next:

$ make CC="ccache gcc" W=1 drivers/net/wireless/ath/ath10k/htt_tx.o
  CC [M]  drivers/net/wireless/ath/ath10k/htt_tx.o
In file included from drivers/net/wireless/ath/ath10k/htt_tx.c:19:
drivers/net/wireless/ath/ath10k/htt.h:1727:1: warning: alignment 1 of ‘struct 
ath10k_htt_txbuf_32’ is less than 4 [-Wpacked-not-aligned]
 } __packed;
 ^
drivers/net/wireless/ath/ath10k/htt.h:1734:1: warning: alignment 1 of ‘struct 
ath10k_htt_txbuf_64’ is less than 4 [-Wpacked-not-aligned]
 } __packed;
 ^

In my Makefile I have:

KBUILD_CFLAGS  += $(call cc-option,-Wimplicit-fallthrough=3,)


Thanks
--
Gustavo



















[PATCH] NFC: pn533: mark expected switch fall-throughs

2019-02-13 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/nfc/pn533/pn533.c: In function ‘pn533_transceive’:
drivers/nfc/pn533/pn533.c:2142:6: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   if (dev->tgt_active_prot == NFC_PROTO_FELICA) {
  ^
drivers/nfc/pn533/pn533.c:2150:2: note: here
  default:
  ^~~
drivers/nfc/pn533/pn533.c: In function ‘pn533_wq_mi_recv’:
drivers/nfc/pn533/pn533.c:2267:6: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   if (dev->tgt_active_prot == NFC_PROTO_FELICA) {
  ^
drivers/nfc/pn533/pn533.c:2276:2: note: here
  default:
  ^~~

Warning level 3 was used: -Wimplicit-fallthrough=3

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Addresses-Coverity-ID: 1230487 ("Missing break in switch")
Addresses-Coverity-ID: 1230488 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/nfc/pn533/pn533.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a0cc1cc45292..5961f14259e5 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2147,6 +2147,7 @@ static int pn533_transceive(struct nfc_dev *nfc_dev,
 
break;
}
+   /* fall through */
default:
/* jumbo frame ? */
if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) {
@@ -2273,6 +2274,7 @@ static void pn533_wq_mi_recv(struct work_struct *work)
 
break;
}
+   /* fall through */
default:
skb_put_u8(skb, 1); /*TG*/
 
-- 
2.20.1



[PATCH] NFC: st21nfca: Fix a couple of fall-through warnings

2019-02-12 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings by adding a missing break
and a fall-through annotation:

drivers/nfc/st21nfca/dep.c: In function ‘st21nfca_tm_event_send_data’:
drivers/nfc/st21nfca/dep.c:391:3: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   switch (cmd1) {
   ^~
drivers/nfc/st21nfca/dep.c:404:2: note: here
  default:
  ^~~
In file included from ./include/linux/kernel.h:15,
 from ./include/linux/skbuff.h:17,
 from ./include/net/nfc/hci.h:21,
 from drivers/nfc/st21nfca/dep.c:17:
drivers/nfc/st21nfca/dep.c: In function ‘st21nfca_im_recv_dep_res_cb’:
./include/linux/printk.h:303:2: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
  printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
  ^~~
drivers/nfc/st21nfca/dep.c:622:4: note: in expansion of macro ‘pr_err’
pr_err("Received a ACK/NACK PDU\n");
^~
drivers/nfc/st21nfca/dep.c:623:3: note: here
   case ST21NFCA_NFC_DEP_PFB_I_PDU:
   ^~~~

Warning level 3 was used: -Wimplicit-fallthrough=3

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/nfc/st21nfca/dep.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nfc/st21nfca/dep.c b/drivers/nfc/st21nfca/dep.c
index 3420c5104c94..60f013063f80 100644
--- a/drivers/nfc/st21nfca/dep.c
+++ b/drivers/nfc/st21nfca/dep.c
@@ -401,6 +401,7 @@ static int st21nfca_tm_event_send_data(struct nfc_hci_dev 
*hdev,
default:
return 1;
}
+   break;
default:
return 1;
}
@@ -620,6 +621,7 @@ static void st21nfca_im_recv_dep_res_cb(void *context, 
struct sk_buff *skb,
switch (ST21NFCA_NFC_DEP_PFB_TYPE(dep_res->pfb)) {
case ST21NFCA_NFC_DEP_PFB_ACK_NACK_PDU:
pr_err("Received a ACK/NACK PDU\n");
+   /* fall through */
case ST21NFCA_NFC_DEP_PFB_I_PDU:
info->dep_info.curr_nfc_dep_pni =
ST21NFCA_NFC_DEP_PFB_PNI(dep_res->pfb + 1);
-- 
2.20.1



Re: [PATCH] iwlwifi: eeprom-parse: use struct_size() in kzalloc()

2019-01-29 Thread Gustavo A. R. Silva



On 1/22/19 4:58 AM, Luciano Coelho wrote:
> On Tue, 2019-01-08 at 11:17 -0600, Gustavo A. R. Silva wrote:
>> One of the more common cases of allocation size calculations is
>> finding the
>> size of a structure that has a zero-sized array at the end, along
>> with memory
>> for some number of elements for that array. For example:
>>
>> struct foo {
>> int stuff;
>> void *entry[];
>> };
>>
>> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count,
>> GFP_KERNEL);
>>
>> Instead of leaving these open-coded and prone to type mistakes, we
>> can now
>> use the new struct_size() helper:
>>
>> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>>
>> This code was detected with the help of Coccinelle.
>>
>> Signed-off-by: Gustavo A. R. Silva 
>> ---
> 
> Applied to our internal tree, thanks! It will reach the mainline
> following our normal upstreaming process.
> 

Great. :)

Thanks, Luca.

--
Gustavo


[PATCH][next] cfg80211: mark expected switch fall-throughs

2019-01-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

This patch fixes the following warnings:

net/wireless/wext-compat.c:1327:6: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
net/wireless/wext-compat.c:1341:6: warning: this statement may fall through 
[-Wimplicit-fallthrough=]

Warning level 3 was used: -Wimplicit-fallthrough=3

This patch is part of the ongoing efforts to enabling
-Wimplicit-fallthrough

Signed-off-by: Gustavo A. R. Silva 
---
 net/wireless/wext-compat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 06943d9c9835..d522787c7354 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -1337,6 +1337,7 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
wstats.qual.qual = sig + 110;
break;
}
+   /* fall through */
case CFG80211_SIGNAL_TYPE_UNSPEC:
if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_SIGNAL)) {
wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
@@ -1345,6 +1346,7 @@ static struct iw_statistics 
*cfg80211_wireless_stats(struct net_device *dev)
wstats.qual.qual = sinfo.signal;
break;
}
+   /* fall through */
default:
wstats.qual.updated |= IW_QUAL_LEVEL_INVALID;
wstats.qual.updated |= IW_QUAL_QUAL_INVALID;
-- 
2.20.1



[PATCH][next] iwlwifi: mvm: use struct_size() in kzalloc()

2019-01-15 Thread Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding the
size of a structure that has a zero-sized array at the end, along with memory
for some number of elements for that array. For example:

struct foo {
int stuff;
struct boo entry[];
};

instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
index d9afedc3d1d9..a2b2bdc5cd09 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
@@ -947,14 +947,12 @@ iwl_parse_nvm_data(struct iwl_trans *trans, const struct 
iwl_cfg *cfg,
const __le16 *ch_section;
 
if (cfg->nvm_type != IWL_NVM_EXT)
-   data = kzalloc(sizeof(*data) +
-  sizeof(struct ieee80211_channel) *
-  IWL_NVM_NUM_CHANNELS,
+   data = kzalloc(struct_size(data, channels,
+  IWL_NVM_NUM_CHANNELS),
   GFP_KERNEL);
else
-   data = kzalloc(sizeof(*data) +
-  sizeof(struct ieee80211_channel) *
-  IWL_NVM_NUM_CHANNELS_EXT,
+   data = kzalloc(struct_size(data, channels,
+  IWL_NVM_NUM_CHANNELS_EXT),
   GFP_KERNEL);
if (!data)
return NULL;
@@ -1444,9 +1442,7 @@ struct iwl_nvm_data *iwl_get_nvm(struct iwl_trans *trans,
if (empty_otp)
IWL_INFO(trans, "OTP is empty\n");
 
-   nvm = kzalloc(sizeof(*nvm) +
- sizeof(struct ieee80211_channel) * IWL_NUM_CHANNELS,
- GFP_KERNEL);
+   nvm = kzalloc(struct_size(nvm, channels, IWL_NUM_CHANNELS), GFP_KERNEL);
if (!nvm) {
ret = -ENOMEM;
goto out;
-- 
2.20.1



[PATCH] iwlwifi: nvm-parse: use struct_size() in kzalloc()

2019-01-08 Thread Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding the
size of a structure that has a zero-sized array at the end, along with memory
for some number of elements for that array. For example:

struct foo {
int stuff;
void *entry[];
};

instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 .../net/wireless/intel/iwlwifi/iwl-nvm-parse.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
index d9afedc3d1d9..8c720b42dc36 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
@@ -947,15 +947,13 @@ iwl_parse_nvm_data(struct iwl_trans *trans, const struct 
iwl_cfg *cfg,
const __le16 *ch_section;
 
if (cfg->nvm_type != IWL_NVM_EXT)
-   data = kzalloc(sizeof(*data) +
-  sizeof(struct ieee80211_channel) *
-  IWL_NVM_NUM_CHANNELS,
-  GFP_KERNEL);
+   data = kzalloc(struct_size(data, channels,
+  IWL_NVM_NUM_CHANNELS),
+  GFP_KERNEL);
else
-   data = kzalloc(sizeof(*data) +
-  sizeof(struct ieee80211_channel) *
-  IWL_NVM_NUM_CHANNELS_EXT,
-  GFP_KERNEL);
+   data = kzalloc(struct_size(data, channels,
+  IWL_NVM_NUM_CHANNELS_EXT),
+  GFP_KERNEL);
if (!data)
return NULL;
 
@@ -1444,9 +1442,7 @@ struct iwl_nvm_data *iwl_get_nvm(struct iwl_trans *trans,
if (empty_otp)
IWL_INFO(trans, "OTP is empty\n");
 
-   nvm = kzalloc(sizeof(*nvm) +
- sizeof(struct ieee80211_channel) * IWL_NUM_CHANNELS,
- GFP_KERNEL);
+   nvm = kzalloc(struct_size(nvm, channels, IWL_NUM_CHANNELS), GFP_KERNEL);
if (!nvm) {
ret = -ENOMEM;
goto out;
-- 
2.20.1



[PATCH] iwlwifi: eeprom-parse: use struct_size() in kzalloc()

2019-01-08 Thread Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding the
size of a structure that has a zero-sized array at the end, along with memory
for some number of elements for that array. For example:

struct foo {
int stuff;
void *entry[];
};

instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c
index 75940ac406b9..04338c3a6205 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c
@@ -850,8 +850,7 @@ iwl_parse_eeprom_data(struct device *dev, const struct 
iwl_cfg *cfg,
if (WARN_ON(!cfg || !cfg->eeprom_params))
return NULL;
 
-   data = kzalloc(sizeof(*data) +
-  sizeof(struct ieee80211_channel) * IWL_NUM_CHANNELS,
+   data = kzalloc(struct_size(data, channels, IWL_NUM_CHANNELS),
   GFP_KERNEL);
if (!data)
return NULL;
-- 
2.20.1



[PATCH] qtnfmac: use struct_size() in kzalloc()

2019-01-08 Thread Gustavo A. R. Silva
One of the more common cases of allocation size calculations is finding the
size of a structure that has a zero-sized array at the end, along with memory
for some number of elements for that array. For example:

struct foo {
int stuff;
void *entry[];
};

instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c 
b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 659e7649fe22..cf386f579060 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -914,9 +914,8 @@ qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus,
if (WARN_ON(resp->n_reg_rules > NL80211_MAX_SUPP_REG_RULES))
return -E2BIG;
 
-   hwinfo->rd = kzalloc(sizeof(*hwinfo->rd)
-+ sizeof(struct ieee80211_reg_rule)
-* resp->n_reg_rules, GFP_KERNEL);
+   hwinfo->rd = kzalloc(struct_size(hwinfo->rd, reg_rules,
+resp->n_reg_rules), GFP_KERNEL);
 
if (!hwinfo->rd)
return -ENOMEM;
-- 
2.20.1



Re: [PATCH] nfc: af_nfc: Fix Spectre v1 vulnerability

2018-12-22 Thread Gustavo A. R. Silva




On 12/22/18 8:42 PM, David Miller wrote:

From: "Gustavo A. R. Silva" 
Date: Sat, 22 Dec 2018 17:37:35 -0600


I wonder if you can take this one too:

https://lore.kernel.org/lkml/20181221212229.GA32635@embeddedor/

It's pretty similar to the af_nfc one.


Sure, done.



Great. Thanks.
--
Gustavo


Re: [PATCH] nfc: af_nfc: Fix Spectre v1 vulnerability

2018-12-22 Thread Gustavo A. R. Silva




On 12/22/18 5:09 PM, David Miller wrote:

From: "Gustavo A. R. Silva" 
Date: Fri, 21 Dec 2018 15:47:53 -0600


proto is indirectly controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.

This issue was detected with the help of Smatch:

net/nfc/af_nfc.c:42 nfc_sock_create() warn: potential spectre issue 'proto_tab' 
[w] (local cap)

Fix this by sanitizing proto before using it to index proto_tab.

Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].

[1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2

Signed-off-by: Gustavo A. R. Silva 


I'll take this directly, and queued up for -stable.



Dave,

I wonder if you can take this one too:

https://lore.kernel.org/lkml/20181221212229.GA32635@embeddedor/

It's pretty similar to the af_nfc one.

Thanks
--
Gustavo


[PATCH] nfc: af_nfc: Fix Spectre v1 vulnerability

2018-12-21 Thread Gustavo A. R. Silva
proto is indirectly controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.

This issue was detected with the help of Smatch:

net/nfc/af_nfc.c:42 nfc_sock_create() warn: potential spectre issue 'proto_tab' 
[w] (local cap)

Fix this by sanitizing proto before using it to index proto_tab.

Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].

[1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2

Signed-off-by: Gustavo A. R. Silva 
---
 net/nfc/af_nfc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/nfc/af_nfc.c b/net/nfc/af_nfc.c
index d3e594eb36d0..256f3c57059e 100644
--- a/net/nfc/af_nfc.c
+++ b/net/nfc/af_nfc.c
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 
 #include "nfc.h"
 
@@ -37,6 +38,7 @@ static int nfc_sock_create(struct net *net, struct socket 
*sock, int proto,
 
if (proto < 0 || proto >= NFC_SOCKPROTO_MAX)
return -EINVAL;
+   proto = array_index_nospec(proto, NFC_SOCKPROTO_MAX);
 
read_lock(&proto_tab_lock);
if (proto_tab[proto] && try_module_get(proto_tab[proto]->owner)) {
-- 
2.20.1



Re: linux-next: build warnings after merge of the wireless-drivers-next tree

2018-12-03 Thread Gustavo A. R. Silva




On 12/3/18 6:38 AM, Luciano Coelho wrote:

On Fri, 2018-11-30 at 09:43 +0200, Kalle Valo wrote:

Stephen Rothwell  writes:


Hi Kalle,

On Fri, 30 Nov 2018 06:33:47 +0200 Kalle Valo 
wrote:
I take it that -Wimplict-fallthrough is not enabled by default
yet? So
Dave and Linus won't see these warnings?


Correct.


What I'm planning is to send a pull request to Dave today and fix
these
warnings in the next pull request. Does that sound good?


That sounds fine.


Great, thanks for the confirmation. I'll then send the pull request
today.


Just FYI, I enabled this by default in our internal tree so we will
notice when it happens before sending the patch upstream.



That's really great.

Thanks, Luca
--
Gustavo


Re: [PATCH v2] wireless: mark expected switch fall-throughs

2018-10-23 Thread Gustavo A. R. Silva


On 10/23/18 10:59 AM, Gustavo A. R. Silva wrote:
> 
> On 10/23/18 9:01 AM, Johannes Berg wrote:
>> On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote:
>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>> where we are expecting to fall through.
>>>
>>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>>
>>> This code was not tested and GCC 7.2.0 was used to compile it.
>>
>> Look, I'm not going to make this any clearer: I'm not applying patches
>> like that where you've invested no effort whatsoever on verifying that
>> they're correct.
>>
> 
> How do you suggest me to verify that every part is correct in this type
> of patches?
> 

BTW... I'm under the impression you think that I don't even look at
the code. Is that correct?

I've been working on this for quite a while, and in every case I try to
understand the code in terms of the context in which every warning is
reported.

I look for dependencies between variables in adjacent switch cases, that could
make think it might be OK for some of those cases to fall through to the one
below. I check the names of the case labels to see if there might be any 
relation
between them, or if they are totally diferent as it might be the case for labels
that indicate the transmision(FOO_TX) or reception of data(FOO_RX). Something
similar to the latter is the case with on/off logic (FOO_ON/FOO_OFF). These are
some of the things I review in the code, so I can have an idea if the warning is
a false positive or an actual bug.

Here is a bug I found yesterday at 
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c

5690 case WLAN_CIPHER_SUITE_CCMP:
5691 key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
5692 break;
5693 case WLAN_CIPHER_SUITE_TKIP:
5694 key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
5695 default:
5696 return -EOPNOTSUPP;
5697 }

Notice how the absence of a break statement is very suspicious in case
WLAN_CIPHER_SUITE_TKIP, because the code for case WLAN_CIPHER_SUITE_CCMP
is pretty similar and in that case there is a break at the bottom. Now,
that's not the only thing that looks supicious: in the absence of a break,
the code falls through to the default case, which returns the error value
-EOPNOTSUPP. So, even when key->flags is updated, the code always returns
an error for case WLAN_CIPHER_SUITE_TKIP. This analysis led me to think
that this is an actual bug, so I sent a patch to fix it:

https://lore.kernel.org/patchwork/patch/1002440/

Notice that this bug has been there since 2015:
commit 26f1fad29ad973b0fb26a9ca3dcb2a73dde781aa


Now, let's take a look at the following warning in net/wireless/chan.c:

net/wireless/chan.c: In function ‘cfg80211_chandef_usable’:
net/wireless/chan.c:748:6: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   if (!ht_cap->ht_supported)
  ^
net/wireless/chan.c:750:2: note: here
  case NL80211_CHAN_WIDTH_20_NOHT:
  ^~~~

This is the piece of code at net/wireless/chan.c:740:

 740 case NL80211_CHAN_WIDTH_5:
 741 width = 5;
 742 break;
 743 case NL80211_CHAN_WIDTH_10:
 744 prohibited_flags |= IEEE80211_CHAN_NO_10MHZ;
 745 width = 10;
 746 break;
 747 case NL80211_CHAN_WIDTH_20:
 748 if (!ht_cap->ht_supported)
 749 return false;
 750 case NL80211_CHAN_WIDTH_20_NOHT:
 751 prohibited_flags |= IEEE80211_CHAN_NO_20MHZ;
 752 width = 20;
 753 break;
 754 case NL80211_CHAN_WIDTH_40:
 755 width = 40;
 756 if (!ht_cap->ht_supported)
 757 return false;
 758 if (!(ht_cap->cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) ||
 759 ht_cap->cap & IEEE80211_HT_CAP_40MHZ_INTOLERANT)
 760 return false;
 761 if (chandef->center_freq1 < control_freq &&
 762 chandef->chan->flags & IEEE80211_CHAN_NO_HT40MINUS)
 763 return false;
 764 if (chandef->center_freq1 > control_freq &&
 765 chandef->chan->flags & IEEE80211_CHAN_NO_HT40PLUS)
 766 return false;
 767 break;

Notice that the warning was reported at line 748, but I'm including more code
here to make it explicitly clear that I not only focus my atention in a very
narrowed piece of code around the warning.

Now, I don't see anything supicious. The labels NL80211_CHAN_WIDTH_20 and
NL80211_CHAN_WIDTH_20_NOHT seem to be related, and it's even less supicious
when there

Re: [PATCH v2] wireless: mark expected switch fall-throughs

2018-10-23 Thread Gustavo A. R. Silva


On 10/23/18 9:01 AM, Johannes Berg wrote:
> On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
>>
>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>
>> This code was not tested and GCC 7.2.0 was used to compile it.
> 
> Look, I'm not going to make this any clearer: I'm not applying patches
> like that where you've invested no effort whatsoever on verifying that
> they're correct.
> 

How do you suggest me to verify that every part is correct in this type
of patches?

Thanks
--
Gustavo





Re: [PATCH] wireless: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva



On 7/6/18 2:29 PM, Johannes Berg wrote:
> Hi Gustavo,
> 
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
> 
> You dropped the remark saying you didn't review them, but did you?
> 

I'll add it in v2.

>>  case NL80211_CHAN_WIDTH_20:
>>  if (!ht_cap->ht_supported)
>>  return false;
>> +/* else: fall through */
> 
> What's the point in else:?
> 
> We also don't necessarily write
> 
> if (!...)
>   return false;
> else
>   do_something();
> 
> but rather
> 
> if (!...)
>   return false;
> do_something().
> 
> I think I'd prefer without the "else:"
> 

Sure thing. I'll change this in v2.

I'll send v2 shortly.

Thanks for the feedback.
--
Gustavo


[PATCH v2] wireless: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Warning level 3 was used: -Wimplicit-fallthrough=3

This code was not tested and GCC 7.2.0 was used to compile it.

Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 - Explicity mention in the commit log that this the code was not
   tested.
 - Use warning level 3 instead of 2.
 - Change the form "else: fall through" to "fall through" instead.
 - Address -Wimplicit-fallthrough warnings in net/wireless/scan.c

 net/wireless/chan.c| 2 ++
 net/wireless/nl80211.c | 9 +
 net/wireless/scan.c| 2 +-
 net/wireless/wext-compat.c | 2 ++
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 2db713d..0a45265 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -747,6 +747,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
case NL80211_CHAN_WIDTH_20:
if (!ht_cap->ht_supported)
return false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20_NOHT:
prohibited_flags |= IEEE80211_CHAN_NO_20MHZ;
width = 20;
@@ -769,6 +770,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK;
if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)
return false;
+   /* fall through */
case NL80211_CHAN_WIDTH_80:
if (!vht_cap->vht_supported)
return false;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 744b585..0276370 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1706,6 +1706,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 1:
if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES,
sizeof(u32) * rdev->wiphy.n_cipher_suites,
@@ -1752,6 +1753,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 2:
if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES,
rdev->wiphy.interface_modes))
@@ -1759,6 +1761,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 3:
nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS);
if (!nl_bands)
@@ -1784,6 +1787,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->chan_start++;
if (state->split)
break;
+   /* fall through */
default:
/* add frequencies */
nl_freqs = nla_nest_start(
@@ -1837,6 +1841,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 4:
nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS);
if (!nl_cmds)
@@ -1863,6 +1868,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 5:
if (rdev->ops->remain_on_channel &&
(rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL) &&
@@ -1880,6 +1886,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 6:
 #ifdef CONFIG_PM
if (nl80211_send_wowlan(msg, rdev, state->split))
@@ -1890,6 +1897,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
 #else
state->split_start++;
 #endif
+   /* fall through */
case 7:
if (nl80211_put_iftypes(msg, NL80211_ATTR_SOFTWARE_IFTYPES,
rdev->wiphy.software_iftypes))
@@ -1902,6 +1910,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
   

[PATCH 17/20] rt2x00: rt61pci: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ralink/rt2x00/rt61pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.c 
b/drivers/net/wireless/ralink/rt2x00/rt61pci.c
index cb0e119..4c5de8f 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.c
@@ -2226,7 +2226,7 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev)
break;
case 6: /* Failure, excessive retries */
__set_bit(TXDONE_EXCESSIVE_RETRY, &txdesc.flags);
-   /* Don't break, this is a failed frame! */
+   /* Fall through - this is a failed frame! */
default: /* Failure */
__set_bit(TXDONE_FAILURE, &txdesc.flags);
}
-- 
2.7.4



[PATCH 19/20] rtlwifi: rtl8821ae: phy: Mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c 
b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
index 176deb2..a75451c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
@@ -394,6 +394,7 @@ static void _rtl8812ae_phy_set_rfe_reg_24g(struct 
ieee80211_hw *hw)
rtl_set_bbreg(hw, RB_RFE_INV, BMASKRFEINV, 0x000);
break;
}
+   /* fall through */
case 0:
case 2:
default:
-- 
2.7.4



[PATCH 06/20] carl9170: tx: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/carl9170/tx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/carl9170/tx.c 
b/drivers/net/wireless/ath/carl9170/tx.c
index 8c75651..2407931 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -830,10 +830,12 @@ static bool carl9170_tx_rts_check(struct ar9170 *ar,
case CARL9170_ERP_AUTO:
if (ampdu)
break;
+   /* fall through */
 
case CARL9170_ERP_MAC80211:
if (!(rate->flags & IEEE80211_TX_RC_USE_RTS_CTS))
break;
+   /* fall through */
 
case CARL9170_ERP_RTS:
if (likely(!multi))
@@ -854,6 +856,7 @@ static bool carl9170_tx_cts_check(struct ar9170 *ar,
case CARL9170_ERP_MAC80211:
if (!(rate->flags & IEEE80211_TX_RC_USE_CTS_PROTECT))
break;
+   /* fall through */
 
case CARL9170_ERP_CTS:
return true;
-- 
2.7.4



[PATCH 12/20] prism54: islpci_dev: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 114947 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intersil/prism54/islpci_dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/intersil/prism54/islpci_dev.c 
b/drivers/net/wireless/intersil/prism54/islpci_dev.c
index 325176d..ad6d3a5 100644
--- a/drivers/net/wireless/intersil/prism54/islpci_dev.c
+++ b/drivers/net/wireless/intersil/prism54/islpci_dev.c
@@ -932,6 +932,7 @@ islpci_set_state(islpci_private *priv, islpci_state_t 
new_state)
switch (new_state) {
case PRV_STATE_OFF:
priv->state_off++;
+   /* fall through */
default:
priv->state = new_state;
break;
-- 
2.7.4



[PATCH 11/20] prism54: isl_ioctl: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intersil/prism54/isl_ioctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/intersil/prism54/isl_ioctl.c 
b/drivers/net/wireless/intersil/prism54/isl_ioctl.c
index 334717b0..3ccf2a4 100644
--- a/drivers/net/wireless/intersil/prism54/isl_ioctl.c
+++ b/drivers/net/wireless/intersil/prism54/isl_ioctl.c
@@ -1691,6 +1691,7 @@ static int prism54_get_encodeext(struct net_device *ndev,
case DOT11_AUTH_BOTH:
case DOT11_AUTH_SK:
wrqu->encoding.flags |= IW_ENCODE_RESTRICTED;
+   /* fall through */
case DOT11_AUTH_OS:
default:
wrqu->encoding.flags |= IW_ENCODE_OPEN;
-- 
2.7.4



[PATCH 14/20] rt2x00: rt2400pci: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c 
b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
index 0bc8b02..49a7327 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
@@ -1302,7 +1302,7 @@ static void rt2400pci_txdone(struct rt2x00_dev *rt2x00dev,
break;
case 2: /* Failure, excessive retries */
__set_bit(TXDONE_EXCESSIVE_RETRY, &txdesc.flags);
-   /* Don't break, this is a failed frame! */
+   /* Fall through - this is a failed frame! */
default: /* Failure */
__set_bit(TXDONE_FAILURE, &txdesc.flags);
}
-- 
2.7.4



[PATCH 09/20] orinoco_usb: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c 
b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 21bb684..40a8b941 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -908,6 +908,7 @@ static int ezusb_access_ltv(struct ezusb_priv *upriv,
case EZUSB_CTX_REQ_SUBMITTED:
if (!ctx->in_rid)
break;
+   /* fall through */
default:
err("%s: Unexpected context state %d", __func__,
state);
-- 
2.7.4



[PATCH 07/20] iwlegacy: 4965-mac: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c 
b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
index 280cd8a..6b4488a 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
@@ -559,7 +559,7 @@ il4965_translate_rx_status(struct il_priv *il, u32 
decrypt_in)
decrypt_out |= RX_RES_STATUS_BAD_KEY_TTAK;
break;
}
-   /* fall through if TTAK OK */
+   /* fall through - if TTAK OK */
default:
if (!(decrypt_in & RX_MPDU_RES_STATUS_ICV_OK))
decrypt_out |= RX_RES_STATUS_BAD_ICV_MIC;
-- 
2.7.4



[PATCH 05/20] carl9170: rx: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1056534 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/carl9170/rx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/carl9170/rx.c 
b/drivers/net/wireless/ath/carl9170/rx.c
index 7050632..f7c2f19 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -766,6 +766,7 @@ static void carl9170_rx_untie_data(struct ar9170 *ar, u8 
*buf, int len)
 
goto drop;
}
+   /* fall through */
 
case AR9170_RX_STATUS_MPDU_MIDDLE:
/*  These are just data + mac status */
-- 
2.7.4



[PATCH 02/20] ath9k: ar5008_phy: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1056532 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/ar5008_phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar5008_phy.c 
b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
index 11d6f975..dae9540 100644
--- a/drivers/net/wireless/ath/ath9k/ar5008_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
@@ -586,7 +586,7 @@ static void ar5008_hw_init_chain_masks(struct ath_hw *ah)
REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, 0x7);
break;
}
-   /* else: fall through */
+   /* fall through */
case 0x1:
case 0x2:
case 0x7:
-- 
2.7.4



[PATCH 00/20] Mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, this patchset aims
to mark switch cases where we are expecting to fall through.

Thanks!

Gustavo A. R. Silva (20):
  ath6kl: Mark expected switch fall-through
  ath9k: mark expected switch fall-through
  ath9k: ar9002_phy: mark expected switch fall-throughs
  ath9k_hw: mark expected switch fall-through
  carl9170: rx: mark expected switch fall-through
  carl9170: tx: mark expected switch fall-throughs
  iwlegacy: 4965-mac: mark expected switch fall-through
  iwlegacy: common: mark expected switch fall-throughs
  orinoco_usb: mark expected switch fall-through
  prism54: isl_38xx: Mark expected switch fall-through
  prism54: isl_ioctl: mark expected switch fall-through
  prism54: islpci_dev: mark expected switch fall-through
  mwifiex: Mark expected switch fall-through
  rt2x00: rt2400pci: mark expected switch fall-through
  rt2x00: rt2500pci: mark expected switch fall-through
  rt2x00: rt2800lib: mark expected switch fall-through
  rt2x00: rt61pci: mark expected switch fall-through
  ray_cs: mark expected switch fall-throughs
  rtlwifi: rtl8821ae: phy: Mark expected switch fall-through
  zd1201: mark expected switch fall-through

 drivers/net/wireless/ath/ath6kl/main.c   | 1 +
 drivers/net/wireless/ath/ath9k/ar5008_phy.c  | 2 +-
 drivers/net/wireless/ath/ath9k/ar9002_phy.c  | 2 +-
 drivers/net/wireless/ath/ath9k/hw.c  | 1 +
 drivers/net/wireless/ath/carl9170/rx.c   | 1 +
 drivers/net/wireless/ath/carl9170/tx.c   | 3 +++
 drivers/net/wireless/intel/iwlegacy/4965-mac.c   | 2 +-
 drivers/net/wireless/intel/iwlegacy/common.c | 2 ++
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c  | 1 +
 drivers/net/wireless/intersil/prism54/isl_38xx.c | 1 +
 drivers/net/wireless/intersil/prism54/isl_ioctl.c| 1 +
 drivers/net/wireless/intersil/prism54/islpci_dev.c   | 1 +
 drivers/net/wireless/marvell/mwifiex/ie.c| 1 +
 drivers/net/wireless/ralink/rt2x00/rt2400pci.c   | 2 +-
 drivers/net/wireless/ralink/rt2x00/rt2500pci.c   | 2 +-
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c   | 4 
 drivers/net/wireless/ralink/rt2x00/rt61pci.c | 2 +-
 drivers/net/wireless/ray_cs.c| 2 ++
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 1 +
 drivers/net/wireless/zydas/zd1201.c  | 1 +
 20 files changed, 27 insertions(+), 6 deletions(-)

-- 
2.7.4



[PATCH 20/20] zd1201: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/zydas/zd1201.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/zydas/zd1201.c 
b/drivers/net/wireless/zydas/zd1201.c
index 2534038..22c70f1 100644
--- a/drivers/net/wireless/zydas/zd1201.c
+++ b/drivers/net/wireless/zydas/zd1201.c
@@ -969,6 +969,7 @@ static int zd1201_set_mode(struct net_device *dev,
 */
zd1201_join(zd, "\0-*#\0", 5);
/* Put port in pIBSS */
+   /* Fall through */
case 8: /* No pseudo-IBSS in wireless extensions (yet) */
porttype = ZD1201_PORTTYPE_PSEUDOIBSS;
break;
-- 
2.7.4



[PATCH 18/20] ray_cs: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 114948 ("Missing break in switch")
Addresses-Coverity-ID: 114949 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ray_cs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index 08c607c..33ad875 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -889,8 +889,10 @@ static int ray_hw_xmit(unsigned char *data, int len, 
struct net_device *dev,
switch (ccsindex = get_free_tx_ccs(local)) {
case ECCSBUSY:
pr_debug("ray_hw_xmit tx_ccs table busy\n");
+   /* fall through */
case ECCSFULL:
pr_debug("ray_hw_xmit No free tx ccs\n");
+   /* fall through */
case ECARDGONE:
netif_stop_queue(dev);
return XMIT_NO_CCS;
-- 
2.7.4



[PATCH 16/20] rt2x00: rt2800lib: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 145198 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c 
b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 9e7b893..0e9 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -2482,6 +2482,7 @@ static void rt2800_config_channel_rf3052(struct 
rt2x00_dev *rt2x00dev,
switch (rt2x00dev->default_ant.tx_chain_num) {
case 1:
rt2x00_set_field8(&rfcsr, RFCSR1_TX1_PD, 1);
+   /* fall through */
case 2:
rt2x00_set_field8(&rfcsr, RFCSR1_TX2_PD, 1);
break;
@@ -2490,6 +2491,7 @@ static void rt2800_config_channel_rf3052(struct 
rt2x00_dev *rt2x00dev,
switch (rt2x00dev->default_ant.rx_chain_num) {
case 1:
rt2x00_set_field8(&rfcsr, RFCSR1_RX1_PD, 1);
+   /* fall through */
case 2:
rt2x00_set_field8(&rfcsr, RFCSR1_RX2_PD, 1);
break;
@@ -9457,8 +9459,10 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev 
*rt2x00dev)
switch (rx_chains) {
case 3:
spec->ht.mcs.rx_mask[2] = 0xff;
+   /* fall through */
case 2:
spec->ht.mcs.rx_mask[1] = 0xff;
+   /* fall through */
case 1:
spec->ht.mcs.rx_mask[0] = 0xff;
spec->ht.mcs.rx_mask[4] = 0x1; /* MCS32 */
-- 
2.7.4



[PATCH 15/20] rt2x00: rt2500pci: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c 
b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
index 1ff5434..e8e7bfe 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
@@ -1430,7 +1430,7 @@ static void rt2500pci_txdone(struct rt2x00_dev *rt2x00dev,
break;
case 2: /* Failure, excessive retries */
__set_bit(TXDONE_EXCESSIVE_RETRY, &txdesc.flags);
-   /* Don't break, this is a failed frame! */
+   /* Fall through - this is a failed frame! */
default: /* Failure */
__set_bit(TXDONE_FAILURE, &txdesc.flags);
}
-- 
2.7.4



[PATCH 13/20] mwifiex: Mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/marvell/mwifiex/ie.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/ie.c 
b/drivers/net/wireless/marvell/mwifiex/ie.c
index 75cbd60..6845eb5 100644
--- a/drivers/net/wireless/marvell/mwifiex/ie.c
+++ b/drivers/net/wireless/marvell/mwifiex/ie.c
@@ -363,6 +363,7 @@ static int mwifiex_uap_parse_tail_ies(struct 
mwifiex_private *priv,
(const u8 *)hdr,
hdr->len + sizeof(struct 
ieee_types_header)))
break;
+   /* fall through */
default:
memcpy(gen_ie->ie_buffer + ie_len, hdr,
   hdr->len + sizeof(struct ieee_types_header));
-- 
2.7.4



[PATCH 10/20] prism54: isl_38xx: Mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 114944 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intersil/prism54/isl_38xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/intersil/prism54/isl_38xx.c 
b/drivers/net/wireless/intersil/prism54/isl_38xx.c
index ce9d4db..b0eb58a 100644
--- a/drivers/net/wireless/intersil/prism54/isl_38xx.c
+++ b/drivers/net/wireless/intersil/prism54/isl_38xx.c
@@ -235,6 +235,7 @@ isl38xx_in_queue(isl38xx_control_block *cb, int queue)
/* send queues */
case ISL38XX_CB_TX_MGMTQ:
BUG_ON(delta > ISL38XX_CB_MGMT_QSIZE);
+   /* fall through */
 
case ISL38XX_CB_TX_DATA_LQ:
case ISL38XX_CB_TX_DATA_HQ:
-- 
2.7.4



[PATCH 08/20] iwlegacy: common: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 201384 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/intel/iwlegacy/common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlegacy/common.c 
b/drivers/net/wireless/intel/iwlegacy/common.c
index 6514baf..a2f86cb 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -2695,6 +2695,7 @@ il_set_decrypted_flag(struct il_priv *il, struct 
ieee80211_hdr *hdr,
if ((decrypt_res & RX_RES_STATUS_DECRYPT_TYPE_MSK) ==
RX_RES_STATUS_BAD_KEY_TTAK)
break;
+   /* fall through */
 
case RX_RES_STATUS_SEC_TYPE_WEP:
if ((decrypt_res & RX_RES_STATUS_DECRYPT_TYPE_MSK) ==
@@ -2704,6 +2705,7 @@ il_set_decrypted_flag(struct il_priv *il, struct 
ieee80211_hdr *hdr,
D_RX("Packet destroyed\n");
return -1;
}
+   /* fall through */
case RX_RES_STATUS_SEC_TYPE_CCMP:
if ((decrypt_res & RX_RES_STATUS_DECRYPT_TYPE_MSK) ==
RX_RES_STATUS_DECRYPT_OK) {
-- 
2.7.4



[PATCH 04/20] ath9k: hw: mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1056532 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c 
b/drivers/net/wireless/ath/ath9k/hw.c
index bb319f2..8581d91 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -2279,6 +2279,7 @@ void ath9k_hw_beaconinit(struct ath_hw *ah, u32 
next_beacon, u32 beacon_period)
case NL80211_IFTYPE_ADHOC:
REG_SET_BIT(ah, AR_TXCFG,
AR_TXCFG_ADHOC_BEACON_ATIM_TX_POLICY);
+   /* fall through */
case NL80211_IFTYPE_MESH_POINT:
case NL80211_IFTYPE_AP:
REG_WRITE(ah, AR_NEXT_TBTT_TIMER, next_beacon);
-- 
2.7.4



[PATCH 03/20] ath9k: ar9002_phy: mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1056532 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/ar9002_phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_phy.c 
b/drivers/net/wireless/ath/ath9k/ar9002_phy.c
index 7132918..6f32b8d 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_phy.c
@@ -119,7 +119,7 @@ static int ar9002_hw_set_channel(struct ath_hw *ah, struct 
ath9k_channel *chan)
aModeRefSel = 2;
if (aModeRefSel)
break;
-   /* else: fall through */
+   /* fall through */
case 1:
default:
aModeRefSel = 0;
-- 
2.7.4



[PATCH 01/20] ath6kl: Mark expected switch fall-through

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 201383 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath6kl/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c 
b/drivers/net/wireless/ath/ath6kl/main.c
index cb59016..5e7ea83 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -389,6 +389,7 @@ void ath6kl_connect_ap_mode_bss(struct ath6kl_vif *vif, u16 
channel)
if (!ik->valid || ik->key_type != WAPI_CRYPT)
break;
/* for WAPI, we need to set the delayed group key, continue: */
+   /* fall through */
case WPA_PSK_AUTH:
case WPA2_PSK_AUTH:
case (WPA_PSK_AUTH | WPA2_PSK_AUTH):
-- 
2.7.4



Re: [PATCH 0/2] Mark expected switch fall-throughs and fix missing break

2018-10-22 Thread Gustavo A. R. Silva


On 10/22/18 4:36 PM, Jes Sorensen wrote:
> On 10/22/18 7:49 AM, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, this patchset aims
>> to mark multiple switch cases where we are expecting to fall through.
>>
>> Also, the second patch in this series fixes a missing break in switch.
> 
> Enabling that sounds like a great way to inflict pain and suffering.
> 

Not really. The -Wimplicit-fallthrough will be enabled until after all the
current warnings have been addressed.

There are 600 of these issues left. So, hopefully I will complete this task
during the next development cycle.

Thanks
--
Gustavo


[PATCH 2/2] rtl8xxxu: Fix missing break in switch

2018-10-22 Thread Gustavo A. R. Silva
Add missing break statement in order to prevent the code from falling
through to the default case.

Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c6b0686..2bd4305 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5692,6 +5692,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum 
set_key_cmd cmd,
break;
case WLAN_CIPHER_SUITE_TKIP:
key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
+   break;
default:
return -EOPNOTSUPP;
}
-- 
2.7.4



[PATCH 1/2] rtl8xxxu: Mark expected switch fall-throughs

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1357355 ("Missing break in switch")
Addresses-Coverity-ID: 1357378 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 56040b1..c6b0686 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw)
switch (hw->conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
ht = false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
opmode |= BW_OPMODE_20MHZ;
rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
@@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
switch (hw->conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
ht = false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
subchannel = 0;
@@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv 
*priv)
case 3:
priv->ep_tx_low_queue = 1;
priv->ep_tx_count++;
+   /* fall through */
case 2:
priv->ep_tx_normal_queue = 1;
priv->ep_tx_count++;
+   /* fall through */
case 1:
priv->ep_tx_high_queue = 1;
priv->ep_tx_count++;
-- 
2.7.4



[PATCH 0/2] Mark expected switch fall-throughs and fix missing break

2018-10-22 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, this patchset aims
to mark multiple switch cases where we are expecting to fall through.

Also, the second patch in this series fixes a missing break in switch.

Thanks

Gustavo A. R. Silva (2):
  rtl8xxxu: Mark expected switch fall-throughs
  rtl8xxxu: Fix missing break in switch

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +
 1 file changed, 5 insertions(+)

-- 
2.7.4



Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats

2018-10-13 Thread Gustavo A. R. Silva


On 10/13/18 7:23 PM, Kalle Valo wrote:
> 
> Patch applied to ath-next branch of ath.git, thanks.
> 
> 9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in 
> ath10k_update_per_peer_tx_stats
> 

Thank you, Kalle.
--
Gustavo


Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats

2018-10-09 Thread Gustavo A. R. Silva


> 
> I have sent a patch to address this,
> https://patchwork.kernel.org/patch/10611943/
> 

That's great.

Thanks for letting me know. :)

--
Gustavo


[PATCH] ath10k: remove unnecessary comparison of unsigned integer with < 0

2018-10-05 Thread Gustavo A. R. Silva
There is no need to compare *ps_state_enable* with < 0 because
such variable is of type u8 (8 bits, unsigned), making it
impossible to hold a negative value.

Fix this by removing such comparison.

Addresses-Coverity-ID: 1473921 ("Unsigned compared against 0")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath10k/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index 2c0cb67..15964b3 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2421,7 +2421,7 @@ static ssize_t ath10k_write_ps_state_enable(struct file 
*file,
if (kstrtou8_from_user(user_buf, count, 0, &ps_state_enable))
return -EINVAL;
 
-   if (ps_state_enable > 1 || ps_state_enable < 0)
+   if (ps_state_enable > 1)
return -EINVAL;
 
mutex_lock(&ar->conf_mutex);
-- 
2.7.4



Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats

2018-10-05 Thread Gustavo A. R. Silva



On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
> 
> 
> On 10/5/18 9:09 PM, Ben Greear wrote:
>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>> Currently, the error handling for the call to function
>>> ath10k_get_legacy_rate_idx() doesn't work because
>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>> makes it impossible for it to hold a value less
>>> than 0.
>>>
>>> Fix this by changing the type of variable *rate_idx*
>>> to s8 (8 bits, signed).
>>
>> There are more than 127 rates, are you sure this is doing
>> what you want?
>>
> 
> Based on the following function, rate_idx can only hold values from 0 to 11
> 

... and of course -EINVAL too

> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
> {
> static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
>   18, 24, 36, 48, 54};
> int i;
> 
> for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
> if (rate == legacy_rates[i])
> return i;
> }
> 
> ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
> return -EINVAL;
> }
> 
> Thanks
> --
> Gustavo
> 
>> Thanks,
>> Ben
>>
>>>
>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the 
>>> txrate table")
>>> Signed-off-by: Gustavo A. R. Silva 
>>> ---
>>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index f240525..edd0e74 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>>  struct ath10k_per_peer_tx_stats *peer_stats)
>>>  {
>>>  struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>> -    u8 rate = 0, rate_idx = 0, sgi;
>>> +    u8 rate = 0, sgi;
>>> +    s8 rate_idx = 0;
>>>  struct rate_info txrate;
>>>
>>>  lockdep_assert_held(&ar->data_lock);
>>>
>>
>>


Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats

2018-10-05 Thread Gustavo A. R. Silva



On 10/5/18 9:09 PM, Ben Greear wrote:
> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>> Currently, the error handling for the call to function
>> ath10k_get_legacy_rate_idx() doesn't work because
>> *rate_idx* is of type u8 (8 bits, unsigned), which
>> makes it impossible for it to hold a value less
>> than 0.
>>
>> Fix this by changing the type of variable *rate_idx*
>> to s8 (8 bits, signed).
> 
> There are more than 127 rates, are you sure this is doing
> what you want?
> 

Based on the following function, rate_idx can only hold values from 0 to 11

static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
{
static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
  18, 24, 36, 48, 54};
int i;

for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
if (rate == legacy_rates[i])
return i;
}

ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
return -EINVAL;
}

Thanks
--
Gustavo

> Thanks,
> Ben
> 
>>
>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate 
>> table")
>> Signed-off-by: Gustavo A. R. Silva 
>> ---
>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index f240525..edd0e74 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>  struct ath10k_per_peer_tx_stats *peer_stats)
>>  {
>>  struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>> -    u8 rate = 0, rate_idx = 0, sgi;
>> +    u8 rate = 0, sgi;
>> +    s8 rate_idx = 0;
>>  struct rate_info txrate;
>>
>>  lockdep_assert_held(&ar->data_lock);
>>
> 
> 


[PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats

2018-10-05 Thread Gustavo A. R. Silva
Currently, the error handling for the call to function
ath10k_get_legacy_rate_idx() doesn't work because
*rate_idx* is of type u8 (8 bits, unsigned), which
makes it impossible for it to hold a value less
than 0.

Fix this by changing the type of variable *rate_idx*
to s8 (8 bits, signed).

Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate 
table")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index f240525..edd0e74 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
struct ath10k_per_peer_tx_stats *peer_stats)
 {
struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
-   u8 rate = 0, rate_idx = 0, sgi;
+   u8 rate = 0, sgi;
+   s8 rate_idx = 0;
struct rate_info txrate;
 
lockdep_assert_held(&ar->data_lock);
-- 
2.7.4



[PATCH] NFC: st21nfca: Mark expected switch fall-through

2018-10-05 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1373889 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/nfc/st21nfca/dep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nfc/st21nfca/dep.c b/drivers/nfc/st21nfca/dep.c
index 3420c51..6219e92 100644
--- a/drivers/nfc/st21nfca/dep.c
+++ b/drivers/nfc/st21nfca/dep.c
@@ -620,6 +620,7 @@ static void st21nfca_im_recv_dep_res_cb(void *context, 
struct sk_buff *skb,
switch (ST21NFCA_NFC_DEP_PFB_TYPE(dep_res->pfb)) {
case ST21NFCA_NFC_DEP_PFB_ACK_NACK_PDU:
pr_err("Received a ACK/NACK PDU\n");
+   /* fall through */
case ST21NFCA_NFC_DEP_PFB_I_PDU:
info->dep_info.curr_nfc_dep_pni =
ST21NFCA_NFC_DEP_PFB_PNI(dep_res->pfb + 1);
-- 
2.7.4



[PATCH] NFC: pn533: mark expected switch fall-throughs

2018-10-05 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Addresses-Coverity-ID: 1230487 ("Missing break in switch")
Addresses-Coverity-ID: 1230488 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/nfc/pn533/pn533.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a0cc1cc..5961f14 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2147,6 +2147,7 @@ static int pn533_transceive(struct nfc_dev *nfc_dev,
 
break;
}
+   /* fall through */
default:
/* jumbo frame ? */
if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) {
@@ -2273,6 +2274,7 @@ static void pn533_wq_mi_recv(struct work_struct *work)
 
break;
}
+   /* fall through */
default:
skb_put_u8(skb, 1); /*TG*/
 
-- 
2.7.4



[PATCH] ssb: chipcommon: fix fall-through annotation

2018-10-03 Thread Gustavo A. R. Silva
Replace "Fallthough" with a proper "fall through" annotation.

This fix is part of the ongoing efforts to enabling
-Wimplicit-fallthrough

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ssb/driver_chipcommon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/driver_chipcommon.c b/drivers/ssb/driver_chipcommon.c
index 99a4656..3861cb6 100644
--- a/drivers/ssb/driver_chipcommon.c
+++ b/drivers/ssb/driver_chipcommon.c
@@ -425,7 +425,7 @@ void ssb_chipco_get_clockcontrol(struct ssb_chipcommon *cc,
*m = chipco_read32(cc, SSB_CHIPCO_CLOCK_M2);
break;
}
-   /* Fallthough */
+   /* Fall through */
default:
*m = chipco_read32(cc, SSB_CHIPCO_CLOCK_SB);
}
-- 
2.7.4



[PATCH] mwifiex: mark expected switch fall-throughs

2018-05-25 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 4 
 drivers/net/wireless/marvell/mwifiex/scan.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 54a2297..16a705d 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1158,6 +1158,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_UNSPECIFIED:
mwifiex_dbg(priv->adapter, INFO,
"%s: kept type as IBSS\n", dev->name);
+   /* fall through */
case NL80211_IFTYPE_ADHOC:  /* This shouldn't happen */
return 0;
default:
@@ -1188,6 +1189,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_UNSPECIFIED:
mwifiex_dbg(priv->adapter, INFO,
"%s: kept type as STA\n", dev->name);
+   /* fall through */
case NL80211_IFTYPE_STATION:/* This shouldn't happen */
return 0;
default:
@@ -1210,6 +1212,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_UNSPECIFIED:
mwifiex_dbg(priv->adapter, INFO,
"%s: kept type as AP\n", dev->name);
+   /* fall through */
case NL80211_IFTYPE_AP: /* This shouldn't happen */
return 0;
default:
@@ -1249,6 +1252,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
case NL80211_IFTYPE_UNSPECIFIED:
mwifiex_dbg(priv->adapter, INFO,
"%s: kept type as P2P\n", dev->name);
+   /* fall through */
case NL80211_IFTYPE_P2P_CLIENT:
case NL80211_IFTYPE_P2P_GO:
return 0;
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c 
b/drivers/net/wireless/marvell/mwifiex/scan.c
index d7ce7f7..19df92b 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1308,6 +1308,7 @@ int mwifiex_update_bss_desc_with_ie(struct 
mwifiex_adapter *adapter,
 
case WLAN_EID_CHANNEL_SWITCH:
bss_entry->chan_sw_ie_present = true;
+   /* fall through */
case WLAN_EID_PWR_CAPABILITY:
case WLAN_EID_TPC_REPORT:
case WLAN_EID_QUIET:
-- 
2.7.4



[PATCH] ath9k: mark expected switch fall-throughs

2018-05-25 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/ar5008_phy.c | 2 ++
 drivers/net/wireless/ath/ath9k/ar9002_phy.c | 1 +
 drivers/net/wireless/ath/ath9k/main.c   | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/ar5008_phy.c 
b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
index 7922550..ef2dd68 100644
--- a/drivers/net/wireless/ath/ath9k/ar5008_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
@@ -583,12 +583,14 @@ static void ar5008_hw_init_chain_masks(struct ath_hw *ah)
case 0x5:
REG_SET_BIT(ah, AR_PHY_ANALOG_SWAP,
AR_PHY_SWAP_ALT_CHAIN);
+   /* fall through */
case 0x3:
if (ah->hw_version.macVersion == AR_SREV_REVISION_5416_10) {
REG_WRITE(ah, AR_PHY_RX_CHAINMASK, 0x7);
REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, 0x7);
break;
}
+   /* else: fall through */
case 0x1:
case 0x2:
case 0x7:
diff --git a/drivers/net/wireless/ath/ath9k/ar9002_phy.c 
b/drivers/net/wireless/ath/ath9k/ar9002_phy.c
index 61a9b85..7132918 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_phy.c
@@ -119,6 +119,7 @@ static int ar9002_hw_set_channel(struct ath_hw *ah, struct 
ath9k_channel *chan)
aModeRefSel = 2;
if (aModeRefSel)
break;
+   /* else: fall through */
case 1:
default:
aModeRefSel = 0;
diff --git a/drivers/net/wireless/ath/ath9k/main.c 
b/drivers/net/wireless/ath/ath9k/main.c
index a3be8ad..11d84f4 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1928,6 +1928,7 @@ static int ath9k_ampdu_action(struct ieee80211_hw *hw,
case IEEE80211_AMPDU_TX_STOP_FLUSH:
case IEEE80211_AMPDU_TX_STOP_FLUSH_CONT:
flush = true;
+   /* fall through */
case IEEE80211_AMPDU_TX_STOP_CONT:
ath9k_ps_wakeup(sc);
ath_tx_aggr_stop(sc, sta, tid);
-- 
2.7.4



Re: [PATCH v2] ath6kl: mark expected switch fall-throughs

2018-05-25 Thread Gustavo A. R. Silva



On 05/25/2018 01:27 PM, Steve deRosier wrote:

On Fri, May 25, 2018 at 11:23 AM Gustavo A. R. Silva

wrote:


In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.



Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
- Place code comments on a line of their own.



drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++
1 file changed, 3 insertions(+)



diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c

b/drivers/net/wireless/ath/ath6kl/cfg80211.c

index 2ba8cf3..a16ee5d 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -3899,16 +3899,19 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
   switch (ar->hw.cap) {
   case WMI_11AN_CAP:
   ht = true;
+   /* fall through */
   case WMI_11A_CAP:
   band_5gig = true;
   break;
   case WMI_11GN_CAP:
   ht = true;
+   /* fall through */
   case WMI_11G_CAP:
   band_2gig = true;
   break;
   case WMI_11AGN_CAP:
   ht = true;
+   /* fall through */
   case WMI_11AG_CAP:
   band_2gig = true;
   band_5gig = true;
--
2.7.4



Gustavo,

Thanks for the adjustment.  It now looks good to me.



Glad to help. :)


Reviewed-by: Steve deRosier 


Thanks
--
Gustavo


Re: [PATCH] ath6kl: mark expected switch fall-throughs

2018-05-25 Thread Gustavo A. R. Silva



On 05/25/2018 01:10 PM, Kalle Valo wrote:

Yeah, I was wondering the same. Was there a particular reason for this?



Sometimes people use this style for a one-line code block.

I can change it to the traditional style. No problem.


I would prefer that. So if you can send v2 that would be great.



Yep. No problem. I'll send it shortly.

Thanks
--
Gustavo


[PATCH v2] ath6kl: mark expected switch fall-throughs

2018-05-25 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 - Place code comments on a line of their own.

 drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c 
b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 2ba8cf3..a16ee5d 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -3899,16 +3899,19 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
switch (ar->hw.cap) {
case WMI_11AN_CAP:
ht = true;
+   /* fall through */
case WMI_11A_CAP:
band_5gig = true;
break;
case WMI_11GN_CAP:
ht = true;
+   /* fall through */
case WMI_11G_CAP:
band_2gig = true;
break;
case WMI_11AGN_CAP:
ht = true;
+   /* fall through */
case WMI_11AG_CAP:
band_2gig = true;
band_5gig = true;
-- 
2.7.4



Re: [PATCH] ath6kl: mark expected switch fall-throughs

2018-05-25 Thread Gustavo A. R. Silva



On 05/25/2018 08:30 AM, Kalle Valo wrote:

Sergei Shtylyov  writes:


On 5/25/2018 2:13 AM, Gustavo A. R. Silva wrote:


In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
   drivers/net/wireless/ath/ath6kl/cfg80211.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c 
b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 2ba8cf3..29e32cd 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -3898,17 +3898,17 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
wiphy->max_scan_ie_len = 1000; /* FIX: what is correct limit? */
switch (ar->hw.cap) {
case WMI_11AN_CAP:
-   ht = true;
+   ht = true; /* fall through */
case WMI_11A_CAP:
band_5gig = true;
break;
case WMI_11GN_CAP:
-   ht = true;
+   ht = true; /* fall through */
case WMI_11G_CAP:
band_2gig = true;
break;
case WMI_11AGN_CAP:
-   ht = true;
+   ht = true; /* fall through */
case WMI_11AG_CAP:
band_2gig = true;
band_5gig = true;


Hm, typically such comments are done on a line of their own, have
never seen this style...


Yeah, I was wondering the same. Was there a particular reason for this?



Sometimes people use this style for a one-line code block.

I can change it to the traditional style. No problem.

Thanks
--
Gustavo


[PATCH] ath5k: mark expected switch fall-through

2018-05-24 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath5k/pcu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath5k/pcu.c 
b/drivers/net/wireless/ath/ath5k/pcu.c
index f23c851..05140d8 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -670,6 +670,7 @@ ath5k_hw_init_beacon_timers(struct ath5k_hw *ah, u32 
next_beacon, u32 interval)
break;
case NL80211_IFTYPE_ADHOC:
AR5K_REG_ENABLE_BITS(ah, AR5K_TXCFG, AR5K_TXCFG_ADHOC_BCN_ATIM);
+   /* fall through */
default:
/* On non-STA modes timer1 is used as next DMA
 * beacon alert (DBA) timer and timer2 as next
-- 
2.7.4



[PATCH] ath10k: htt_tx: mark expected switch fall-throughs

2018-05-24 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Notice that in this particular case, I replaced "pass through" with
a proper "fall through" comment, which is what GCC is expecting
to find.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath10k/htt_tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c 
b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 5d8b97a..89157c5 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -1202,7 +1202,7 @@ static int ath10k_htt_tx_32(struct ath10k_htt *htt,
case ATH10K_HW_TXRX_RAW:
case ATH10K_HW_TXRX_NATIVE_WIFI:
flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
-   /* pass through */
+   /* fall through */
case ATH10K_HW_TXRX_ETHERNET:
if (ar->hw_params.continuous_frag_desc) {
ext_desc_t = htt->frag_desc.vaddr_desc_32;
@@ -1404,7 +1404,7 @@ static int ath10k_htt_tx_64(struct ath10k_htt *htt,
case ATH10K_HW_TXRX_RAW:
case ATH10K_HW_TXRX_NATIVE_WIFI:
flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
-   /* pass through */
+   /* fall through */
case ATH10K_HW_TXRX_ETHERNET:
if (ar->hw_params.continuous_frag_desc) {
ext_desc_t = htt->frag_desc.vaddr_desc_64;
-- 
2.7.4



[PATCH] ath6kl: mark expected switch fall-throughs

2018-05-24 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c 
b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 2ba8cf3..29e32cd 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -3898,17 +3898,17 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
wiphy->max_scan_ie_len = 1000; /* FIX: what is correct limit? */
switch (ar->hw.cap) {
case WMI_11AN_CAP:
-   ht = true;
+   ht = true; /* fall through */
case WMI_11A_CAP:
band_5gig = true;
break;
case WMI_11GN_CAP:
-   ht = true;
+   ht = true; /* fall through */
case WMI_11G_CAP:
band_2gig = true;
break;
case WMI_11AGN_CAP:
-   ht = true;
+   ht = true; /* fall through */
case WMI_11AG_CAP:
band_2gig = true;
band_5gig = true;
-- 
2.7.4



Re: [PATCH] rtlwifi: remove duplicate code

2018-05-24 Thread Gustavo A. R. Silva

Hi Joe,

On 05/24/2018 02:24 PM, Joe Perches wrote:

On Thu, 2018-05-24 at 13:54 -0500, Gustavo A. R. Silva wrote:

Remove and refactor some code in order to avoid having identical code
for different branches.


True and nice tool and patch submittal thanks.


Notice that the logic has been there since 2014.


But perhaps the original logic is a defective copy/paste
and it should be corrected instead.

Can anyone from realtek verify this?



I actually used gitk to track down the last changes made to this code 
and, it doesn't look like a copy/paste issue:


commit: c6821613e653aae4f54c75689e229e3f063b7f69
commit: 27a31a60a4de4c1b45e371152bb6e701e1a8cc40

Thanks
--
Gustavo


Addresses-Coverity-ID: 1426199 ("Identical code for different branches")
Signed-off-by: Gustavo A. R. Silva 
---
  .../realtek/rtlwifi/btcoexist/halbtc8723b2ant.c| 23 --
  1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c
index 279fe01..df3facc 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c
@@ -2876,25 +2876,10 @@ static void btc8723b2ant_action_hid(struct btc_coexist 
*btcoexist)
btc8723b2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 13);
  
  	/* sw mechanism */

-   if (BTC_WIFI_BW_HT40 == wifi_bw) {
-   if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
-   (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
-   btc8723b2ant_sw_mechanism(btcoexist, true, true,
- false, false);
-   } else {
-   btc8723b2ant_sw_mechanism(btcoexist, true, true,
- false, false);
-   }
-   } else {
-   if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
-   (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
-   btc8723b2ant_sw_mechanism(btcoexist, false, true,
- false, false);
-   } else {
-   btc8723b2ant_sw_mechanism(btcoexist, false, true,
- false, false);
-   }
-   }
+   if (wifi_bw == BTC_WIFI_BW_HT40)
+   btc8723b2ant_sw_mechanism(btcoexist, true, true, false, false);
+   else
+   btc8723b2ant_sw_mechanism(btcoexist, false, true, false, false);
  }
  
  /* A2DP only / PAN(EDR) only/ A2DP+PAN(HS) */


[PATCH] rtlwifi: remove duplicate code

2018-05-24 Thread Gustavo A. R. Silva
Remove and refactor some code in order to avoid having identical code
for different branches.

Notice that the logic has been there since 2014.

Addresses-Coverity-ID: 1426199 ("Identical code for different branches")
Signed-off-by: Gustavo A. R. Silva 
---
 .../realtek/rtlwifi/btcoexist/halbtc8723b2ant.c| 23 --
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c
index 279fe01..df3facc 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c
@@ -2876,25 +2876,10 @@ static void btc8723b2ant_action_hid(struct btc_coexist 
*btcoexist)
btc8723b2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 13);
 
/* sw mechanism */
-   if (BTC_WIFI_BW_HT40 == wifi_bw) {
-   if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
-   (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
-   btc8723b2ant_sw_mechanism(btcoexist, true, true,
- false, false);
-   } else {
-   btc8723b2ant_sw_mechanism(btcoexist, true, true,
- false, false);
-   }
-   } else {
-   if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
-   (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
-   btc8723b2ant_sw_mechanism(btcoexist, false, true,
- false, false);
-   } else {
-   btc8723b2ant_sw_mechanism(btcoexist, false, true,
- false, false);
-   }
-   }
+   if (wifi_bw == BTC_WIFI_BW_HT40)
+   btc8723b2ant_sw_mechanism(btcoexist, true, true, false, false);
+   else
+   btc8723b2ant_sw_mechanism(btcoexist, false, true, false, false);
 }
 
 /* A2DP only / PAN(EDR) only/ A2DP+PAN(HS) */
-- 
2.7.4



[PATCH] staging: wilc1000: fix infinite loop and out-of-bounds access

2018-04-30 Thread Gustavo A. R. Silva
If i < slot_id is initially true then it will remain true. Also,
as i is being decremented it will end up accessing memory out of
bounds.

Fix this by incrementing *i* instead of decrementing it.

Addresses-Coverity-ID: 1468454 ("Infinite loop")
Fixes: faa657641081 ("staging: wilc1000: refactor scan() to free kmalloc
memory on failure cases")
Signed-off-by: Gustavo A. R. Silva 
---

BTW... at first sight it seems to me that variables slot_id
and i should be of type unsigned instead of signed.

 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 3ca0c97..67104e8 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -608,7 +608,7 @@ wilc_wfi_cfg_alloc_fill_ssid(struct cfg80211_scan_request 
*request,
 
 out_free:
 
-   for (i = 0; i < slot_id ; i--)
+   for (i = 0; i < slot_id; i++)
kfree(ntwk->net_info[i].ssid);
 
kfree(ntwk->net_info);
-- 
2.7.4



Re: [PATCH] rsi: fix a bug in rsi_hal_key_config()

2018-04-27 Thread Gustavo A. R. Silva

Hi Dan,

On 04/27/2018 06:44 AM, Dan Carpenter wrote:

On Fri, Apr 27, 2018 at 02:32:20PM +0300, Kalle Valo wrote:


Gustavo submitted an identical patch also for this one :)

https://patchwork.kernel.org/patch/10365997/



Hey Gustavo,

We keep on sending duplicate patches.  Most of the static checker people
CC kernel-janit...@vger.kernel.org so we can see what's already been
sent.



Oh, I didn't know that.  I'll start CCing that list then.

Thanks for letting me know.
--
Gustavo


[PATCH] rsi_91x_mac80211: fix structurally dead code

2018-04-26 Thread Gustavo A. R. Silva
Function rsi_hal_key_config returns before reaching code at line
922 if (status), hence this code is structurally dead.

Fix this by storing the value returned by rsi_hal_load_key
into _status_ for its further evaluation and use.

Addresses-Coverity-ID: 1468409 ("Structurally dead code")
Fixes: 4fd6c4762f37 ("rsi: roaming enhancements")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/rsi/rsi_91x_mac80211.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c 
b/drivers/net/wireless/rsi/rsi_91x_mac80211.c
index 766d874..80e7f4f 100644
--- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c
+++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c
@@ -911,14 +911,14 @@ static int rsi_hal_key_config(struct ieee80211_hw *hw,
}
}
 
-   return rsi_hal_load_key(adapter->priv,
-   key->key,
-   key->keylen,
-   key_type,
-   key->keyidx,
-   key->cipher,
-   sta_id,
-   vif);
+   status = rsi_hal_load_key(adapter->priv,
+ key->key,
+ key->keylen,
+ key_type,
+ key->keyidx,
+ key->cipher,
+ sta_id,
+ vif);
if (status)
return status;
 
-- 
2.7.4



[PATCH] rsi_91x_usb: fix uninitialized variable

2018-04-26 Thread Gustavo A. R. Silva
There is a potential execution path in which variable ret is returned
without being properly initialized previously.

Fix this by storing the value returned by function
rsi_usb_master_reg_write into _ret_.

Addresses-Coverity-ID: 1468407 ("Uninitialized scalar variable")
Fixes: 16d3bb7b2f37 ("rsi: disable fw watchdog timer during reset")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/rsi/rsi_91x_usb.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c 
b/drivers/net/wireless/rsi/rsi_91x_usb.c
index b065438..6ce6b75 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -687,9 +687,10 @@ static int rsi_reset_card(struct rsi_hw *adapter)
 */
msleep(100);
 
-   if (rsi_usb_master_reg_write(adapter, SWBL_REGOUT,
-RSI_FW_WDT_DISABLE_REQ,
-RSI_COMMON_REG_SIZE) < 0) {
+   ret = rsi_usb_master_reg_write(adapter, SWBL_REGOUT,
+  RSI_FW_WDT_DISABLE_REQ,
+  RSI_COMMON_REG_SIZE);
+   if (ret < 0) {
rsi_dbg(ERR_ZONE, "Disabling firmware watchdog timer failed\n");
goto fail;
}
-- 
2.7.4



Re: [PATCH] qtnfmac: pearl: pcie: fix memory leak in qtnf_fw_work_handler

2018-04-05 Thread Gustavo A. R. Silva

Hi Sergey,

On 04/05/2018 11:31 AM, Sergey Matyukevich wrote:

Hello Gustavo,


In case memory resources for fw were succesfully allocated, release
them before jumping to fw_load_fail.

Addresses-Coverity-ID: 1466092 ("Resource leak")
Fixes: c3b2f7ca4186 ("qtnfmac: implement asynchronous firmware loading")
Signed-off-by: Gustavo A. R. Silva 
---
  drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 4 
  1 file changed, 4 insertions(+)


Thanks for the patch!



Glad to help. :)


Reviewed-by: Sergey Matyukevich 



Thanks
--
Gustavo



[PATCH] qtnfmac: pearl: pcie: fix memory leak in qtnf_fw_work_handler

2018-04-05 Thread Gustavo A. R. Silva
In case memory resources for fw were succesfully allocated, release
them before jumping to fw_load_fail.

Addresses-Coverity-ID: 1466092 ("Resource leak")
Fixes: c3b2f7ca4186 ("qtnfmac: implement asynchronous firmware loading")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c 
b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index f117904..6c1e139 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -1185,6 +1185,10 @@ static void qtnf_fw_work_handler(struct work_struct 
*work)
if (qtnf_poll_state(&priv->bda->bda_ep_state, QTN_EP_FW_LOADRDY,
QTN_FW_DL_TIMEOUT_MS)) {
pr_err("card is not ready\n");
+
+   if (!flashboot)
+   release_firmware(fw);
+
goto fw_load_fail;
}
 
-- 
2.7.4



[rtlwifi-btcoex] Suspicious code in halbtc8821a1ant driver

2018-04-04 Thread Gustavo A. R. Silva
Hi all,

While doing some static analysis I came across the following piece of code at 
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:1581:

1581 static void btc8821a1ant_act_bt_sco_hid_only_busy(struct btc_coexist 
*btcoexist,
1582   u8 wifi_status)
1583 {
1584 /* tdma and coex table */
1585 btc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 5);
1586 
1587 if (BT_8821A_1ANT_WIFI_STATUS_NON_CONNECTED_ASSO_AUTH_SCAN ==
1588 wifi_status)
1589 btc8821a1ant_coex_table_with_type(btcoexist, NORMAL_EXEC, 
1);
1590 else
1591 btc8821a1ant_coex_table_with_type(btcoexist, NORMAL_EXEC, 
1);
1592 }

The issue here is that the code for both branches of the if-else statement is 
identical.

The if-else was introduced a year ago in this commit c6821613e653

I wonder if an argument should be changed in any of the calls to 
btc8821a1ant_coex_table_with_type?

What do you think?

Thanks
--
Gustavo


[PATCH] brcm80211: brcmsmac: phy_lcn: remove duplicate code

2018-04-04 Thread Gustavo A. R. Silva
Remove and refactor some code in order to avoid having identical code
for different branches.

Notice that this piece of code hasn't been modified since 2011.

Addresses-Coverity-ID: 1226756 ("Identical code for different branches")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c
index 93d4cde..9d830d2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c
@@ -3388,13 +3388,8 @@ void wlc_lcnphy_deaf_mode(struct brcms_phy *pi, bool 
mode)
u8 phybw40;
phybw40 = CHSPEC_IS40(pi->radio_chanspec);
 
-   if (LCNREV_LT(pi->pubpi.phy_rev, 2)) {
-   mod_phy_reg(pi, 0x4b0, (0x1 << 5), (mode) << 5);
-   mod_phy_reg(pi, 0x4b1, (0x1 << 9), 0 << 9);
-   } else {
-   mod_phy_reg(pi, 0x4b0, (0x1 << 5), (mode) << 5);
-   mod_phy_reg(pi, 0x4b1, (0x1 << 9), 0 << 9);
-   }
+   mod_phy_reg(pi, 0x4b0, (0x1 << 5), (mode) << 5);
+   mod_phy_reg(pi, 0x4b1, (0x1 << 9), 0 << 9);
 
if (phybw40 == 0) {
mod_phy_reg((pi), 0x410,
-- 
2.7.4



[PATCH] mt7601u: phy: mark expected switch fall-through

2018-03-30 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/mediatek/mt7601u/phy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/mediatek/mt7601u/phy.c 
b/drivers/net/wireless/mediatek/mt7601u/phy.c
index ca09a5d..9a90f1f 100644
--- a/drivers/net/wireless/mediatek/mt7601u/phy.c
+++ b/drivers/net/wireless/mediatek/mt7601u/phy.c
@@ -795,6 +795,7 @@ mt7601u_phy_rf_pa_mode_val(struct mt7601u_dev *dev, int 
phy_mode, int tx_rate)
switch (phy_mode) {
case MT_PHY_TYPE_OFDM:
tx_rate += 4;
+   /* fall through */
case MT_PHY_TYPE_CCK:
reg = dev->rf_pa_mode[0];
break;
-- 
2.7.4



[PATCH] ath9k: dfs: remove accidental use of stack VLA

2018-03-30 Thread Gustavo A. R. Silva
In preparation to enabling -Wvla, remove accidental use of stack VLA.

This avoids an accidental stack VLA (since the compiler thinks
the value of FFT_NUM_SAMPLES can change, even when marked
"const"). This just replaces it with a #define.

Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/dfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/dfs.c 
b/drivers/net/wireless/ath/ath9k/dfs.c
index 6fee9a4..c8844f5 100644
--- a/drivers/net/wireless/ath/ath9k/dfs.c
+++ b/drivers/net/wireless/ath/ath9k/dfs.c
@@ -41,7 +41,7 @@ static const int BIN_DELTA_MAX= 10;
 
 /* we need at least 3 deltas / 4 samples for a reliable chirp detection */
 #define NUM_DIFFS 3
-static const int FFT_NUM_SAMPLES   = (NUM_DIFFS + 1);
+#define FFT_NUM_SAMPLES(NUM_DIFFS + 1)
 
 /* Threshold for difference of delta peaks */
 static const int MAX_DIFF  = 2;
@@ -114,7 +114,7 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 
*data,
 
ath_dbg(common, DFS, "HT40: datalen=%d, num_fft_packets=%d\n",
datalen, num_fft_packets);
-   if (num_fft_packets < (FFT_NUM_SAMPLES)) {
+   if (num_fft_packets < FFT_NUM_SAMPLES) {
ath_dbg(common, DFS, "not enough packets for chirp\n");
return false;
}
@@ -136,7 +136,7 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 
*data,
return false;
ath_dbg(common, DFS, "HT20: datalen=%d, num_fft_packets=%d\n",
datalen, num_fft_packets);
-   if (num_fft_packets < (FFT_NUM_SAMPLES)) {
+   if (num_fft_packets < FFT_NUM_SAMPLES) {
ath_dbg(common, DFS, "not enough packets for chirp\n");
return false;
}
-- 
2.7.4



Re: [PATCH] mac80211: aes-cmac: remove VLA usage

2018-03-21 Thread Gustavo A. R. Silva



On 03/21/2018 08:58 AM, Johannes Berg wrote:

On Wed, 2018-03-21 at 08:57 -0500, Gustavo A. R. Silva wrote:


SHA_DESC_ON_STACK is currently being used in multiple places. But, yeah,
I think we can define multiple macros of the same kind and adjust to the
characteristics of each the component.

How big do you think tfm can get?


I have no idea, I guess you'll have to take that with Herbert.

johannes



I see. I'll contact him then.

Thanks for the feedback.
--
Gustavo


Re: [PATCH] mac80211: aes-cmac: remove VLA usage

2018-03-21 Thread Gustavo A. R. Silva



On 03/21/2018 08:48 AM, Johannes Berg wrote:

On Wed, 2018-03-21 at 08:42 -0500, Gustavo A. R. Silva wrote:

In preparation to enabling -Wvla, remove VLAs and replace them
with dynamic memory allocation instead.

The use of stack Variable Length Arrays needs to be avoided, as they
can be a vector for stack exhaustion, which can be both a runtime bug
or a security flaw. Also, in general, as code evolves it is easy to
lose track of how big a VLA can get. Thus, we can end up having runtime
failures that are hard to debug.

Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Gustavo A. R. Silva 
---
  net/mac80211/aes_cmac.c | 36 
  1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index 2fb6558..c9444bf 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -27,30 +27,42 @@ static const u8 zero[CMAC_TLEN_256];
  void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
const u8 *data, size_t data_len, u8 *mic)
  {
-   SHASH_DESC_ON_STACK(desc, tfm);
+   struct shash_desc *shash;
u8 out[AES_BLOCK_SIZE];
  
-	desc->tfm = tfm;

+   shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm),
+   GFP_KERNEL);
+   if (!shash)
+   return;


Honestly, this seems like a really bad idea - you're now hitting
kmalloc for every TX/RX frame here.

SHA_DESC_ON_STACK() should just be fixed to not need a VLA, but take
some sort of maximum, I guess?



SHA_DESC_ON_STACK is currently being used in multiple places. But, yeah, 
I think we can define multiple macros of the same kind and adjust to the 
characteristics of each the component.


How big do you think tfm can get?

Thanks
--
Gustavo


[PATCH] mac80211: aes-cmac: remove VLA usage

2018-03-21 Thread Gustavo A. R. Silva
In preparation to enabling -Wvla, remove VLAs and replace them
with dynamic memory allocation instead.

The use of stack Variable Length Arrays needs to be avoided, as they
can be a vector for stack exhaustion, which can be both a runtime bug
or a security flaw. Also, in general, as code evolves it is easy to
lose track of how big a VLA can get. Thus, we can end up having runtime
failures that are hard to debug.

Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Gustavo A. R. Silva 
---
 net/mac80211/aes_cmac.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index 2fb6558..c9444bf 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -27,30 +27,42 @@ static const u8 zero[CMAC_TLEN_256];
 void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
const u8 *data, size_t data_len, u8 *mic)
 {
-   SHASH_DESC_ON_STACK(desc, tfm);
+   struct shash_desc *shash;
u8 out[AES_BLOCK_SIZE];
 
-   desc->tfm = tfm;
+   shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm),
+   GFP_KERNEL);
+   if (!shash)
+   return;
 
-   crypto_shash_init(desc);
-   crypto_shash_update(desc, aad, AAD_LEN);
-   crypto_shash_update(desc, data, data_len - CMAC_TLEN);
-   crypto_shash_finup(desc, zero, CMAC_TLEN, out);
+   shash->tfm = tfm;
+
+   crypto_shash_init(shash);
+   crypto_shash_update(shash, aad, AAD_LEN);
+   crypto_shash_update(shash, data, data_len - CMAC_TLEN);
+   crypto_shash_finup(shash, zero, CMAC_TLEN, out);
 
memcpy(mic, out, CMAC_TLEN);
+   kfree(shash);
 }
 
 void ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
const u8 *data, size_t data_len, u8 *mic)
 {
-   SHASH_DESC_ON_STACK(desc, tfm);
+   struct shash_desc *shash;
+
+   shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm),
+   GFP_KERNEL);
+   if (!shash)
+   return;
 
-   desc->tfm = tfm;
+   shash->tfm = tfm;
 
-   crypto_shash_init(desc);
-   crypto_shash_update(desc, aad, AAD_LEN);
-   crypto_shash_update(desc, data, data_len - CMAC_TLEN_256);
-   crypto_shash_finup(desc, zero, CMAC_TLEN_256, mic);
+   crypto_shash_init(shash);
+   crypto_shash_update(shash, aad, AAD_LEN);
+   crypto_shash_update(shash, data, data_len - CMAC_TLEN_256);
+   crypto_shash_finup(shash, zero, CMAC_TLEN_256, mic);
+   kfree(shash);
 }
 
 struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[],
-- 
2.7.4



Re: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage

2018-03-10 Thread Gustavo A. R. Silva



On 03/10/2018 05:12 PM, Kees Cook wrote:

On Sat, Mar 10, 2018 at 3:06 PM, Arend van Spriel
 wrote:

On 3/9/2018 1:30 PM, Andreas Christoforou wrote:


The kernel would like to have all stack VLA usage removed.



I think there was a remark made earlier to give more explanation here. It
should explain why we want "VLA on stack" removed.


Signed-off-by: Andreas Christoforou 
---
   drivers/net/wireless/ath/ath9k/dfs.c | 3 +--
   1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/dfs.c
b/drivers/net/wireless/ath/ath9k/dfs.c
index 6fee9a4..cfb0f84 100644
--- a/drivers/net/wireless/ath/ath9k/dfs.c
+++ b/drivers/net/wireless/ath/ath9k/dfs.c
@@ -41,7 +41,6 @@ static const int BIN_DELTA_MAX= 10;

   /* we need at least 3 deltas / 4 samples for a reliable chirp detection
*/
   #define NUM_DIFFS 3
-static const int FFT_NUM_SAMPLES   = (NUM_DIFFS + 1);



What about this instead?

#define FFT_NUM_SAMPLES (NUM_DIFFS + 1)


   /* Threshold for difference of delta peaks */
   static const int MAX_DIFF = 2;
@@ -101,7 +100,7 @@ static bool ath9k_check_chirping(struct ath_softc *sc,
u8 *data,
  int datalen, bool is_ctl, bool is_ext)
   {
 int i;
-   int max_bin[FFT_NUM_SAMPLES];
+   int max_bin[NUM_DIFFS + 1];



Just wondering. Is this actually a VLA. FFT_NUM_SAMPLES was static const so
not really going to show a lot of variation. This array will always have the
same size on the stack.


The problem is that it's not a "constant expression", so the compiler
frontend still yells about it under -Wvla. I would characterize this
mainly as a fix for "accidental VLA" or "misdetected VLA" or something
like that. AIUI, there really isn't a functional change here.

-Kees



--
Gustavo


[PATCH] ssb: return boolean instead of integer in ssb_dma_translation_special_bit

2018-01-18 Thread Gustavo A. R. Silva
Return statements in functions returning bool should use
true/false instead of 1/0.

This issue was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ssb/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index 65420a9..fdb89b6 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -1116,7 +1116,7 @@ static bool ssb_dma_translation_special_bit(struct 
ssb_device *dev)
chip_id == 43231 || chip_id == 43222);
}
 
-   return 0;
+   return false;
 }
 
 u32 ssb_dma_translation(struct ssb_device *dev)
-- 
2.7.4



[PATCH] rsi: rsi_91x_ps: remove redundant code in str_psstate

2017-11-06 Thread Gustavo A. R. Silva
"INVALID_STATE" is already being returned in the default case and this
code cannot be reached.

Addresses-Coverity-ID: 1398384
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/rsi/rsi_91x_ps.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_ps.c 
b/drivers/net/wireless/rsi/rsi_91x_ps.c
index 523f532..01472fa 100644
--- a/drivers/net/wireless/rsi/rsi_91x_ps.c
+++ b/drivers/net/wireless/rsi/rsi_91x_ps.c
@@ -36,7 +36,6 @@ char *str_psstate(enum ps_state state)
default:
return "INVALID_STATE";
}
-   return "INVALID_STATE";
 }
 
 static inline void rsi_modify_ps_state(struct rsi_hw *adapter,
-- 
2.7.4



[PATCH] ath9k: dfs: use swap macro in ath9k_check_chirping

2017-11-03 Thread Gustavo A. R. Silva
Make use of the swap macro and remove unnecessary variable temp.
This makes the code easier to read and maintain.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/dfs.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/dfs.c 
b/drivers/net/wireless/ath/ath9k/dfs.c
index 40a397f..6fee9a4 100644
--- a/drivers/net/wireless/ath/ath9k/dfs.c
+++ b/drivers/net/wireless/ath/ath9k/dfs.c
@@ -123,11 +123,9 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 
*data,
fft = (struct ath9k_dfs_fft_40 *) (data + 2);
ath_dbg(common, DFS, "fixing datalen by 2\n");
}
-   if (IS_CHAN_HT40MINUS(ah->curchan)) {
-   int temp = is_ctl;
-   is_ctl = is_ext;
-   is_ext = temp;
-   }
+   if (IS_CHAN_HT40MINUS(ah->curchan))
+   swap(is_ctl, is_ext);
+
for (i = 0; i < FFT_NUM_SAMPLES; i++)
max_bin[i] = ath9k_get_max_index_ht40(fft + i, is_ctl,
  is_ext);
-- 
2.7.4



[PATCH] net: wireless: mark expected switch fall-throughs

2017-10-20 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Signed-off-by: Gustavo A. R. Silva 
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.

 net/wireless/chan.c|  2 ++
 net/wireless/nl80211.c | 10 ++
 net/wireless/scan.c|  3 ++-
 net/wireless/wext-compat.c |  2 ++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index eb82427..6072613 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -741,6 +741,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
case NL80211_CHAN_WIDTH_20:
if (!ht_cap->ht_supported)
return false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20_NOHT:
prohibited_flags |= IEEE80211_CHAN_NO_20MHZ;
width = 20;
@@ -763,6 +764,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK;
if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)
return false;
+   /* fall through */
case NL80211_CHAN_WIDTH_80:
if (!vht_cap->vht_supported)
return false;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fce2cbe..a8bbb6c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1509,6 +1509,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 1:
if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES,
sizeof(u32) * rdev->wiphy.n_cipher_suites,
@@ -1555,6 +1556,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 2:
if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES,
rdev->wiphy.interface_modes))
@@ -1562,6 +1564,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 3:
nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS);
if (!nl_bands)
@@ -1587,6 +1590,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->chan_start++;
if (state->split)
break;
+   /* fall through */
default:
/* add frequencies */
nl_freqs = nla_nest_start(
@@ -1640,6 +1644,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 4:
nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS);
if (!nl_cmds)
@@ -1666,6 +1671,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 5:
if (rdev->ops->remain_on_channel &&
(rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL) &&
@@ -1683,6 +1689,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 6:
 #ifdef CONFIG_PM
if (nl80211_send_wowlan(msg, rdev, state->split))
@@ -1693,6 +1700,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
 #else
state->split_start++;
 #endif
+   /* fall through */
case 7:
if (nl80211_put_iftypes(msg, NL80211_ATTR_SOFTWARE_IFTYPES,
rdev->wiphy.software_iftypes))
@@ -1705,6 +1713,7 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
state->split_start++;
if (state->split)
break;
+   /* fall through */
case 8:
if ((rdev->wiphy.flags & WIPHY_FLAG_HAVE_AP_SME) &&

[PATCH] net: mac80211: mark expected switch fall-throughs

2017-10-17 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Notice that in some cases I replaced "fall through on else" and
"otherwise fall through" comments with just a "fall through" comment,
which is what GCC is expecting to find.

Signed-off-by: Gustavo A. R. Silva 
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify that the actual intention of the code is to fall through.

 net/mac80211/cfg.c| 3 +++
 net/mac80211/ht.c | 1 +
 net/mac80211/iface.c  | 2 +-
 net/mac80211/mesh.c   | 2 ++
 net/mac80211/mesh_hwmp.c  | 1 +
 net/mac80211/mesh_plink.c | 2 +-
 net/mac80211/mlme.c   | 1 +
 net/mac80211/offchannel.c | 4 ++--
 net/mac80211/tdls.c   | 1 +
 net/mac80211/wme.c| 1 +
 10 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a354f19..9bd8bef 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -573,10 +573,12 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct 
net_device *dev,
case WLAN_CIPHER_SUITE_BIP_CMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
 offsetof(typeof(kseq), aes_cmac));
+   /* fall through */
case WLAN_CIPHER_SUITE_BIP_GMAC_128:
case WLAN_CIPHER_SUITE_BIP_GMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
 offsetof(typeof(kseq), aes_gmac));
+   /* fall through */
case WLAN_CIPHER_SUITE_GCMP:
case WLAN_CIPHER_SUITE_GCMP_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
@@ -2205,6 +2207,7 @@ static int ieee80211_scan(struct wiphy *wiphy,
 * for now fall through to allow scanning only when
 * beaconing hasn't been configured yet
 */
+   /* fall through */
case NL80211_IFTYPE_AP:
/*
 * If the scan has been forced (and the driver supports
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 41f5e48..e55dabf 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -491,6 +491,7 @@ int ieee80211_send_smps_action(struct ieee80211_sub_if_data 
*sdata,
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_NUM_MODES:
WARN_ON(1);
+   /* fall through */
case IEEE80211_SMPS_OFF:
action_frame->u.action.u.ht_smps.smps_control =
WLAN_HT_SMPS_CONTROL_DISABLED;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 13b16f9..435e735 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1633,7 +1633,7 @@ static void ieee80211_assign_perm_addr(struct 
ieee80211_local *local,
goto out_unlock;
}
}
-   /* otherwise fall through */
+   /* fall through */
default:
/* assign a new address if possible -- try n_addresses first */
for (i = 0; i < local->hw.wiphy->n_addresses; i++) {
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 7a76c4a..d29a545 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -988,8 +988,10 @@ ieee80211_mesh_process_chnswitch(struct 
ieee80211_sub_if_data *sdata,
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
sta_flags |= IEEE80211_STA_DISABLE_HT;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
sta_flags |= IEEE80211_STA_DISABLE_40MHZ;
+   /* fall through */
case NL80211_CHAN_WIDTH_40:
sta_flags |= IEEE80211_STA_DISABLE_VHT;
break;
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 146ec6c..0e75abf 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -1247,6 +1247,7 @@ void mesh_path_tx_root_frame(struct ieee80211_sub_if_data 
*sdata)
break;
case IEEE80211_PROACTIVE_PREQ_WITH_PREP:
flags |= IEEE80211_PREQ_PROACTIVE_PREP_FLAG;
+   /* fall through */
case IEEE80211_PROACTIVE_PREQ_NO_PREP:
interval = ifmsh->mshcfg.dot11MeshHWMPactivePathToRootTimeout;
target_flags |= IEEE80211_PREQ_TO_FLAG |
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index e2d00cc..0f6c9ca 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -672,7 +672,7 @@ void mesh_plink_timer(struct timer_list *t)
break;
}
reason = WLAN_REASON_MESH_MAX_RETRIES;
-   /* fall through on else */
+   /* fall through */
case NL80211_PLINK_CNF_RCVD:
/* confirm timer */
if (!reason)
diff --git a/net/mac80211/mlme.c b/net/

Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs

2017-10-11 Thread Gustavo A. R. Silva

Hi Jes,

Quoting Jes Sorensen :


On 10/11/2017 04:41 AM, Kalle Valo wrote:

Jes Sorensen  writes:


On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.


While this isn't harmful, to me this looks like pointless patch churn
for zero gain and it's just ugly.


In general I find it useful to mark fall through cases. And it's just a
comment with two words, so they cannot hurt your eyes that much.


I don't see them being harmful in the code, but I don't see them of  
much use either. If it happened as part of natural code development,  
fine. My objection is to people running around doing this  
systematically causing patch churn for little to zero gain.


Jes



I understand that you think this is of zero gain for you, but as  
Florian Fainelli pointed out:


"That is the canonical way to tell static analyzers and compilers that
fall throughs are wanted and not accidental mistakes in the code. For
people that deal with these kinds of errors, it's quite helpful, unless
you suggest disabling that particular GCC warning specific for that
file/directory?"

this is very helpful for people working on fixing issues reported by  
static analyzers. It saves a huge amount of time when dealing with  
False Positives. Also, there are cases when an apparently intentional  
fall-through turns out to be an actual missing break or continue.


So there is an ongoing effort to detect such cases and avoid them to  
show up in the future by at least warning people about a potential  
issue in their code. And this is helpful for everybody.


Thanks
--
Gustavo A. R. Silva







[PATCH] rtl8xxxu: mark expected switch fall-throughs

2017-10-10 Thread Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Cc: Jes Sorensen 
Cc: Kalle Valo 
Cc: linux-wireless@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 7806a4d..e66be05 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw)
switch (hw->conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
ht = false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
opmode |= BW_OPMODE_20MHZ;
rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
@@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
switch (hw->conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
ht = false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
subchannel = 0;
@@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv 
*priv)
case 3:
priv->ep_tx_low_queue = 1;
priv->ep_tx_count++;
+   /* fall through */
case 2:
priv->ep_tx_normal_queue = 1;
priv->ep_tx_count++;
+   /* fall through */
case 1:
priv->ep_tx_high_queue = 1;
priv->ep_tx_count++;
@@ -5691,6 +5695,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum 
set_key_cmd cmd,
break;
case WLAN_CIPHER_SUITE_TKIP:
key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
+   /* fall through */
default:
return -EOPNOTSUPP;
}
-- 
2.7.4



Re: [PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches

2017-09-07 Thread Gustavo A. R. Silva

Hi Larry,

On 08/30/2017 11:48 PM, Larry Finger wrote:

On 08/30/2017 08:42 AM, Gustavo A. R. Silva wrote:

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1226788
Signed-off-by: Gustavo A. R. Silva 
---
This issue was reported by Coverity and it was tested by compilation
only.
I'm suspicious this may be a copy/paste error. Please, verify.

  .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c   | 10
++
  1 file changed, 2 insertions(+), 8 deletions(-)


This change is not correct. When bt_link_info->sco_exist is true, the
call should be

halbtc8723b1ant_limited_rx(btcoexist,
   NORMAL_EXEC, true,
   false, 0x5);

NACK

I will push the correct patch.



Great. Good to know.

Thanks
--
Gustavo A. R. Silva


[PATCH] rtlwifi: rtl8723be: fix duplicated code for different branches

2017-08-30 Thread Gustavo A. R. Silva
Refactor code in order to avoid identical code for different branches.

Addresses-Coverity-ID: 1248728
Signed-off-by: Gustavo A. R. Silva 
---
This issue was reported by Coverity and it was tested by compilation only.
Please, verify if this is not a copy/paste error.
Also, notice this code has been there since 2014.

 drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c 
b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c
index 131c0d1..15c117e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c
@@ -883,12 +883,8 @@ static void 
rtl8723be_dm_txpower_tracking_callback_thermalmeter(
if ((rtldm->power_index_offset[RF90_PATH_A] != 0) &&
(rtldm->txpower_track_control)) {
rtldm->done_txpower = true;
-   if (thermalvalue > rtlefuse->eeprom_thermalmeter)
-   rtl8723be_dm_tx_power_track_set_power(hw, BBSWING, 0,
-index_for_channel);
-   else
-   rtl8723be_dm_tx_power_track_set_power(hw, BBSWING, 0,
-index_for_channel);
+   rtl8723be_dm_tx_power_track_set_power(hw, BBSWING, 0,
+ index_for_channel);
 
rtldm->swing_idx_cck_base = rtldm->swing_idx_cck;
rtldm->swing_idx_ofdm_base[RF90_PATH_A] =
-- 
2.5.0



Re: [PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches

2017-08-30 Thread Gustavo A. R. Silva

Hi Larry,

On 08/30/2017 11:37 AM, Larry Finger wrote:

On 08/30/2017 08:42 AM, Gustavo A. R. Silva wrote:

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1226788
Signed-off-by: Gustavo A. R. Silva 
---
This issue was reported by Coverity and it was tested by compilation
only.
I'm suspicious this may be a copy/paste error. Please, verify.


I have referred this change to the engineers at Realtek. For the moment,
please hold this patch.

Thanks for reporting the condition.



Glad to help. :)

--
Gustavo A. R. Silva


[PATCH] rtlwifi: refactor code in halbtcoutsrc module

2017-08-30 Thread Gustavo A. R. Silva
Function halbtc_get_wifi_rssi always returns rtlpriv->dm.undec_sm_pwdb.
So this function can be removed and the value of
rtlpriv->dm.undec_sm_pwdb assigned to *s32_tmp directly.

This issue was first reported by Coverity as "identical code for different
branches" in function halbtc_get_wifi_rssi.

Addresses-Coverity-ID: 1226793
Signed-off-by: Gustavo A. R. Silva 
---
This code was reported by Coverity and it was tested by compilation only.
Chances are this may be a copy/paste error in function
halbtc_get_wifi_rssi. Please, verify.
Also, notice this code has been there since 2014.

 .../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c   | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index c1eacd8..2a47b97 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -373,17 +373,6 @@ u32 halbtc_get_wifi_link_status(struct btc_coexist 
*btcoexist)
return ret_val;
 }
 
-static s32 halbtc_get_wifi_rssi(struct rtl_priv *rtlpriv)
-{
-   int undec_sm_pwdb = 0;
-
-   if (rtlpriv->mac80211.link_state >= MAC80211_LINKED)
-   undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb;
-   else /* associated entry pwdb */
-   undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb;
-   return undec_sm_pwdb;
-}
-
 static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf)
 {
struct btc_coexist *btcoexist = (struct btc_coexist *)void_btcoexist;
@@ -479,7 +468,7 @@ static bool halbtc_get(void *void_btcoexist, u8 get_type, 
void *out_buf)
*bool_tmp = false;
break;
case BTC_GET_S4_WIFI_RSSI:
-   *s32_tmp = halbtc_get_wifi_rssi(rtlpriv);
+   *s32_tmp = rtlpriv->dm.undec_sm_pwdb;
break;
case BTC_GET_S4_HS_RSSI:
*s32_tmp = 0;
-- 
2.5.0



[PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches

2017-08-30 Thread Gustavo A. R. Silva
Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1226788
Signed-off-by: Gustavo A. R. Silva 
---
This issue was reported by Coverity and it was tested by compilation only.
I'm suspicious this may be a copy/paste error. Please, verify.

 .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c   | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c
index c044252..960ce80f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c
@@ -2260,14 +2260,8 @@ static void halbtc8723b1ant_run_coexist_mechanism(struct 
btc_coexist *btcoexist)
 
if (iot_peer != BTC_IOT_PEER_CISCO &&
iot_peer != BTC_IOT_PEER_BROADCOM) {
-   if (bt_link_info->sco_exist)
-   halbtc8723b1ant_limited_rx(btcoexist,
-  NORMAL_EXEC, false,
-  false, 0x5);
-   else
-   halbtc8723b1ant_limited_rx(btcoexist,
-  NORMAL_EXEC, false,
-  false, 0x5);
+   halbtc8723b1ant_limited_rx(btcoexist, NORMAL_EXEC,
+  false, false, 0x5);
} else {
if (bt_link_info->sco_exist) {
halbtc8723b1ant_limited_rx(btcoexist,
-- 
2.5.0



[PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches

2017-08-17 Thread Gustavo A. R. Silva
Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1415177
Signed-off-by: Gustavo A. R. Silva 
---
This issue was reported by Coverity and it was tested by compilation only.

 .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c   | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c
index 03998d2..c044252 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c
@@ -600,14 +600,8 @@ static void halbtc8723b1ant_coex_table_with_type(struct 
btc_coexist *btcoexist,
   0xff, 0x3);
break;
case 5:
-   if ((coex_sta->cck_ever_lock) && (coex_sta->scan_ap_num <= 5))
-   halbtc8723b1ant_coex_table(btcoexist, force_exec,
-  0x5a5a5a5a, 0x5aaa5a5a,
-  0xff, 0x3);
-   else
-   halbtc8723b1ant_coex_table(btcoexist, force_exec,
-  0x5a5a5a5a, 0x5aaa5a5a,
-  0xff, 0x3);
+   halbtc8723b1ant_coex_table(btcoexist, force_exec, 0x5a5a5a5a,
+  0x5aaa5a5a, 0xff, 0x3);
break;
case 6:
halbtc8723b1ant_coex_table(btcoexist, force_exec, 0x,
-- 
2.5.0



[PATCH] rtlwifi: remove useless code

2017-07-18 Thread Gustavo A. R. Silva
Remove useless local variables last_read_point and last_txw_point and
the code related.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c 
b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c
index 55f238a..c58393e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c
@@ -478,7 +478,6 @@ u16 rtl92ee_rx_desc_buff_remained_cnt(struct ieee80211_hw 
*hw, u8 queue_index)
struct rtl_priv *rtlpriv = rtl_priv(hw);
u16 read_point = 0, write_point = 0, remind_cnt = 0;
u32 tmp_4byte = 0;
-   static u16 last_read_point;
static bool start_rx;
 
tmp_4byte = rtl_read_dword(rtlpriv, REG_RXQ_TXBD_IDX);
@@ -506,7 +505,6 @@ u16 rtl92ee_rx_desc_buff_remained_cnt(struct ieee80211_hw 
*hw, u8 queue_index)
 
rtlpci->rx_ring[queue_index].next_rx_rp = write_point;
 
-   last_read_point = read_point;
return remind_cnt;
 }
 
@@ -917,7 +915,6 @@ void rtl92ee_set_desc(struct ieee80211_hw *hw, u8 *pdesc, 
bool istx,
struct rtl_priv *rtlpriv = rtl_priv(hw);
u16 cur_tx_rp = 0;
u16 cur_tx_wp = 0;
-   static u16 last_txw_point;
static bool over_run;
u32 tmp = 0;
u8 q_idx = *val;
@@ -951,9 +948,6 @@ void rtl92ee_set_desc(struct ieee80211_hw *hw, u8 *pdesc, 
bool istx,
rtl_write_word(rtlpriv,
   get_desc_addr_fr_q_idx(q_idx),
   ring->cur_tx_wp);
-
-   if (q_idx == 1)
-   last_txw_point = cur_tx_wp;
}
 
if (ring->avl_desc < (max_tx_desc - 15)) {
-- 
2.5.0



[PATCH] wireless: airo: remove unnecessary static in writerids()

2017-07-18 Thread Gustavo A. R. Silva
Remove unnecessary static on local function pointer _writer_.
Such pointer is initialized before being used, on every
execution path throughout the function. The static has no
benefit and, removing it reduces the object file size.

This issue was detected using Coccinelle and the following semantic patch:

@bad exists@
position p;
identifier x;
type T;
@@

static T x@p;
...
x = <+...x...+>

@@
identifier x;
expression e;
type T;
position p != bad.p;
@@

-static
 T x@p;
 ... when != x
 when strict
?x = e;

In the following log you can see a significant difference in the object
file size. This log is the output of the size command, before and after
the code change:

before:
   textdata bss dec hex filename
 113797   191521216  134165   20c15 drivers/net/wireless/cisco/airo.o

after:
   textdata bss dec hex filename
 113881   190961152  134129   20bf1 drivers/net/wireless/cisco/airo.o

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/cisco/airo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/cisco/airo.c 
b/drivers/net/wireless/cisco/airo.c
index 84143a0..1066d84 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -7837,7 +7837,7 @@ static int writerids(struct net_device *dev, 
aironet_ioctl *comp) {
struct airo_info *ai = dev->ml_priv;
int  ridcode;
 int  enabled;
-   static int (* writer)(struct airo_info *, u16 rid, const void *, int, 
int);
+   int (*writer)(struct airo_info *, u16 rid, const void *, int, int);
unsigned char *iobuf;
 
/* Only super-user can write RIDs */
-- 
2.5.0



Re: ti: wl18xx: add checks on wl18xx_top_reg_write() return value

2017-06-28 Thread Gustavo A. R. Silva


Quoting Kalle Valo :


"Gustavo A. R. Silva"  wrote:


Check return value from call to wl18xx_top_reg_write(),
so in case of error jump to goto label out and return.

Also, remove unnecessary value check before goto label out.

Addresses-Coverity-ID: 1226938
Signed-off-by: Gustavo A. R. Silva 


The prefix should be "wl18xx:", I'll fix that.



Thanks, Kalle.
--
Gustavo A. R. Silva







[PATCH] ath9k: remove useless variable assignment in ath_mci_intr()

2017-06-26 Thread Gustavo A. R. Silva
Value assigned to variable offset at line 551 is overwritten at line 562,
before it can be used. This makes such variable assignment useless.

Addresses-Coverity-ID: 1226941
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ath/ath9k/mci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/mci.c 
b/drivers/net/wireless/ath/ath9k/mci.c
index 66596b9..cf23fd8 100644
--- a/drivers/net/wireless/ath/ath9k/mci.c
+++ b/drivers/net/wireless/ath/ath9k/mci.c
@@ -548,7 +548,7 @@ void ath_mci_intr(struct ath_softc *sc)
 
if (mci_int_rxmsg & AR_MCI_INTERRUPT_RX_MSG_SCHD_INFO) {
mci_int_rxmsg &= ~AR_MCI_INTERRUPT_RX_MSG_SCHD_INFO;
-   offset = ar9003_mci_state(ah, MCI_STATE_LAST_SCHD_MSG_OFFSET);
+   ar9003_mci_state(ah, MCI_STATE_LAST_SCHD_MSG_OFFSET);
}
 
if (mci_int_rxmsg & AR_MCI_INTERRUPT_RX_MSG_GPM) {
-- 
2.5.0



[PATCH] ti: wl18xx: add checks on wl18xx_top_reg_write() return value

2017-06-26 Thread Gustavo A. R. Silva
Check return value from call to wl18xx_top_reg_write(),
so in case of error jump to goto label out and return.

Also, remove unnecessary value check before goto label out.

Addresses-Coverity-ID: 1226938
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/ti/wl18xx/main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ti/wl18xx/main.c 
b/drivers/net/wireless/ti/wl18xx/main.c
index d1aa3ee..0cf3b40 100644
--- a/drivers/net/wireless/ti/wl18xx/main.c
+++ b/drivers/net/wireless/ti/wl18xx/main.c
@@ -793,9 +793,13 @@ static int wl18xx_set_clk(struct wl1271 *wl)
ret = wl18xx_top_reg_write(wl, PLLSH_WCS_PLL_P_FACTOR_CFG_2,
(wl18xx_clk_table[clk_freq].p >> 16) &
PLLSH_WCS_PLL_P_FACTOR_CFG_2_MASK);
+   if (ret < 0)
+   goto out;
} else {
ret = wl18xx_top_reg_write(wl, PLLSH_WCS_PLL_SWALLOW_EN,
   PLLSH_WCS_PLL_SWALLOW_EN_VAL2);
+   if (ret < 0)
+   goto out;
}
 
/* choose WCS PLL */
@@ -819,8 +823,6 @@ static int wl18xx_set_clk(struct wl1271 *wl)
/* reset the swallowing logic */
ret = wl18xx_top_reg_write(wl, PLLSH_COEX_PLL_SWALLOW_EN,
   PLLSH_COEX_PLL_SWALLOW_EN_VAL2);
-   if (ret < 0)
-   goto out;
 
 out:
return ret;
-- 
2.5.0



Re: [PATCH] nfc: nci: remove unnecessary null check

2017-06-22 Thread Gustavo A. R. Silva

Hi Samuel,

Quoting Samuel Ortiz :


Hi Gustavo,

On Tue, Jun 13, 2017 at 11:37:18AM -0500, Gustavo A. R. Silva wrote:

Remove unnecessary NULL check for pointer conn_info.
conn_info is set in list_for_each_entry() using container_of(),
which is never NULL.

Addresses-Coverity-ID: 1362349
Cc: Guenter Roeck 
Signed-off-by: Gustavo A. R. Silva 
---
 net/nfc/nci/core.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

Applied to nfc-next, thanks.



Absolutely, glad to help.

Thanks
--
Gustavo A. R. Silva







[PATCH] rtlwifi: rtl8821ae: remove unused variable

2017-06-13 Thread Gustavo A. R. Silva
Remove unused variable rtlhal.

Addresses-Coverity-ID: 1248810
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c 
b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 2bc6bac..d158e34 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1360,7 +1360,6 @@ static bool _rtl8821ae_reset_pcie_interface_dma(struct 
ieee80211_hw *hw,
 static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw)
 {
struct rtl_priv *rtlpriv = rtl_priv(hw);
-   struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
struct rtl_ps_ctl *ppsc = rtl_psc(rtlpriv);
u8 fw_reason = 0;
struct timeval ts;
@@ -1372,8 +1371,6 @@ static void _rtl8821ae_get_wakeup_reason(struct 
ieee80211_hw *hw)
 
ppsc->wakeup_reason = 0;
 
-   rtlhal->last_suspend_sec = ts.tv_sec;
-
switch (fw_reason) {
case FW_WOW_V2_PTK_UPDATE_EVENT:
ppsc->wakeup_reason = WOL_REASON_PTK_UPDATE;
-- 
2.5.0



[PATCH] nfc: nci: remove unnecessary null check

2017-06-13 Thread Gustavo A. R. Silva
Remove unnecessary NULL check for pointer conn_info.
conn_info is set in list_for_each_entry() using container_of(),
which is never NULL.

Addresses-Coverity-ID: 1362349
Cc: Guenter Roeck 
Signed-off-by: Gustavo A. R. Silva 
---
 net/nfc/nci/core.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 61fff42..c15cb88 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -73,11 +73,10 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev 
*ndev, u8 dest_type,
if (conn_info->dest_type == dest_type) {
if (!params)
return conn_info->conn_id;
-   if (conn_info) {
-   if (params->id == conn_info->dest_params->id &&
-   params->protocol == 
conn_info->dest_params->protocol)
-   return conn_info->conn_id;
-   }
+
+   if (params->id == conn_info->dest_params->id &&
+   params->protocol == 
conn_info->dest_params->protocol)
+   return conn_info->conn_id;
}
}
 
-- 
2.5.0



[PATCH] nfc: nci: fix potential NULL pointer dereference

2017-06-12 Thread Gustavo A. R. Silva
NULL check at line 76: if (conn_info) {, implies that pointer conn_info
might be NULL, but this pointer is being previously dereferenced,
which might cause a NULL pointer dereference.

Add NULL check before dereferencing pointer conn_info in order to
avoid a potential NULL pointer dereference.

Addresses-Coverity-ID: 1362349
Signed-off-by: Gustavo A. R. Silva 
---
 net/nfc/nci/core.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 61fff42..d2198ce 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -70,14 +70,13 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev 
*ndev, u8 dest_type,
struct nci_conn_info *conn_info;
 
list_for_each_entry(conn_info, &ndev->conn_info_list, list) {
-   if (conn_info->dest_type == dest_type) {
+   if (conn_info && conn_info->dest_type == dest_type) {
if (!params)
return conn_info->conn_id;
-   if (conn_info) {
-   if (params->id == conn_info->dest_params->id &&
-   params->protocol == 
conn_info->dest_params->protocol)
-   return conn_info->conn_id;
-   }
+
+   if (params->id == conn_info->dest_params->id &&
+   params->protocol == 
conn_info->dest_params->protocol)
+   return conn_info->conn_id;
}
}
 
-- 
2.5.0



  1   2   >