New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 762439 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Check brcmfmac to see whether bcmdhd vulnerabilities are present

Project Member Reported by mnissler@chromium.org, Sep 6 2017

Issue description

We got notice of several bugs in Broadcoms bcmdhd kernel driver for their WiFi parts. Chrome OS is using the brcmfmac driver, so is not directly affected. As far as I know these drivers are close cousins though, so the vulnerabilities might be present in brcmfmac as well.

See https://source.android.com/security/bulletin/2017-09-01#broadcom-components for a list of CVE IDs.

Assigning to snanda@ to find an owner on the kernel team.

Tentatively setting Severity-High and Impact-Stable for now, to be revised once we have an idea of how we fare.

 
Cc: terry-ht...@broadcom.com
Here is some input from Broadcom on the subset of relevant CVEs:

V2017053103 / CVE-2017-0788: Heap overflow in "dhd_handle_hotlist_scan_ev
V2017053102 / CVE-2017-0789: Security Vulnerability - Broadcom: Heap overflow in "dhd_handle_swc_evt", invalid patch for cve-2017-056
V2017053101 / CVE-2017-0790: Security Vulnerability - Broadcom: Heap overflow in "dhd_process_full_gscan_result"  => There is no Gscan/hotlist_scan and SWC function in brcmfmac.


V2017060101 / CVE-2017-0786: Security Vulnerability - Broadcom: Potential heap overflow in "wl_escan_handler "      => Apply in brcmf_cfg80211_escan_handler instead of wl_escan_handler.
Project Member

Comment 2 by sheriffbot@chromium.org, Sep 6 2017

Labels: M-61
Project Member

Comment 3 by sheriffbot@chromium.org, Sep 6 2017

Status: Assigned (was: Available)
Cc: snanda@chromium.org
Owner: cernekee@chromium.org
Cc: mnissler@chromium.org
Status: Started (was: Assigned)
So, if needed, I can go through each CVE listed in the Android security bulletin, look up the CVE in Buganizer, and try to manually determine whether it applies to brcmfmac (a driver I've looked at, but am by no means an expert on).  And then hack together a fix.

But according to MAINTAINERS, the upstream brcmfmac driver is actively supported by Broadcom engineers.  I see them posting as recently as yesterday on http://www.spinics.net/lists/linux-wireless/ .  I do not see phrases like "heap overflow" or "escan_handler" mentioned on that page.  So, like Mattias, I am also confused as to how these bugs will be addressed in brcmfmac.

Should we:

a) Wait until they release official fixes for these driver bugs and then merge them as FROMLIST/UPSTREAM

b) Coordinate privately with the maintainers to help them develop suitable fixes

c) Come up with our own local CHROMIUM: fixes and replace them with official fixes later

d) Something else?

I have reached out to the bcm80211-dev-list to ask about their plans and offer my assistance.
a) seems to be a waste of energy for everyone involved. We have a fix available, so we should at least give upstream a chance to use it and not forte them to develop their own fix. The same is true with c), since that also assumes that someone else repeats the work we have already done.

Maybe a variant of b/d: Submit our version (from Android) as a proposed fix upstream and go from there. The CVE is public, so I would think that should be ok. Worst case our patch won't be accepted, but even then the gesture of submitting it would improve our upstream reputation and, hopefully, make upstream more receptive for subsequent patches. I understand that it takes time to handle the back-and-forth, but I think it would be worth it.

> We have a fix available

I don't think we have brcmfmac fixes yet?  AIUI we only have bcmdhd fixes, which in some cases will be similar to brcmfmac but in other cases will not.

Some of the comments from Broadcom indicated that someone has already started assessing which CVEs are applicable to brcmfmac.  If the fixes are published somewhere, we can pick them up as-is.  Otherwise we may want to coordinate with whoever is in the process of handling them.
#7: You are right, though often the patches we have apply after some rework. Of course, if Broadcom is already working on it, we might as well wait and/or coordinate.


Terry, can you comment on whether Broadcom is planning on submitting fixes to upstream for the bugs in question?
Hi,

I am asking if there is any update that will reply to you right away
I looked up the ticket + summary for each CVE:

CVE-2017-7065 - b/62575138 - missing length checks gtk processing
 - This is a bug inside the wifi firmware, not the driver.  No action required.

CVE-2017-0786 - b/37336586, b/37351060 - heap overflow in wl_escan_handler()
 - Broadcom says: "Apply in brcmf_cfg80211_escan_handler instead of wl_escan_handler."  See below.

CVE-2017-0787 - b/37334268, b/37722970 - potential OOB write when handling event WLC_E_PFN_NET_FOUND

CVE-2017-0788 - b/37335516, b/37722328 - Heap overflow in "dhd_handle_hotlist_scan_evt

CVE-2017-0789 - b/37334598, b/37685267 - Heap overflow in "dhd_handle_swc_evt", invalid patch for cve-2017-0569

CVE-2017-0790 - b/37335524, b/37357704 - Heap overflow in "dhd_process_full_gscan_result"
 - Broadcom says: "There is no Gscan/hotlist_scan and SWC function in brcmfmac"

CVE-2017-0791 - b/37325346, b/37306719 - potential remote-dos when handling event WLC_E_ACTION_FRAME_RX
 - I see this constant in brcmfmac, didn't look at the code.

CVE-2017-0792 - b/37325330, b/37305578 - potential remote-dos when handling event WLC_E_PROXD 
 - I only see this constant in bcmdhd, not brcmfmac.


In a separate thread, Franky from Broadcom told me:

For issues mentioned in the bulletin:
CVE-2017-7065 is firmware issue.
CVE-2017-0786 has a fix in the pipeline and will be published soon.
CVE-2017-0787 CVE-2017-0791 have been fixed.
4835f37e3baf ("brcmfmac: add length checks in scheduled scan result handler")
0aedbcaf6f18 "(brcmfmac: Add length checks on firmware events")
Others are not applicable to brcmfmac.

Vulnerabilities published in older bulletins are addressed if applicable.

We did not mention CVE number in the commit message. Arend started a thread on the mail list about how to do this properly.

-- 8< --

I backported 0aedbcaf6f18 and 4835f37e3baf to our wireless-3.8 tree in Linux 3.14:

https://chromium-review.googlesource.com/656260
https://chromium-review.googlesource.com/656261
Thanks Kevin. Took a quick look at the reviews - 0aedbcaf6f18 seems to have issues that we may need to report back to upstream.
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 11 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7b21f04726ba3c9a528cb3f14c3ff76669ecae36

commit 7b21f04726ba3c9a528cb3f14c3ff76669ecae36
Author: Hante Meuleman <meuleman@broadcom.com>
Date: Mon Sep 11 18:14:25 2017

BACKPORT: brcmfmac: Add length checks on firmware events

Add additional length checks on firmware events to create more
robust code.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Lei Zhang <leizh@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
(cherry picked from commit 0aedbcaf6f182690790d98d90d5fe1e64c846c34)
[cernekee: Hand-merged patch.  Omitting wowl net detect fix, because
 that feature isn't in our old version of the driver.  packet_len is
 unused in the upstream code, too.]

BUG= chromium:762439 
TEST=boot up minnie and ensure that wifi still works
TEST=add an #error into one of the files to make sure we're updating the
     correct driver

Change-Id: I865bf791c45a7fb054c4a8d79e37d263ba55cf9f
Reviewed-on: https://chromium-review.googlesource.com/656260
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/7b21f04726ba3c9a528cb3f14c3ff76669ecae36/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c
[modify] https://crrev.com/7b21f04726ba3c9a528cb3f14c3ff76669ecae36/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.h
[modify] https://crrev.com/7b21f04726ba3c9a528cb3f14c3ff76669ecae36/drivers/net/wireless-3.8/brcm80211/brcmfmac/p2p.c
[modify] https://crrev.com/7b21f04726ba3c9a528cb3f14c3ff76669ecae36/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/82ce86243bc40ed0a3db824798a44f92008f14f2

commit 82ce86243bc40ed0a3db824798a44f92008f14f2
Author: Arend Van Spriel <arend.vanspriel@broadcom.com>
Date: Tue Sep 12 00:43:50 2017

BACKPORT: brcmfmac: add length checks in scheduled scan result handler

Assure the event data buffer is long enough to hold the array
of netinfo items and that SSID length does not exceed the maximum
of 32 characters as per 802.11 spec.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
(cherry picked from commit 4835f37e3bafc138f8bfa3cbed2920dd56fed283)
[cernekee: Adapted the upstream changes to our version of the driver
 by hand.]

BUG= chromium:762439 
TEST=boot up minnie and ensure that wifi still works

Change-Id: I7018878100007b19e22fb895faa04202fb237795
Reviewed-on: https://chromium-review.googlesource.com/656261
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/82ce86243bc40ed0a3db824798a44f92008f14f2/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/84892ea946c99efbaf50ea602d4f692ffc6d9b6f

commit 84892ea946c99efbaf50ea602d4f692ffc6d9b6f
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Sep 12 00:43:51 2017

FROMLIST: brcmfmac: Avoid possible out-of-bounds read

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 <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: http://www.spinics.net/lists/linux-wireless/msg165984.html

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: I5868070015f1f48b23b71f857f1e226cfa12cb03
Reviewed-on: https://chromium-review.googlesource.com/659118
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/84892ea946c99efbaf50ea602d4f692ffc6d9b6f/drivers/net/wireless-3.8/brcm80211/brcmfmac/p2p.c

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3c1cb7a7c3333f07f0702a6bf22fc1fa9a6c36bc

commit 3c1cb7a7c3333f07f0702a6bf22fc1fa9a6c36bc
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Sep 12 00:43:52 2017

FROMLIST: brcmfmac: Delete redundant length check

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 <arend.vanspriel@broadcom.com>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Link: http://www.spinics.net/lists/linux-wireless/msg165985.html

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Ib13bb59be4fb57e10835bfb1fc672e9fce8ee5be
Reviewed-on: https://chromium-review.googlesource.com/659119
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/3c1cb7a7c3333f07f0702a6bf22fc1fa9a6c36bc/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c

Project Member

Comment 17 by sheriffbot@chromium.org, Sep 12 2017

Status: Fixed (was: Started)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Merge-Request-62 Merge-Request-61
Project Member

Comment 19 by sheriffbot@chromium.org, Sep 12 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 20 by sheriffbot@chromium.org, Sep 13 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Merge-Review-61 -Merge-Request-62 Merge-Approved-62 Merge-Approved-61
Approving merge to M61 and M62.
Project Member

Comment 22 by sheriffbot@chromium.org, Sep 18 2017

Cc: keta...@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/aecdf7f543b3c7fa5063d51ced00b2545f2dfb4d

commit aecdf7f543b3c7fa5063d51ced00b2545f2dfb4d
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Thu Sep 21 18:54:24 2017

FROMLIST: brcmfmac: Add check for short event packets

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 <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patchwork.kernel.org/patch/9954607/

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Ibde9a85c1849067ce536d6661c2c451b8579633f
Reviewed-on: https://chromium-review.googlesource.com/659120
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>

[modify] https://crrev.com/aecdf7f543b3c7fa5063d51ced00b2545f2dfb4d/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8b761b19f56c1796c55cbcf9863c98a301d2d6cd

commit 8b761b19f56c1796c55cbcf9863c98a301d2d6cd
Author: Arend Van Spriel <arend.vanspriel@broadcom.com>
Date: Thu Sep 21 18:54:25 2017

FROMLIST: brcmfmac: add length check in brcmf_cfg80211_escan_handler()

Upon handling the firmware notification for scans the length was
checked properly and may result in corrupting kernel heap memory
due to buffer overruns. This fix addresses CVE-2017-0786.

Cc: stable@vger.kernel.org # v4.0.x
Cc: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patchwork.kernel.org/patch/9948689/

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Idcf362e57f26faa3d7846c5d82f723aef9640d53
Reviewed-on: https://chromium-review.googlesource.com/670162
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/8b761b19f56c1796c55cbcf9863c98a301d2d6cd/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 25 by sheriffbot@chromium.org, Sep 22 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 26 by bugdroid1@chromium.org, Sep 23 2017

Labels: merge-merged-release-R61-9765.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/660fab0d7c09bde202f081692819d2b14d84e0ef

commit 660fab0d7c09bde202f081692819d2b14d84e0ef
Author: Hante Meuleman <meuleman@broadcom.com>
Date: Sat Sep 23 03:04:00 2017

