* Re: [RFC 3/4] mac80211_hwsim: explicitly set netlink parallel ops to false
From: Benjamin Beichler @ 2017-09-08 15:07 UTC (permalink / raw)
To: linux-wireless, johannes
In-Reply-To: <1504880360.20347.0.camel@sipsolutions.net>
Am 8=2E September 2017 16:19:20 MESZ schrieb Johannes Berg <johannes@sipso=
lutions=2Enet>:
>On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
>> The ops field is zero initialized, therefore parallel ops is already
>> false=2E
>
>Therefore this patch is completely pointless?
Sorry my first message was missing regarding this=2E My question is, wheth=
er this is intentionally, and if it is parallel, whether we need extensive =
locking here=2E
>
>johannes
--=20
M=2ESc=2E Benjamin Beichler
Universit=C3=A4t Rostock, Fakult=C3=A4t f=C3=BCr Informatik und Elektrotec=
hnik
Institut f=C3=BCr Angewandte Mikroelektronik und Datentechnik
University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE
Richard-Wagner-Stra=C3=9Fe 31
18119 Rostock
Deutschland/Germany
phone: +49 (0) 381 498 - 7278
email: Benjamin=2EBeichler@uni-rostock=2Ede
www: http://www=2Eimd=2Euni-rostock=2Ede/
^ permalink raw reply
* Re: [RFC 3/4] mac80211_hwsim: explicitly set netlink parallel ops to false
From: Johannes Berg @ 2017-09-08 15:11 UTC (permalink / raw)
To: Benjamin Beichler, linux-wireless
In-Reply-To: <5BB75376-46E0-45F4-8886-C90CEA37CB2B@uni-rostock.de>
On Fri, 2017-09-08 at 17:07 +0200, Benjamin Beichler wrote:
>
> Am 8. September 2017 16:19:20 MESZ schrieb Johannes Berg <johannes@si
> psolutions.net>:
> > On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
> > > The ops field is zero initialized, therefore parallel ops is
> > > already
> > > false.
> >
> > Therefore this patch is completely pointless?
>
> Sorry my first message was missing regarding this. My question is,
> whether this is intentionally, and if it is parallel, whether we need
> extensive locking here.
It's basically intentional - not sure parallel_ops even existed when
this was first written, but we can probably use parallel_ops if we want
to.
johannes
^ permalink raw reply
* [PATCH] rsi: fix a dereference on adapter before it has been null checked
From: Colin King @ 2017-09-08 15:24 UTC (permalink / raw)
To: Kalle Valo, Amitkumar Karwar, Prameela Rani Garnepudi,
Karun Eagalapati, linux-wireless, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The assignment of dev is dereferencing adapter before adapter has
been null checked, potentially leading to a null pointer dereference.
Fix this by simply moving the assignment of dev to a later point
after the sanity null check of adapter.
Detected by CoverityScan CID#1398383 ("Dereference before null check")
Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 81df09dd2636..08730227cd18 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -73,8 +73,7 @@ static int rsi_write_multiple(struct rsi_hw *adapter,
u8 *data,
u32 count)
{
- struct rsi_91x_usbdev *dev =
- (struct rsi_91x_usbdev *)adapter->rsi_dev;
+ struct rsi_91x_usbdev *dev;
if (!adapter)
return -ENODEV;
@@ -82,6 +81,7 @@ static int rsi_write_multiple(struct rsi_hw *adapter,
if (endpoint == 0)
return -EINVAL;
+ dev = (struct rsi_91x_usbdev *)adapter->rsi_dev;
if (dev->write_fail)
return -ENETDOWN;
--
2.14.1
^ permalink raw reply related
* pull-request: wireless-drivers 2017-09-08
From: Kalle Valo @ 2017-09-08 15:50 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
few fixes to net tree for 4.14. Note that this pull request contains the
iwlwifi fix Linus hopes to have by end of the merge window. Please let
me know if there are any problems.
Kalle
The following changes since commit 8e0deed92406d93ae0365cb8a6134db5721e7aca:
tipc: remove unnecessary call to dev_net() (2017-09-06 21:25:52 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2017-09-08
for you to fetch changes up to f957dd3c8db2781c8a334b166800788feb618625:
brcmfmac: feature check for multi-scheduled scan fails on bcm4345 devices (2017-09-08 12:25:24 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 4.14
Few fixes to regressions introduced in the last one or two releases.
The iwlwifi fix is for a regression reported by Linus.
rtlwifi
* fix two antenna selection related bugs
iwlwifi
* fix regression with older firmwares
brcmfmac
* workaround firmware crash for bcm4345
----------------------------------------------------------------
Ian W MORRISON (1):
brcmfmac: feature check for multi-scheduled scan fails on bcm4345 devices
Larry Finger (2):
rtlwifi: btcoexist: Fix breakage of ant_sel for rtl8723be
rtlwifi: btcoexist: Fix antenna selection code
Luca Coelho (1):
iwlwifi: mvm: only send LEDS_CMD when the FW supports it
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 3 ++-
drivers/net/wireless/intel/iwlwifi/fw/file.h | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/led.c | 3 ++-
.../realtek/rtlwifi/btcoexist/halbtc8723b2ant.c | 5 ++++-
.../realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 23 +++++++++++++++-------
5 files changed, 25 insertions(+), 10 deletions(-)
^ permalink raw reply
* [PATCH v2 2/2] wireless: return correct mandatory rates
From: Richard Schütz @ 2017-09-08 16:07 UTC (permalink / raw)
To: linux-wireless; +Cc: Richard Schütz
In-Reply-To: <20170907154744.28357-2-rschuetz@uni-koblenz.de>
Use IEEE80211_RATE_MANDATORY_G instead of IEEE80211_RATE_MANDATORY_B to get
all mandatory rates in 2.4 GHz band. It is safe to do so because ERP
mandatory rates are a superset of HR/DSSS mandatory rates. Also limit to
OFDM rates for 10 MHz and 5 MHz channels as originally intended by commit
74608aca4d82e.
Signed-off-by: Richard Schütz <rschuetz@uni-koblenz.de>
---
net/wireless/util.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index c69b5c31caf8..386070e0035a 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -51,16 +51,17 @@ u32 ieee80211_mandatory_rates(struct ieee80211_supported_band *sband,
if (sband->band == NL80211_BAND_2GHZ) {
if (scan_width == NL80211_BSS_CHAN_WIDTH_5 ||
scan_width == NL80211_BSS_CHAN_WIDTH_10)
- mandatory_flag = IEEE80211_RATE_MANDATORY_G;
+ mandatory_flag = IEEE80211_RATE_MANDATORY_G |
+ IEEE80211_RATE_ERP_G;
else
- mandatory_flag = IEEE80211_RATE_MANDATORY_B;
+ mandatory_flag = IEEE80211_RATE_MANDATORY_G;
} else {
mandatory_flag = IEEE80211_RATE_MANDATORY_A;
}
bitrates = sband->bitrates;
for (i = 0; i < sband->n_bitrates; i++)
- if (bitrates[i].flags & mandatory_flag)
+ if ((bitrates[i].flags & mandatory_flag) == mandatory_flag)
mandatory_rates |= BIT(i);
return mandatory_rates;
}
--
2.14.1
^ permalink raw reply related
* Re: pull-request: wireless-drivers 2017-09-08
From: David Miller @ 2017-09-08 17:10 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87o9qljil2.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Fri, 08 Sep 2017 18:50:01 +0300
> few fixes to net tree for 4.14. Note that this pull request contains the
> iwlwifi fix Linus hopes to have by end of the merge window. Please let
> me know if there are any problems.
Pulled, thanks for following up particularly with the iwlwifi fix.
^ permalink raw reply
* [PATCH 0/3] New brcmfmac bounds checks
From: Kevin Cernekee @ 2017-09-08 19:13 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
These were suggested by the Chrome OS security team[1]. Compile-tested
only.
[1] http://crosreview.com/656260
Kevin Cernekee (3):
brcmfmac: Avoid possible out-of-bounds read
brcmfmac: Don't print out-of-bounds event data
brcmfmac: Add check for short event packets
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 13 +++++++------
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
2 files changed, 8 insertions(+), 8 deletions(-)
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply
* [PATCH 1/3] brcmfmac: Avoid possible out-of-bounds read
From: Kevin Cernekee @ 2017-09-08 19:13 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170908191342.28053-1-cernekee@chromium.org>
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 printed in a debug message. 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>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH 2/3] brcmfmac: Don't print out-of-bounds event data
From: Kevin Cernekee @ 2017-09-08 19:13 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170908191342.28053-1-cernekee@chromium.org>
The debug print that dumps out newly-dequeued events uses emsg.datalen
before that field has been validated, which may lead to an out-of-bounds
read. Assume that any properly-formed event message has a valid length
field, and move the debug print below the length check.
Suggested-by: Mattias Nissler <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 4eb1e1ce9ace..5aabdc9ed7e0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -252,17 +252,17 @@ static void brcmf_fweh_event_worker(struct work_struct *work)
emsg.ifidx = emsg_be->ifidx;
emsg.bsscfgidx = emsg_be->bsscfgidx;
- brcmf_dbg(EVENT, " version %u flags %u status %u reason %u\n",
- emsg.version, emsg.flags, emsg.status, emsg.reason);
- 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;
}
+ brcmf_dbg(EVENT, " version %u flags %u status %u reason %u\n",
+ emsg.version, emsg.flags, emsg.status, emsg.reason);
+ brcmf_dbg_hex_dump(BRCMF_EVENT_ON(), event->data,
+ min_t(u32, emsg.datalen, 64),
+ "event payload, len=%d\n", emsg.datalen);
/* special handling of interface event */
if (event->code == BRCMF_E_IF) {
brcmf_fweh_handle_if_event(drvr, &emsg, event->data);
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH 3/3] brcmfmac: Add check for short event packets
From: Kevin Cernekee @ 2017-09-08 19:13 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170908191342.28053-1-cernekee@chromium.org>
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>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 5aabdc9ed7e0..4cad1f0d2a82 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -429,7 +429,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.581.gf28d330327-goog
^ permalink raw reply related
* Re: [PATCH] mwifiex: check for mfg_mode in add_virtual_intf
From: Brian Norris @ 2017-09-08 19:42 UTC (permalink / raw)
To: Ganapathi Bhat
Cc: linux-wireless, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao,
Mangesh Malusare
In-Reply-To: <1504816363-31015-1-git-send-email-gbhat@marvell.com>
Hi,
Could have used a 'v2' in the subject, but hopefully that doesn't bother
Kalle too much.
On Fri, Sep 08, 2017 at 02:02:43AM +0530, Ganapathi Bhat wrote:
> If driver is loaded with 'mfg_mode' enabled, then the sending
> commands are not allowed. So, skip sending commands, to firmware
> in mwifiex_add_virtual_intf if 'mfg_mode' is enabled.
>
> Fixes: 7311ea850079 ("mwifiex: fix AP start problem for newly added interface")
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> ---
> v2: addressed Brian's comments.
> ---
> drivers/net/wireless/marvell/mwifiex/cfg80211.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index 32c5074..ad1ebd8 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -2959,18 +2959,21 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
> }
>
> mwifiex_init_priv_params(priv, dev);
> - mwifiex_set_mac_address(priv, dev);
>
> priv->netdev = dev;
>
> - ret = mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE,
> - HostCmd_ACT_GEN_SET, 0, NULL, true);
> - if (ret)
> - goto err_set_bss_mode;
> + if (!adapter->mfg_mode) {
> + mwifiex_set_mac_address(priv, dev);
>
> - ret = mwifiex_sta_init_cmd(priv, false, false);
> - if (ret)
> - goto err_sta_init;
> + ret = mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE,
> + HostCmd_ACT_GEN_SET, 0, NULL, true);
> + if (ret)
> + goto err_set_bss_mode;
> +
> + ret = mwifiex_sta_init_cmd(priv, false, false);
> + if (ret)
> + goto err_sta_init;
> + }
Seems better to me.
Reviewed-by: Brian Norris <briannorris@chromium.org>
>
> mwifiex_setup_ht_caps(&wiphy->bands[NL80211_BAND_2GHZ]->ht_cap, priv);
> if (adapter->is_hw_11ac_capable)
> --
> 1.9.1
>
^ permalink raw reply
* Re: Incorrect mesh path seq num
From: Thomas Pedersen @ 2017-09-08 19:58 UTC (permalink / raw)
To: Johannes Berg; +Cc: Greg Maitz, linux-wireless
In-Reply-To: <1504531173.27801.0.camel@sipsolutions.net>
On Mon, Sep 4, 2017 at 6:19 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2017-09-01 at 13:07 -0700, Thomas Pedersen wrote:
>> On Thu, Aug 31, 2017 at 11:30 PM, Greg Maitz <ghh19622@gmail.com>
>> wrote:
>> > Hi guys,
>> >
>> > I'm seeing a problem when I work on the wireless mesh between two
>> > linux devices. The root node has 3.18 kernel while the next hop
>> > station runs 2.6.37 kernel. I found the mpath->sn value is
>> > incorrect
>> > most of the time on the device having 2.6.37 kernel. After
>> > examining
>> > the code, in function hwmp_route_info_get [mesh_hwmp.c], after
>> > mesh_path_lookup, the sequence number (i.e, mpath->sn) is
>> > incorrect.
>> > For instance, I see mpath->sn having value 0x30950000. It should be
>> > 0x9530, while the orig_sn is having value 0x9531.
>>
>> Looks like an endianess bug. Are you testing on two platforms of
>> different endianess?
>
> Even if that's the case, wouldn't it mean some kind of conversion is
> missing somewhere?
Yes. I looked for a missing conversion, but couldn't find it.
Greg, where / how are you printing mpath->sn? mpath dump or a printk you added?
--
thomas
^ permalink raw reply
* synchronization question about sdata->skb_queue
From: dztemplor . @ 2017-09-09 1:28 UTC (permalink / raw)
To: linux-wireless
Hi, everyone.
I'm a newbie in mac80211 and have a question about synchronization for
sdata->skb_queue.
sdata->skb_queue is shared between ieee80211_iface_work() and rx
handlers like ieee80211_rx_h_action()
ieee80211_iface_work() runs in process context.
ieee80211_rx_h_action() runs in softirq context.
why there is no spin_lock_bh() in ieee80211_iface_work() when it
dequeues from sdata->skb_queue?
I know the question may sounds stupid, will appreciate if anyone can
explain to me.
Regards,
Spike.
^ permalink raw reply
* Re: synchronization question about sdata->skb_queue
From: dztemplor . @ 2017-09-09 1:35 UTC (permalink / raw)
To: linux-wireless
In-Reply-To: <CABBOhL4UjgKHk_HBHr+rD_5CSqbXNX-52AQydKPBG1-KqeTOtw@mail.gmail.com>
sorry, just realized skb_dequeue() will call spin_lock_irqsave() inside.
please ignore this question.
On Sat, Sep 9, 2017 at 9:28 AM, dztemplor . <zdu.shanghai@gmail.com> wrote:
> Hi, everyone.
> I'm a newbie in mac80211 and have a question about synchronization for
> sdata->skb_queue.
> sdata->skb_queue is shared between ieee80211_iface_work() and rx
> handlers like ieee80211_rx_h_action()
> ieee80211_iface_work() runs in process context.
> ieee80211_rx_h_action() runs in softirq context.
> why there is no spin_lock_bh() in ieee80211_iface_work() when it
> dequeues from sdata->skb_queue?
> I know the question may sounds stupid, will appreciate if anyone can
> explain to me.
>
> Regards,
> Spike.
^ permalink raw reply
* Re: [PATCH 1/3] brcmfmac: Avoid possible out-of-bounds read
From: Arend van Spriel @ 2017-09-09 7:45 UTC (permalink / raw)
To: Kevin Cernekee, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170908191342.28053-2-cernekee@chromium.org>
On 08-09-17 21:13, Kevin Cernekee wrote:
> 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 printed in a debug message. Since we already have a
The debug message is after the length validation so there is not
unintialized data being printed.
> 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.
However, there is no real for the chanspec variable for the given reason
so...
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Suggested-by: Mattias Nissler <mnissler@chromium.org>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH 2/3] brcmfmac: Don't print out-of-bounds event data
From: Arend van Spriel @ 2017-09-09 8:12 UTC (permalink / raw)
To: Kevin Cernekee, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170908191342.28053-3-cernekee@chromium.org>
On 08-09-17 21:13, Kevin Cernekee wrote:
> The debug print that dumps out newly-dequeued events uses emsg.datalen
> before that field has been validated, which may lead to an out-of-bounds
> read. Assume that any properly-formed event message has a valid length
> field, and move the debug print below the length check.
The length check is a bit redundant as event->datalen is assigned to
emsg.datalen upon queuing the event which also does validation. So I
would propose to just remove the length check here.
Regards,
Arend
> Suggested-by: Mattias Nissler <mnissler@chromium.org>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
^ permalink raw reply
* Re: [PATCH 3/3] brcmfmac: Add check for short event packets
From: Arend van Spriel @ 2017-09-09 8:14 UTC (permalink / raw)
To: Kevin Cernekee, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170908191342.28053-4-cernekee@chromium.org>
On 08-09-17 21:13, Kevin Cernekee wrote:
> 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.
Franky made an almost identical change which I had queued up for
submission. So you beat us to it ;-)
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Suggested-by: Mattias Nissler <mnissler@chromium.org>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read
From: Kevin Cernekee @ 2017-09-09 19:30 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
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>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
V1->V2: Clarify changelog re: whether the uninitialized data is printed.
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.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH V2 2/3] brcmfmac: Delete redundant length check
From: Kevin Cernekee @ 2017-09-09 19:30 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170909193020.19061-1-cernekee@chromium.org>
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>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 5 -----
1 file changed, 5 deletions(-)
V1->V2: Delete the check instead of moving it.
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.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH V2 3/3] brcmfmac: Add check for short event packets
From: Kevin Cernekee @ 2017-09-09 19:30 UTC (permalink / raw)
To: arend.vanspriel, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170909193020.19061-1-cernekee@chromium.org>
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>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
V1->V2: No change.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 27e661fa356f..28361bb865f3 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.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH 3.16 191/233] mac80211/wpa: use constant time memory comparison for MACs
From: Ben Hutchings @ 2017-09-09 21:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: akpm, Johannes Berg, Johannes Berg, linux-wireless,
Jason A. Donenfeld
In-Reply-To: <lsq.1504993633.197134470@decadent.org.uk>
3.16.48-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
commit 98c67d187db7808b1f3c95f2110dd4392d034182 upstream.
Otherwise, we enable all sorts of forgeries via timing attack.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
[bwh: Backported to 3.16: drop changes in
ieee80211_crypto_aes_{cmac_256,mac}_decrypt()]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/mac80211/wpa.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -16,6 +16,7 @@
#include <asm/unaligned.h>
#include <net/mac80211.h>
#include <crypto/aes.h>
+#include <crypto/algapi.h>
#include "ieee80211_i.h"
#include "michael.h"
@@ -147,7 +148,7 @@ ieee80211_rx_h_michael_mic_verify(struct
data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
key = &rx->key->conf.key[NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY];
michael_mic(key, hdr, data, data_len, mic);
- if (memcmp(mic, data + data_len, MICHAEL_MIC_LEN) != 0)
+ if (crypto_memneq(mic, data + data_len, MICHAEL_MIC_LEN))
goto mic_fail;
/* remove Michael MIC from payload */
@@ -768,7 +769,7 @@ ieee80211_crypto_aes_cmac_decrypt(struct
bip_aad(skb, aad);
ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
skb->data + 24, skb->len - 24, mic);
- if (memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+ if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
}
^ permalink raw reply
* Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read
From: Arend van Spriel @ 2017-09-10 18:50 UTC (permalink / raw)
To: Kevin Cernekee, franky.lin
Cc: brcm80211-dev-list.pdl, linux-wireless, mnissler
In-Reply-To: <20170909193020.19061-1-cernekee@chromium.org>
On 09-09-17 21:30, Kevin Cernekee wrote:
> 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>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>
> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
This patch and the others in this series look fine to me.
Thanks,
Arend
^ permalink raw reply
* Re: Patch "rtlwifi: btcoexist: Fix antenna selection code" has been added to the 4.13-stable tree
From: Sven Joachim @ 2017-09-10 20:42 UTC (permalink / raw)
To: gregkh
Cc: Larry.Finger, birming, kvalo, pkshih, shaofu, steventing,
yhchuang, stable, linux-wireless
In-Reply-To: <1505043448199217@kroah.com>
On 2017-09-10 13:37 +0200, gregkh@linuxfoundation.org wrote:
> This is a note to let you know that I've just added the patch titled
>
> rtlwifi: btcoexist: Fix antenna selection code
>
> to the 4.13-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> rtlwifi-btcoexist-fix-antenna-selection-code.patch
> and it can be found in the queue-4.13 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>
>
> From 6d622692836950b3c943776f84c4557ff6c02f3b Mon Sep 17 00:00:00 2001
> From: Larry Finger <Larry.Finger@lwfinger.net>
> Date: Mon, 4 Sep 2017 12:51:34 -0500
> Subject: rtlwifi: btcoexist: Fix antenna selection code
>
> From: Larry Finger <Larry.Finger@lwfinger.net>
>
> commit 6d622692836950b3c943776f84c4557ff6c02f3b upstream.
>
> In commit 87d8a9f35202 ("rtlwifi: btcoex: call bind to setup btcoex"),
> the code turns on a call to exhalbtc_bind_bt_coex_withadapter(). This
> routine contains a bug that causes incorrect antenna selection for those
> HP laptops with only one antenna and an incorrectly programmed EFUSE.
> These boxes are the ones that need the ant_sel module parameter.
I am the unlucky owner of such a laptop.
> Fixes: 87d8a9f35202 ("rtlwifi: btcoex: call bind to setup btcoex")
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Ping-Ke Shih <pkshih@realtek.com>
> Cc: Yan-Hsuan Chuang <yhchuang@realtek.com>
> Cc: Birming Chiu <birming@realtek.com>
> Cc: Shaofu <shaofu@realtek.com>
> Cc: Steven Ting <steventing@realtek.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> ---
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 23 ++++++----
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -173,6 +173,16 @@ static u8 halbtc_get_wifi_central_chnl(s
>
> u8 rtl_get_hwpg_single_ant_path(struct rtl_priv *rtlpriv)
> {
> + struct rtl_mod_params *mod_params = rtlpriv->cfg->mod_params;
> +
> + /* override ant_num / ant_path */
> + if (mod_params->ant_sel) {
> + rtlpriv->btcoexist.btc_info.ant_num =
> + (mod_params->ant_sel == 1 ? ANT_X2 : ANT_X1);
> +
> + rtlpriv->btcoexist.btc_info.single_ant_path =
> + (mod_params->ant_sel == 1 ? 0 : 1);
> + }
> return rtlpriv->btcoexist.btc_info.single_ant_path;
> }
>
> @@ -183,6 +193,7 @@ u8 rtl_get_hwpg_bt_type(struct rtl_priv
>
> u8 rtl_get_hwpg_ant_num(struct rtl_priv *rtlpriv)
> {
> + struct rtl_mod_params *mod_params = rtlpriv->cfg->mod_params;
> u8 num;
>
> if (rtlpriv->btcoexist.btc_info.ant_num == ANT_X2)
> @@ -190,6 +201,10 @@ u8 rtl_get_hwpg_ant_num(struct rtl_priv
> else
> num = 1;
>
> + /* override ant_num / ant_path */
> + if (mod_params->ant_sel)
> + num = (mod_params->ant_sel == 1 ? ANT_X2 : ANT_X1) + 1;
> +
> return num;
> }
>
> @@ -861,7 +876,7 @@ bool exhalbtc_bind_bt_coex_withadapter(v
> {
> struct btc_coexist *btcoexist = &gl_bt_coexist;
> struct rtl_priv *rtlpriv = adapter;
> - u8 ant_num = 2, chip_type, single_ant_path = 0;
> + u8 ant_num = 2, chip_type;
>
> if (btcoexist->binded)
> return false;
> @@ -896,12 +911,6 @@ bool exhalbtc_bind_bt_coex_withadapter(v
> ant_num = rtl_get_hwpg_ant_num(rtlpriv);
> exhalbtc_set_ant_num(rtlpriv, BT_COEX_ANT_TYPE_PG, ant_num);
>
> - /* set default antenna position to main port */
> - btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT;
> -
> - single_ant_path = rtl_get_hwpg_single_ant_path(rtlpriv);
> - exhalbtc_set_single_ant_path(single_ant_path);
> -
> if (rtl_get_hwpg_package_type(rtlpriv) == 0)
> btcoexist->board_info.tfbga_package = false;
> else if (rtl_get_hwpg_package_type(rtlpriv) == 1)
>
>
> Patches currently in stable-queue which might be from Larry.Finger@lwfinger.net are
>
> queue-4.13/rtlwifi-btcoexist-fix-breakage-of-ant_sel-for-rtl8723be.patch
> queue-4.13/rtlwifi-btcoexist-fix-antenna-selection-code.patch
After applying these patches on top of 4.13.1 the WiFi on my laptop
works again (thanks, Larry!), but now rtl8723be needs the ant_sel=2
parameter which is a bit odd, because previously it had been working
(only) with ant_sel=1. This looks like it has not been intended?
Cheers,
Sven
^ permalink raw reply
* Re: Patch "rtlwifi: btcoexist: Fix antenna selection code" has been added to the 4.13-stable tree
From: Larry Finger @ 2017-09-11 0:28 UTC (permalink / raw)
To: Sven Joachim, gregkh
Cc: birming, kvalo, pkshih, shaofu, steventing, yhchuang, stable,
linux-wireless
In-Reply-To: <87bmmib812.fsf@turtle.gmx.de>
On 09/10/2017 03:42 PM, Sven Joachim wrote:
>> queue-4.13/rtlwifi-btcoexist-fix-antenna-selection-code.patch
>
> After applying these patches on top of 4.13.1 the WiFi on my laptop
> works again (thanks, Larry!), but now rtl8723be needs the ant_sel=2
> parameter which is a bit odd, because previously it had been working
> (only) with ant_sel=1. This looks like it has not been intended?
The changes in the BT coexistence are rather substantial, and I had no part in
those changes. That code is used in both Windows and Linux, and the Linux group
is presented with the package. The best we can do is divide their massive
changes into pieces that are small enough to be accepted by the Linux community.
One thing I can say is that when a single antenna is connected to port 2 on my
card, ant_sel=1 works. When it is connected to post 1, then ant_sel=0 works;
however, my card is encoded to use two antennas. As I do not know what value is
encoded in your EFUSE, I cannot predict what value should work for you.
What I am reasonably certain is that if you were to open your computer and move
the single antenna lead to the other port, then you would not need to use any
ant_sel command as the new port would match the EFUSE value. According to
reports, that would break the Windows driver, but I do not have the setup
necessary to test that assertion.
As I have stated before, this ant_sel code was implemented to provide a service
for Linux users that were screwed by their vendor. If I had known how much grief
this attempt would cause me, I would have told the affected users to complain to
that vendor. It is clearly true that no good deed goes unpunished!
Larry
^ permalink raw reply
* Re: [PATCH 3/3] brcmfmac: Add check for short event packets
From: Mattias Nissler @ 2017-09-11 9:19 UTC (permalink / raw)
To: Kevin Cernekee
Cc: arend.vanspriel, franky.lin, brcm80211-dev-list.pdl,
linux-wireless
In-Reply-To: <20170908191342.28053-4-cernekee@chromium.org>
On Fri, Sep 8, 2017 at 9:13 PM, Kevin Cernekee <cernekee@chromium.org> wrote:
>
> 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>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
> index 5aabdc9ed7e0..4cad1f0d2a82 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
> @@ -429,7 +429,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)
Shouldn't this check be larger-than, i.e. we need the packet to be at
least sizeof(*event_packet) + its payload size?
> return;
>
> if (in_interrupt())
> --
> 2.14.1.581.gf28d330327-goog
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox