[PATCH V3 1/3] brcmfmac: Avoid possible out-of-bounds read

[Kevin Cernekee]

In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
the length of rxframe is validated.  This could lead to uninitialized
data being accessed (but not printed).  Since we already have a
perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
and ch.chspec is not modified by decchspec(), avoid the extra
assignment and use ch.chspec in the debug print.

Suggested-by: Mattias Nissler 
Signed-off-by: Kevin Cernekee 
Reviewed-by: Arend van Spriel 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


V2->V3: No change


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 2ce675ab40ef..1c450c0727cb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -1853,7 +1853,6 @@ s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if 
*ifp,
struct afx_hdl *afx_hdl = &p2p->afx_hdl;
struct brcmf_cfg80211_vif *vif = ifp->vif;
struct brcmf_rx_mgmt_data *rxframe = (struct brcmf_rx_mgmt_data *)data;
-   u16 chanspec = be16_to_cpu(rxframe->chanspec);
struct brcmu_chan ch;
u8 *mgmt_frame;
u32 mgmt_frame_len;
@@ -1906,7 +1905,7 @@ s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if 
*ifp,
cfg80211_rx_mgmt(&vif->wdev, freq, 0, mgmt_frame, mgmt_frame_len, 0);
 
brcmf_dbg(INFO, "mgmt_frame_len (%d) , e->datalen (%d), chanspec 
(%04x), freq (%d)\n",
- mgmt_frame_len, e->datalen, chanspec, freq);
+ mgmt_frame_len, e->datalen, ch.chspec, freq);
 
return 0;
 }
-- 
2.14.1.690.gbb1197296e-goog

[PATCH V3 3/3] brcmfmac: Add check for short event packets

[Kevin Cernekee]

The length of the data in the received skb is currently passed into
brcmf_fweh_process_event() as packet_len, but this value is not checked.
event_packet should be followed by DATALEN bytes of additional event
data.  Ensure that the received packet actually contains at least
DATALEN bytes of additional data, to avoid copying uninitialized memory
into event->data.

Suggested-by: Mattias Nissler 
Signed-off-by: Kevin Cernekee 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