BACKPORT: brcmfmac: Add length checks on firmware events

Add additional length checks on firmware events to create more
robust code.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Lei Zhang <leizh@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
(cherry picked from commit 0aedbcaf6f182690790d98d90d5fe1e64c846c34)
[cernekee: Hand-merged patch.  Omitting wowl net detect fix, because
 that feature isn't in our old version of the driver.  packet_len is
 unused in the upstream code, too.]

BUG= chromium:762439 
TEST=boot up minnie and ensure that wifi still works
TEST=add an #error into one of the files to make sure we're updating the
     correct driver

Change-Id: I865bf791c45a7fb054c4a8d79e37d263ba55cf9f
Reviewed-on: https://chromium-review.googlesource.com/656260
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 7b21f04726ba3c9a528cb3f14c3ff76669ecae36)
Reviewed-on: https://chromium-review.googlesource.com/679876
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
Commit-Queue: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/660fab0d7c09bde202f081692819d2b14d84e0ef/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c
[modify] https://crrev.com/660fab0d7c09bde202f081692819d2b14d84e0ef/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.h
[modify] https://crrev.com/660fab0d7c09bde202f081692819d2b14d84e0ef/drivers/net/wireless-3.8/brcm80211/brcmfmac/p2p.c
[modify] https://crrev.com/660fab0d7c09bde202f081692819d2b14d84e0ef/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 23 2017

Labels: merge-merged-release-R62-9901.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/9b6eaf28a6c1913c7b1d29bf41cd2218111047ca

commit 9b6eaf28a6c1913c7b1d29bf41cd2218111047ca
Author: Hante Meuleman <meuleman@broadcom.com>
Date: Sat Sep 23 03:04:12 2017

BACKPORT: brcmfmac: Add length checks on firmware events

Add additional length checks on firmware events to create more
robust code.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Lei Zhang <leizh@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
(cherry picked from commit 0aedbcaf6f182690790d98d90d5fe1e64c846c34)
[cernekee: Hand-merged patch.  Omitting wowl net detect fix, because
 that feature isn't in our old version of the driver.  packet_len is
 unused in the upstream code, too.]

BUG= chromium:762439 
TEST=boot up minnie and ensure that wifi still works
TEST=add an #error into one of the files to make sure we're updating the
     correct driver

Change-Id: I865bf791c45a7fb054c4a8d79e37d263ba55cf9f
Reviewed-on: https://chromium-review.googlesource.com/656260
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 7b21f04726ba3c9a528cb3f14c3ff76669ecae36)
Reviewed-on: https://chromium-review.googlesource.com/679879
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
Trybot-Ready: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/9b6eaf28a6c1913c7b1d29bf41cd2218111047ca/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c
[modify] https://crrev.com/9b6eaf28a6c1913c7b1d29bf41cd2218111047ca/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.h
[modify] https://crrev.com/9b6eaf28a6c1913c7b1d29bf41cd2218111047ca/drivers/net/wireless-3.8/brcm80211/brcmfmac/p2p.c
[modify] https://crrev.com/9b6eaf28a6c1913c7b1d29bf41cd2218111047ca/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 28 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/91215290e968ae5ddf6bccc2302848a83100a8fb

commit 91215290e968ae5ddf6bccc2302848a83100a8fb
Author: Arend Van Spriel <arend.vanspriel@broadcom.com>
Date: Sat Sep 23 03:04:39 2017

BACKPORT: brcmfmac: add length checks in scheduled scan result handler

Assure the event data buffer is long enough to hold the array
of netinfo items and that SSID length does not exceed the maximum
of 32 characters as per 802.11 spec.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
(cherry picked from commit 4835f37e3bafc138f8bfa3cbed2920dd56fed283)
[cernekee: Adapted the upstream changes to our version of the driver
 by hand.]

BUG= chromium:762439 
TEST=boot up minnie and ensure that wifi still works

Change-Id: I7018878100007b19e22fb895faa04202fb237795
Reviewed-on: https://chromium-review.googlesource.com/656261
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 82ce86243bc40ed0a3db824798a44f92008f14f2)
Reviewed-on: https://chromium-review.googlesource.com/679882
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/91215290e968ae5ddf6bccc2302848a83100a8fb/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 29 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/65d776cf454096ab11dba656556e80b7458df08f

commit 65d776cf454096ab11dba656556e80b7458df08f
Author: Arend Van Spriel <arend.vanspriel@broadcom.com>
Date: Sat Sep 23 03:04:41 2017

BACKPORT: brcmfmac: add length checks in scheduled scan result handler

Assure the event data buffer is long enough to hold the array
of netinfo items and that SSID length does not exceed the maximum
of 32 characters as per 802.11 spec.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
(cherry picked from commit 4835f37e3bafc138f8bfa3cbed2920dd56fed283)
[cernekee: Adapted the upstream changes to our version of the driver
 by hand.]

BUG= chromium:762439 
TEST=boot up minnie and ensure that wifi still works

Change-Id: I7018878100007b19e22fb895faa04202fb237795
Reviewed-on: https://chromium-review.googlesource.com/656261
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 82ce86243bc40ed0a3db824798a44f92008f14f2)
Reviewed-on: https://chromium-review.googlesource.com/679878
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
Trybot-Ready: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/65d776cf454096ab11dba656556e80b7458df08f/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 30 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e9d9805f3596b5bddac7b376d3387a256b3c9837

commit e9d9805f3596b5bddac7b376d3387a256b3c9837
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Sep 23 03:04:58 2017

FROMLIST: brcmfmac: Avoid possible out-of-bounds read

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 <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: http://www.spinics.net/lists/linux-wireless/msg165984.html

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: I5868070015f1f48b23b71f857f1e226cfa12cb03
Reviewed-on: https://chromium-review.googlesource.com/659118
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 84892ea946c99efbaf50ea602d4f692ffc6d9b6f)
Reviewed-on: https://chromium-review.googlesource.com/679881
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/e9d9805f3596b5bddac7b376d3387a256b3c9837/drivers/net/wireless-3.8/brcm80211/brcmfmac/p2p.c

Project Member

Comment 31 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/59194002bde301682bc6371f862c85e53c3251e2

commit 59194002bde301682bc6371f862c85e53c3251e2
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Sep 23 03:05:01 2017

FROMLIST: brcmfmac: Avoid possible out-of-bounds read

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 <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: http://www.spinics.net/lists/linux-wireless/msg165984.html

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: I5868070015f1f48b23b71f857f1e226cfa12cb03
Reviewed-on: https://chromium-review.googlesource.com/659118
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 84892ea946c99efbaf50ea602d4f692ffc6d9b6f)
Reviewed-on: https://chromium-review.googlesource.com/679884
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/59194002bde301682bc6371f862c85e53c3251e2/drivers/net/wireless-3.8/brcm80211/brcmfmac/p2p.c

Project Member

Comment 32 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/be762c259c5cbef24909f881490cf1bfd0beaa1f

commit be762c259c5cbef24909f881490cf1bfd0beaa1f
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Sep 23 03:06:22 2017

FROMLIST: brcmfmac: Delete redundant length check

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 <arend.vanspriel@broadcom.com>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Link: http://www.spinics.net/lists/linux-wireless/msg165985.html

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Ib13bb59be4fb57e10835bfb1fc672e9fce8ee5be
Reviewed-on: https://chromium-review.googlesource.com/659119
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 3c1cb7a7c3333f07f0702a6bf22fc1fa9a6c36bc)
Reviewed-on: https://chromium-review.googlesource.com/679888
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/be762c259c5cbef24909f881490cf1bfd0beaa1f/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c

Project Member

Comment 33 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/fdd24f69ce8d49d64d54a4e973e7b17b94e272cf

commit fdd24f69ce8d49d64d54a4e973e7b17b94e272cf
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Sep 23 03:06:24 2017

FROMLIST: brcmfmac: Delete redundant length check

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 <arend.vanspriel@broadcom.com>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Link: http://www.spinics.net/lists/linux-wireless/msg165985.html

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Ib13bb59be4fb57e10835bfb1fc672e9fce8ee5be
Reviewed-on: https://chromium-review.googlesource.com/659119
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit 3c1cb7a7c3333f07f0702a6bf22fc1fa9a6c36bc)
Reviewed-on: https://chromium-review.googlesource.com/679889
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/fdd24f69ce8d49d64d54a4e973e7b17b94e272cf/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c