V2->V3: Change '<' to '>' and retest


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 27e661fa356f..e7eaa57d11d9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -424,7 +424,8 @@ void brcmf_fweh_process_event(struct brcmf_pub *drvr,
if (code != BRCMF_E_IF && !fweh->evt_handler[code])
return;
 
-   if (datalen > BRCMF_DCMD_MAXLEN)
+   if (datalen > BRCMF_DCMD_MAXLEN ||
+   datalen + sizeof(*event_packet) > packet_len)
return;
 
if (in_interrupt())
-- 
2.14.1.690.gbb1197296e-goog

[PATCH V3 2/3] brcmfmac: Delete redundant length check

[Kevin Cernekee]

brcmf_fweh_process_event() sets event->datalen to the
endian-swapped value of event_packet->msg.datalen, which is the
same as emsg.datalen.  This length is already validated in
brcmf_fweh_process_event(), so there is no need to check it
again upon dequeuing the event.

Suggested-by: Arend van Spriel 
Signed-off-by: Kevin Cernekee 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 5 -
 1 file changed, 5 deletions(-)


V2->V3: No change


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 4eb1e1ce9ace..27e661fa356f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -257,11 +257,6 @@ static void brcmf_fweh_event_worker(struct work_struct 
*work)
brcmf_dbg_hex_dump(BRCMF_EVENT_ON(), event->data,
   min_t(u32, emsg.datalen, 64),
   "event payload, len=%d\n", emsg.datalen);
-   if (emsg.datalen > event->datalen) {
-   brcmf_err("event invalid length header=%d, msg=%d\n",
- event->datalen, emsg.datalen);
-   goto event_free;
-   }
 
/* special handling of interface event */
if (event->code == BRCMF_E_IF) {
-- 
2.14.1.690.gbb1197296e-goog

Re: iwlwifi firmware load broken in current -git

[Jens Axboe]

On 09/15/2017 09:03 PM, Bjorn Helgaas wrote:
> On Fri, Sep 15, 2017 at 01:55:57PM -0600, Jens Axboe wrote:
>> On 09/15/2017 01:51 PM, Luca Coelho wrote:
>>> On Fri, 2017-09-15 at 13:48 -0600, Jens Axboe wrote:
 On 09/15/2017 01:38 PM, Linus Torvalds wrote:
> On Fri, Sep 15, 2017 at 12:32 PM, Jens Axboe  wrote:
>>>
>>> In any case, your patch introduces a regression on systems. Please get
>>> it reverted now, and then you can come up with a new approach to fix the
>>> double enable of the upstream bridge.
>>
>> Who's sending in the revert? I can certainly do it if no one else does,
>> but it needs to be done.
>>
>> I'm not seeing any patches coming out of Srinath to fix up the
>> situation, so we should revert the broken patch until a better solution
>> exists.
>
> Hmm. I don't have the history here (apparently it never made lkml, for
> example), so I don't even know which commit you're talking about.
>
> From some of the context it looks like commit 40f11adc7cd9 ("PCI:
> Avoid race while enabling upstream bridges"), is that correct?

 Yes, Luca says that Bjorn already sent in the revert request, I just
 didn't see it since I wasn't CC'ed on it. So looks like we're all
 good, provided that makes it into -rc1. 40f11adc7cd9 is the broken
 commit.
>>>
>>> Strange... AFAICT you *were* CCed on it.  And so was everyone else in
>>> the original thread (+LKML)...
>>
>> Hmm, never showed up here. Very odd!
> 
> Sorry, I think this is probably because I'm an idiot and sent it from
> an @google.com account and it got rejected because the DMARC check
> failed.

Ah, good to know why it didn't show up. Thanks.

-- 
Jens Axboe

Re: RTL8192EE PCIe Wireless Network Adapter crashed with linux-4.13

[Larry Finger]

On 09/16/2017 06:27 AM, Zwindl wrote:
Hi, I've done the test, and the weird thing happened. The kernel buit with this 
config file https://ptpb.pw/HF1g which is from 
https://aur.archlinux.org/packages/linux-git/  can run properly, the wifi can 
connect, despite which version it is, but, with this config file 
https://ptpb.pw/7GuV which comes from the archlinux's official package build 
repo(linux-package 
), 
all the version begin with 4.13 was failed to connect wifi.
So, I think the issue is not caused by the kernel code, is caused by some 
options in the config file, but I can't fully understand the meaning of these 
options so that I can't determine which option caused that issue, what should I 
do now, maybe report this bug to archlinux's maintainer?
By the way, maybe I'll lost internet connection tomorrow, it's time to back to 
university, but I'm happy to help to push the debug progress.


Yes, you need to report this to archlinux's bugzilla or maintainer, whichever is 
appropriate. I have seen a configuration error cause some feature to be silently 
missing, but leading to a WARN is rare.


I looked at your two configurations, but did not see a definitive difference.

Larry

[PATCH] mwifiex: make const arrays static to shink object code size

[Colin King]

From: Colin Ian King 

Don't populate const arrays on the stack, instead make them static
Makes the object code smaller by nearly 300 bytes:

Before:
   textdata bss dec hex filename
  69260   16149 576   85985   14fe1 cfg80211.o

After:
   textdata bss dec hex filename
  68385   16725 576   85686   14eb6 cfg80211.o

Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 32c5074da84c..c45b86fcfd8d 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -142,7 +142,7 @@ mwifiex_cfg80211_del_key(struct wiphy *wiphy, struct 
net_device *netdev,
 u8 key_index, bool pairwise, const u8 *mac_addr)
 {
struct mwifiex_private *priv = mwifiex_netdev_get_priv(netdev);
-   const u8 bc_mac[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+   static const u8 bc_mac[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
const u8 *peer_mac = pairwise ? mac_addr : bc_mac;
 
if (mwifiex_set_encode(priv, NULL, NULL, 0, key_index, peer_mac, 1)) {
@@ -454,7 +454,7 @@ mwifiex_cfg80211_add_key(struct wiphy *wiphy, struct 
net_device *netdev,
 {
struct mwifiex_private *priv = mwifiex_netdev_get_priv(netdev);
struct mwifiex_wep_key *wep_key;
-   const u8 bc_mac[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+   static const u8 bc_mac[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
const u8 *peer_mac = pairwise ? mac_addr : bc_mac;
 
if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP &&
@@ -3250,8 +3250,8 @@ static int mwifiex_set_wowlan_mef_entry(struct 
mwifiex_private *priv,
int i, filt_num = 0, ret = 0;
bool first_pat = true;
u8 byte_seq[MWIFIEX_MEF_MAX_BYTESEQ + 1];
-   const u8 ipv4_mc_mac[] = {0x33, 0x33};
-   const u8 ipv6_mc_mac[] = {0x01, 0x00, 0x5e};
+   static const u8 ipv4_mc_mac[] = {0x33, 0x33};
+   static const u8 ipv6_mc_mac[] = {0x01, 0x00, 0x5e};
 
mef_entry->mode = MEF_MODE_HOST_SLEEP;
mef_entry->action = MEF_ACTION_ALLOW_AND_WAKEUP_HOST;
@@ -3544,9 +3544,9 @@ static int mwifiex_set_rekey_data(struct wiphy *wiphy, 
struct net_device *dev,
 
 static int mwifiex_get_coalesce_pkt_type(u8 *byte_seq)
 {
-   const u8 ipv4_mc_mac[] = {0x33, 0x33};
-   const u8 ipv6_mc_mac[] = {0x01, 0x00, 0x5e};
-   const u8 bc_mac[] = {0xff, 0xff, 0xff, 0xff};
+   static const u8 ipv4_mc_mac[] = {0x33, 0x33};
+   static const u8 ipv6_mc_mac[] = {0x01, 0x00, 0x5e};
+   static const u8 bc_mac[] = {0xff, 0xff, 0xff, 0xff};
 
if ((byte_seq[0] & 0x01) &&
(byte_seq[MWIFIEX_COALESCE_MAX_BYTESEQ] == 1))
-- 
2.14.1