Labels: -Merge-Approved-61 -Merge-Approved-62
Verified changes on canary, then merged back to M61 + M62.
Project Member

Comment 36 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/04d69a14907e0999cc4fce008cda6f6504dcaac9

commit 04d69a14907e0999cc4fce008cda6f6504dcaac9
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Sep 23 03:06:53 2017

FROMLIST: brcmfmac: Add check for short event packets

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 <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patchwork.kernel.org/patch/9954607/

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Ibde9a85c1849067ce536d6661c2c451b8579633f
Reviewed-on: https://chromium-review.googlesource.com/659120
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>
(cherry picked from commit aecdf7f543b3c7fa5063d51ced00b2545f2dfb4d)
Reviewed-on: https://chromium-review.googlesource.com/679890
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/04d69a14907e0999cc4fce008cda6f6504dcaac9/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c

Project Member

Comment 37 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2dc856d9cd90f6a47d83c8314829614b77626361

commit 2dc856d9cd90f6a47d83c8314829614b77626361
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Sep 23 03:06:54 2017

FROMLIST: brcmfmac: Add check for short event packets

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 <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patchwork.kernel.org/patch/9954607/

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Ibde9a85c1849067ce536d6661c2c451b8579633f
Reviewed-on: https://chromium-review.googlesource.com/659120
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>
(cherry picked from commit aecdf7f543b3c7fa5063d51ced00b2545f2dfb4d)
Reviewed-on: https://chromium-review.googlesource.com/679891
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/2dc856d9cd90f6a47d83c8314829614b77626361/drivers/net/wireless-3.8/brcm80211/brcmfmac/fweh.c

Project Member

Comment 38 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e9a72752d0d5cfec0399ff005fd0f6dceefd9858

commit e9a72752d0d5cfec0399ff005fd0f6dceefd9858
Author: Arend Van Spriel <arend.vanspriel@broadcom.com>
Date: Sat Sep 23 03:07:14 2017

FROMLIST: brcmfmac: add length check in brcmf_cfg80211_escan_handler()

Upon handling the firmware notification for scans the length was
checked properly and may result in corrupting kernel heap memory
due to buffer overruns. This fix addresses CVE-2017-0786.

Cc: stable@vger.kernel.org # v4.0.x
Cc: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patchwork.kernel.org/patch/9948689/

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Idcf362e57f26faa3d7846c5d82f723aef9640d53
Reviewed-on: https://chromium-review.googlesource.com/670162
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
(cherry picked from commit 8b761b19f56c1796c55cbcf9863c98a301d2d6cd)
Reviewed-on: https://chromium-review.googlesource.com/679885

[modify] https://crrev.com/e9a72752d0d5cfec0399ff005fd0f6dceefd9858/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 39 by bugdroid1@chromium.org, Sep 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/cd18617268dacb1943d847aa31912ba323673c12

commit cd18617268dacb1943d847aa31912ba323673c12
Author: Arend Van Spriel <arend.vanspriel@broadcom.com>
Date: Sat Sep 23 03:07:15 2017

FROMLIST: brcmfmac: add length check in brcmf_cfg80211_escan_handler()

Upon handling the firmware notification for scans the length was
checked properly and may result in corrupting kernel heap memory
due to buffer overruns. This fix addresses CVE-2017-0786.

Cc: stable@vger.kernel.org # v4.0.x
Cc: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patchwork.kernel.org/patch/9948689/

BUG= chromium:762439 
TEST=test wireless on minnie

Change-Id: Idcf362e57f26faa3d7846c5d82f723aef9640d53
Reviewed-on: https://chromium-review.googlesource.com/670162
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
(cherry picked from commit 8b761b19f56c1796c55cbcf9863c98a301d2d6cd)
Reviewed-on: https://chromium-review.googlesource.com/679887

[modify] https://crrev.com/cd18617268dacb1943d847aa31912ba323673c12/drivers/net/wireless-3.8/brcm80211/brcmfmac/wl_cfg80211.c

Project Member

Comment 40 by sheriffbot@chromium.org, Dec 19 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 41 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 42 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Project Member

Comment 43 by sheriffbot@chromium.org, Mar 27 2018

Labels: -M-61 M-65

Sign in to add a comment