* Re: BCM43602 firmware reports multiple BRCMF_E_DEAUTH
From: Arend Van Spriel @ 2016-10-14 10:13 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, brcm80211 development
In-Reply-To: <b395146e-1917-e8d7-04e6-5d544f17531e@gmail.com>
Ok. Did you also try the firmware I sent you?
Regards,
Arend
On Fri, Oct 14, 2016 at 8:33 AM, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com>=
wrote:
> On 10/05/2016 11:08 AM, Arend Van Spriel wrote:
>>
>> On 4-10-2016 20:15, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>
>>> On 09/28/2015 11:00 AM, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>>
>>>> I'm using recent brcmfmac and brcmfmac43602-pcie.ap.bin that currently
>>>> sits in linux-firmware.git.
>>>>
>>>> In OpenWrt we have hostapd with a feature of banning STAs. It works in
>>>> a quite simple way. Whenever hostapd gets NL80211_CMD_NEW_STATION for
>>>> STA that is banned it sends NL80211_CMD_DEL_STATION.
>>>>
>>>> The problem is that in such case BCM43602 firmware happens to randomly
>>>> send more than 1 BRCMF_E_DEAUTH event. It seems it can send random
>>>> amount between 1 and 3. Looks a bit like some kind of race. It's
>>>> nothing really critical, just makes hostapd log a bit confusing.
>>>>
>>>> Could someone at Broadcom look at firmware source to see if you can
>>>> fix this, please?
>>>
>>>
>>> Hey, I didn't get any reply on this for a year. I just saw similar
>>> problem with
>>> BCM4366. Below you will find a nice log with my extra comments.
>>>
>>> Could take a look at this issue this time, please?
>>
>>
>> Can try.
>
>
> Thank you and sorry for my late reply! I'm back home now ready to provide
> any
> extra details.
>
>
>>> I think it may be another problem related to the A-MPDU thing (bug?) I
>>> reported
>>> in "AMPDU stalls with brcmfmac4366b-pcie.bin triggering WARNINGs" e-mai=
l
>>> thread.
>>
>>
>> So what firmware version do you have? A colleague pointed me to firmware
>> fix that may be related so I want to know the target string to build.
>> Firmware version is in the bin file:
>>
>> $ hexdump -C fw.bin | tail -40
>
>
> Well, I am pretty sure I was using the one released by Broadcom. Also my
> only
> device with 4366b1 is DIR-885L. Once I was looking for 4366c0 firmware as
> described in the kernel's bugzilla:
> https://bugzilla.kernel.org/show_bug.cgi?id=3D135321
> but I don't think I replaced them or anything.
>
> Anyway, I repeated my tests paying attention to the firmware file. I'm
> pretty
> sure it's the same one you can find in:
> https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/=
tree/brcm/brcmfmac4366b-pcie.bin
>
> root@lede:/# md5sum /lib/firmware/brcm/brcmfmac4366b-pcie.bin
> 596f13d84e0042035cdb41202cfc385a /lib/firmware/brcm/brcmfmac4366b-pcie.b=
in
>
> root@lede:/# hexdump -C /lib/firmware/brcm/brcmfmac4366b-pcie.bin | tail =
-40
> 000f1670 40 00 03 07 07 07 40 00 03 07 07 07 40 00 03 07
> |@.....@.....@...|
> 000f1680 07 07 40 00 72 61 74 65 73 65 6c 00 73 74 66 00
> |..@.ratesel.stf.|
> 000f1690 63 63 6b 5f 6f 6e 65 63 6f 72 65 5f 74 78 00 74
> |cck_onecore_tx.t|
> 000f16a0 65 6d 70 73 5f 70 65 72 69 6f 64 00 74 78 63 68
> |emps_period.txch|
> 000f16b0 61 69 6e 00 72 78 63 68 61 69 6e 00 4d cc 07 00
> |ain.rxchain.M...|
> 000f16c0 69 44 26 00 71 c9 07 00 91 c6 07 00 2d e9 ff 41
> |iD&.q.......-..A|
> 000f16d0 80 46 4f f4 40 70 0d 46 17 46 1e 46 1f f5 4c ff
> |.FO.@p.F.F.F..L.|
> 000f16e0 04 46 48 b9 28 46 1f f5 4d ff 02 46 23 49 24 48
> |.FH.(F..M..F#I$H|
> 000f16f0 13 f7 24 fb 1e 20 3e e0 00 21 4f f4 40 72 12 f5 |..$..
>>..!O.@r..|
> 000f1700 9d fd 0a 9b 40 46 00 93 04 f1 f8 03 01 93 04 f1
> |....@F..........|
> 000f1710 fc 03 02 93 29 46 3a 46 33 46 e0 f7 5f fa c4 f8
> |....)F:F3F.._...|
> 000f1720 f4 00 28 b9 17 48 15 49 0b 25 13 f7 07 fb 1e e0
> |..(..H.I.%......|
> 000f1730 00 22 40 f6 12 01 00 25 22 f5 26 fb c4 f8 00 01
> |."@....%".&.....|
> 000f1740 40 f6 12 01 d4 f8 f4 00 22 f5 06 fa c4 f8 e8 00
> |@.......".......|
> 000f1750 20 46 51 f7 8d fb 0c 21 00 22 20 46 51 f7 94 fb | FQ....!."
> FQ...|
> 000f1760 20 46 4d f7 69 f8 d4 f8 f4 00 df f7 d1 fe 20 46 | FM.i.......=
..
> F|
> 000f1770 1f f5 16 ff 28 46 04 b0 bd e8 f0 81 88 17 2f 00
> |....(F......../.|
> 000f1780 97 1b 2a 00 e2 f1 29 00 77 6c 63 5f 62 6d 61 63
> |..*...).wlc_bmac|
> 000f1790 5f 70 72 6f 63 65 73 73 5f 75 63 6f 64 65 5f 73
> |_process_ucode_s|
> 000f17a0 72 00 00 00 84 73 3b b4 0a 0a 45 ed 3d 22 90 56
> |r....s;...E.=3D".V|
> 000f17b0 00 00 07 00 00 00 00 00 00 00 00 00 00 00 00 a4
> |................|
> 000f17c0 91 7a c4 13 01 bd 32 08 01 00 34 33 36 36 62 31
> |.z....2...4366b1|
> 000f17d0 2d 72 6f 6d 6c 2f 70 63 69 65 2d 61 67 2d 73 70
> |-roml/pcie-ag-sp|
> 000f17e0 6c 69 74 72 78 2d 66 64 61 70 2d 6d 62 73 73 2d
> |litrx-fdap-mbss-|
> 000f17f0 6d 66 70 2d 77 6e 6d 2d 6f 73 65 6e 2d 77 6c 31
> |mfp-wnm-osen-wl1|
> 000f1800 31 6b 2d 77 6c 31 31 75 2d 74 78 62 66 2d 70 6b
> |1k-wl11u-txbf-pk|
> 000f1810 74 63 74 78 2d 61 6d 73 64 75 74 78 2d 61 6d 70
> |tctx-amsdutx-amp|
> 000f1820 64 75 72 65 74 72 79 2d 63 68 6b 64 32 68 64 6d
> |duretry-chkd2hdm|
> 000f1830 61 2d 70 72 6f 70 74 78 73 74 61 74 75 73 2d 31
> |a-proptxstatus-1|
> 000f1840 31 6e 70 72 6f 70 2d 6f 62 73 73 2d 64 62 77 73
> |1nprop-obss-dbws|
> 000f1850 77 2d 72 69 6e 67 65 72 2d 64 6d 61 69 6e 64 65
> |w-ringer-dmainde|
> 000f1860 78 31 36 2d 62 67 64 66 73 20 56 65 72 73 69 6f |x16-bgdfs
> Versio|
> 000f1870 6e 3a 20 31 30 2e 31 30 2e 36 39 2e 32 33 37 20 |n: 10.10.69.=
237
> |
> 000f1880 43 52 43 3a 20 34 62 63 34 38 63 37 62 20 44 61 |CRC: 4bc48c7=
b
> Da|
> 000f1890 74 65 3a 20 46 72 69 20 32 30 31 36 2d 30 31 2d |te: Fri
> 2016-01-|
> 000f18a0 30 38 20 31 32 3a 35 35 3a 32 35 20 50 53 54 20 |08 12:55:25 =
PST
> |
> 000f18b0 55 63 6f 64 65 20 56 65 72 3a 20 31 30 37 33 2e |Ucode Ver:
> 1073.|
> 000f18c0 35 33 31 20 46 57 49 44 3a 20 30 31 2d 63 34 37 |531 FWID:
> 01-c47|
> 000f18d0 61 39 31 61 34 0a 00 0d 01 |a91a4....|
> 000f18d9
>
> And there is a log using that very firmware I verified above.
>
> # I got some timeouts, this time without ampdu_dbg or wlc_ampdu_watchdog
> Fri Oct 14 06:09:07 2016 kern.err kernel: [ 437.579199] brcmfmac:
> brcmf_netdev_wait_pend8021x: Timed out waiting for no pending 802.1x pack=
ets
> Fri Oct 14 06:09:08 2016 kern.err kernel: [ 438.529199] brcmfmac:
> brcmf_netdev_wait_pend8021x: Timed out waiting for no pending 802.1x pack=
ets
>
> # Firmware sends BRCMF_E_DEAUTH and BRCMF_E_DISASSOC_IND events.
> # My smartphone never receives deauth/disassoc and it believes it's still
> # connected to the AP.
> Fri Oct 14 06:09:12 2016 kern.debug kernel: [ 442.276363] brcmfmac:
> CONSOLE: 027172.113 wl0: Proxy STA 78:d6:f0:9b:ba:bc link is already gone
> !!??
> Fri Oct 14 06:09:12 2016 kern.err kernel: [ 442.276405] brcmfmac:
> brcmf_notify_connect_status_ap: event 5, reason 3
> Fri Oct 14 06:09:12 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba=
:bc
> IEEE 802.11: disassociated
> Fri Oct 14 06:09:12 2016 kern.err kernel: [ 442.276447] brcmfmac:
> brcmf_notify_connect_status_ap: event 5, reason 3
> Fri Oct 14 06:09:12 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba=
:bc
> IEEE 802.11: disassociated
>
> # My smartphone starts sending packets. Firmware reacts by sending
> # BRCMF_E_DEAUTH events to the driver.
> Fri Oct 14 06:10:57 2016 kern.err kernel: [ 547.213534] brcmfmac:
> brcmf_notify_connect_status_ap: event 5, reason 7
> Fri Oct 14 06:10:57 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba=
:bc
> IEEE 802.11: disassociated
> Fri Oct 14 06:10:57 2016 kern.err kernel: [ 547.321590] brcmfmac:
> brcmf_notify_connect_status_ap: event 5, reason 7
> Fri Oct 14 06:10:57 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba=
:bc
> IEEE 802.11: disassociated
> Fri Oct 14 06:10:57 2016 kern.err kernel: [ 547.321918] brcmfmac:
> brcmf_notify_connect_status_ap: event 5, reason 7
> Fri Oct 14 06:10:57 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba=
:bc
> IEEE 802.11: disassociated
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 10:00 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu_p7neD-ttCBBA6p7VSkQt8mvfhS9cdcLftdkXNtAAFnQ@mail.gmail.com>
> So why is the performance hit acceptable for ESP but not for WPA? We
> could easily implement the same thing, i.e.,
> kmalloc(GFP_ATOMIC)/kfree the aead_req struct rather than allocate it
> on the stack
Yeah, maybe we should. It's likely a much bigger allocation, but I
don't actually know if that affects speed.
In most cases where you want high performance we never hit this anyway
since we'll have hardware crypto. I know for our (Intel's) devices we
normally never hit these code paths.
But on the other hand, you also did your changes for a reason, and the
only reason I can see of that is performance. So you'd be the one with
most "skin in the game", I guess?
johannes
^ permalink raw reply
* Re: [PATCH] ath9k: Move generic entries below specific ones in ath_pci_id_table.
From: Vittorio Gambaletta (VittGam) @ 2016-10-14 9:49 UTC (permalink / raw)
To: Valo, Kalle; +Cc: linux-wireless, ath9k-devel, ath9k-devel, stable
In-Reply-To: <877f9dehn0.fsf@kamboji.qca.qualcomm.com>
Hello,
On 12/10/2016 17:01:08 CEST, Valo, Kalle wrote:
> "Vittorio Gambaletta (VittGam)" <linux-wireless@vittgam.net> writes:
>
>>>> So, after seeing that the rest of the file is sorted this way (generic
>>>> section after the specific ones), I concluded that the 0x002A sorting
>>>> was wrong in the first place, and so is 0x0029. Then I sent this patch
>>>> to fix this.
>>>
>>> I can't see how changing the order in ath_pci_id_table[] would make any
>>> difference in functionality, but I might be missing something.
>>
>> It does: I've looked through the relevant code, and found that PCI device
>> matching from that table is done sequentially in pci_match_id() from
>> drivers/pci/pci-driver.c.
>>
>> So if a generic PCI_VDEVICE entry (that has both subvendor and subdevice IDs
>> set to PCI_ANY_ID) is put before a more specific one (PCI_DEVICE_SUB), then
>> the generic PCI_VDEVICE entry will match first and will be used.
>
> Ah, now it makes sense. Thanks for patiently explaining this to me :)
You're welcome :)
> So to tell the full story I'll change the commit log to something like
> below. Does it look ok to you?
Yes; but I'd change "So" to "This turned out to be wrong", and add a note
about changing the order for 0x002A too:
----------------------------------------------------------------------
ath9k: Really fix LED polarity for some Mini PCI AR9220 MB92 cards.
The active_high LED of my Wistron DNMA-92 is still being recognized as
active_low on 4.7.6 mainline. When I was preparing my former commit
0f9edcdd88a9 ("ath9k: Fix LED polarity for some Mini PCI AR9220 MB92
cards.") to fix that I must have somehow messed up with testing, because
I tested the final version of that patch before sending it, and it was
apparently working; but now it is not working on 4.7.6 mainline.
I initially added the PCI_DEVICE_SUB section for 0x0029/0x2096 above the
PCI_VDEVICE section for 0x0029; but then I moved the former below the
latter after seeing how 0x002A sections were sorted in the file.
This turned out to be wrong: if a generic PCI_VDEVICE entry (that has
both subvendor and subdevice IDs set to PCI_ANY_ID) is put before a more
specific one (PCI_DEVICE_SUB), then the generic PCI_VDEVICE entry will
match first and will be used.
With this patch, 0x0029/0x2096 has finally got active_high LED on 4.7.6.
While I'm at it, let's fix 0x002A too by also moving its generic definition
below its specific ones.
Fixes: 0f9edcdd88a9 ("ath9k: Fix LED polarity for some Mini PCI AR9220 MB92 cards.")
Cc: <stable@vger.kernel.org> #4.7+
Signed-off-by: Vittorio Gambaletta <linuxbugs@vittgam.net>
[kvalo@qca.qualcomm.com: improve the commit log based on email discussions]
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
----------------------------------------------------------------------
Cheers,
Vittorio
^ permalink raw reply
* [PATCH] p54: memset(0) whole array
From: Jiri Slaby @ 2016-10-14 9:23 UTC (permalink / raw)
To: chunkeey; +Cc: linux-kernel, Jiri Slaby, Kalle Valo, linux-wireless, netdev
gcc 7 complains:
drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
Fix that by passing the correct size to memset.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Christian Lamparter <chunkeey@googlemail.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/intersil/p54/fwio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
index 257a9eadd595..4ac6764f4897 100644
--- a/drivers/net/wireless/intersil/p54/fwio.c
+++ b/drivers/net/wireless/intersil/p54/fwio.c
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u16 mode, u16 dwell)
entry += sizeof(__le16);
chan->pa_points_per_curve = 8;
- memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+ memset(chan->curve_data, 0, sizeof(chan->curve_data));
memcpy(chan->curve_data, entry,
sizeof(struct p54_pa_curve_data_sample) *
min((u8)8, curve_data->points_per_channel));
--
2.10.1
^ permalink raw reply related
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 9:35 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476437149.31114.27.camel@sipsolutions.net>
On 14 October 2016 at 10:25, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 10:21 +0100, Ard Biesheuvel wrote:
>
>> It is annotated with a TODO, though :-)
>>
>> 38320c70d282b (Herbert Xu 2008-01-28 19:35:05
>> -0800 41)
>> * TODO: Use spare space in skb for this where possible.
>
> I saw that, but I don't think generally there will be spare space for
> it - the stuff there is likely far too big. Anyway ... same problem
> that we have.
>
> I'm not inclined to allocate ~500 bytes temporarily for every frame
> either though.
>
> Maybe we could try to manage it in mac80211, we'd "only" need 5 AEAD
> structs (which are today on the stack) in parallel for each key (4 TX,
> 1 RX), but in a typical case of having 3 keys that's already 7.5K worth
> of memory that we almost never use. Again, with more complexity, we
> could know that the TX will not be used if the driver does the TX, but
> the single RX one we'd need unconditionally... decisions decisions...
>
So why is the performance hit acceptable for ESP but not for WPA? We
could easily implement the same thing, i.e., kmalloc(GFP_ATOMIC)/kfree
the aead_req struct rather than allocate it on the stack
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 9:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu-Y6wuMGuRgrHRGCvAM9qZjvufs=7VWL8hAUKFSF=tZWg@mail.gmail.com>
On Fri, 2016-10-14 at 10:21 +0100, Ard Biesheuvel wrote:
> It is annotated with a TODO, though :-)
>
> 38320c70d282b (Herbert Xu 2008-01-28 19:35:05
> -0800 41)
> * TODO: Use spare space in skb for this where possible.
I saw that, but I don't think generally there will be spare space for
it - the stuff there is likely far too big. Anyway ... same problem
that we have.
I'm not inclined to allocate ~500 bytes temporarily for every frame
either though.
Maybe we could try to manage it in mac80211, we'd "only" need 5 AEAD
structs (which are today on the stack) in parallel for each key (4 TX,
1 RX), but in a typical case of having 3 keys that's already 7.5K worth
of memory that we almost never use. Again, with more complexity, we
could know that the TX will not be used if the driver does the TX, but
the single RX one we'd need unconditionally... decisions decisions...
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 9:21 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476436248.31114.21.camel@sipsolutions.net>
On 14 October 2016 at 10:10, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 10:05 +0100, Ard Biesheuvel wrote:
>>
>> Indeed. And the decrypt path does the same for auth_tag[].
>
> Hadn't gotten that far, due to the BUG_ON() in CONFIG_DEBUG_SG in the
> encrypt path :)
>
>> But that still means there are two separate problems here, one which
>> affects the WPA code, and one that only affects the generic CCM
>> chaining mode (but not the accelerated arm64 implementation)
>
> Yes. The generic CCM chaining still doesn't typically have a request on
> the stack though. In fact, ESP (net/ipv4/esp4.c) for example will do
> temporary allocations with kmalloc for every frame, it seems.
>
It is annotated with a TODO, though :-)
38320c70d282b (Herbert Xu 2008-01-28 19:35:05 -0800 41)
* TODO: Use spare space in skb for this where possible.
>> Unsurprisingly, I would strongly prefer those to be fixed properly
>> rather than backing out my patch, but I'm happy to help out whichever
>> solution we reach consensus on.
>
> Yeah, obviously, it would be good to use the accelerated versions after
> all.
>
>> I will check whether this removes the issue when not using
>> crypto/ccm.ko
>
> Ok. I think we can probably live with having those 48 bytes in per-CPU
> buffers, but I suppose we don't really want to have ~500.
>
Agreed.
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 9:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu9KObA61fEYxr1FFj_3SKNS2RUbqZo01yfnm9Bf0AncwA@mail.gmail.com>
On Fri, 2016-10-14 at 10:05 +0100, Ard Biesheuvel wrote:
>
> Indeed. And the decrypt path does the same for auth_tag[].
Hadn't gotten that far, due to the BUG_ON() in CONFIG_DEBUG_SG in the
encrypt path :)
> But that still means there are two separate problems here, one which
> affects the WPA code, and one that only affects the generic CCM
> chaining mode (but not the accelerated arm64 implementation)
Yes. The generic CCM chaining still doesn't typically have a request on
the stack though. In fact, ESP (net/ipv4/esp4.c) for example will do
temporary allocations with kmalloc for every frame, it seems.
> Unsurprisingly, I would strongly prefer those to be fixed properly
> rather than backing out my patch, but I'm happy to help out whichever
> solution we reach consensus on.
Yeah, obviously, it would be good to use the accelerated versions after
all.
> I will check whether this removes the issue when not using
> crypto/ccm.ko
Ok. I think we can probably live with having those 48 bytes in per-CPU
buffers, but I suppose we don't really want to have ~500.
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 9:05 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476435303.31114.14.camel@sipsolutions.net>
On 14 October 2016 at 09:55, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 09:47 +0100, Ard Biesheuvel wrote:
>>
>> Do you have a reference for the sg_set_buf() call on odata?
>> crypto/ccm.c does not seem to have it (afaict),
>
> It's indirect - crypto_ccm_encrypt() calls crypto_ccm_init_crypt()
> which does it.
>
Indeed. And the decrypt path does the same for auth_tag[].
But that still means there are two separate problems here, one which
affects the WPA code, and one that only affects the generic CCM
chaining mode (but not the accelerated arm64 implementation)
Unsurprisingly, I would strongly prefer those to be fixed properly
rather than backing out my patch, but I'm happy to help out whichever
solution we reach consensus on.
>> and the same problem
>> does not exist in the accelerated arm64 implementation. In the mean
>> time, I will try and see if we can move aad[] off the stack in the
>> WPA code.
>
> I had that with per-CPU buffers, just sent the patch upthread.
>
I will check whether this removes the issue when not using crypto/ccm.ko
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu896xme5sd5i8hs7tA=Xt=qQKCiAx7fQg1ZECn50NttbQ@mail.gmail.com>
On Fri, 2016-10-14 at 09:47 +0100, Ard Biesheuvel wrote:
>
> Do you have a reference for the sg_set_buf() call on odata?
> crypto/ccm.c does not seem to have it (afaict),
It's indirect - crypto_ccm_encrypt() calls crypto_ccm_init_crypt()
which does it.
> and the same problem
> does not exist in the accelerated arm64 implementation. In the mean
> time, I will try and see if we can move aad[] off the stack in the
> WPA code.
I had that with per-CPU buffers, just sent the patch upthread.
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:53 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Ard Biesheuvel
In-Reply-To: <1476429916.4382.12.camel@sipsolutions.net>
For reference, this was my patch moving the mac80211 buffers to percpu.
diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7663c28ba353..c3709ddf71e9 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -29,6 +29,8 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *) aead_req_data;
+ printk(KERN_INFO "ccm size: %d\n", sizeof(aead_req_data));
+
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_table(sg, 3);
@@ -37,6 +39,9 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
sg_set_buf(&sg[2], mic, mic_len);
aead_request_set_tfm(aead_req, tfm);
+
+ printk(KERN_INFO "aead: %pf\n", crypto_aead_alg(crypto_aead_reqtfm(aead_req))->encrypt);
+
aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
aead_request_set_ad(aead_req, sg[0].length);
@@ -67,6 +72,8 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
aead_request_set_crypt(aead_req, sg, sg, data_len + mic_len, b_0);
aead_request_set_ad(aead_req, sg[0].length);
+ printk(KERN_INFO "aead: %pf\n", crypto_aead_alg(crypto_aead_reqtfm(aead_req))->decrypt);
+
return crypto_aead_decrypt(aead_req);
}
diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index bdf0790d89cc..ebb8c2dc9928 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -20,7 +20,6 @@
#define CMAC_TLEN 8 /* CMAC TLen = 64 bits (8 octets) */
#define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
-#define AAD_LEN 20
static void gf_mulx(u8 *pad)
@@ -101,7 +100,7 @@ void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
memset(zero, 0, CMAC_TLEN);
addr[0] = aad;
- len[0] = AAD_LEN;
+ len[0] = CMAC_AAD_LEN;
addr[1] = data;
len[1] = data_len - CMAC_TLEN;
addr[2] = zero;
@@ -119,7 +118,7 @@ void ieee80211_aes_cmac_256(struct crypto_cipher *tfm, const u8 *aad,
memset(zero, 0, CMAC_TLEN_256);
addr[0] = aad;
- len[0] = AAD_LEN;
+ len[0] = CMAC_AAD_LEN;
addr[1] = data;
len[1] = data_len - CMAC_TLEN_256;
addr[2] = zero;
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index 3702041f44fd..6645f8963278 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -11,6 +11,8 @@
#include <linux/crypto.h>
+#define CMAC_AAD_LEN 20
+
struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
size_t key_len);
void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
diff --git a/net/mac80211/aes_gcm.c b/net/mac80211/aes_gcm.c
index 3afe361fd27c..13e64d383c46 100644
--- a/net/mac80211/aes_gcm.c
+++ b/net/mac80211/aes_gcm.c
@@ -25,6 +25,8 @@ void ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *)aead_req_data;
+ printk(KERN_DEBUG "gcm size: %d\n", sizeof(aead_req_data));
+
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_table(sg, 3);
diff --git a/net/mac80211/aes_gmac.c b/net/mac80211/aes_gmac.c
index 3ddd927aaf30..a2fc69ec5ca9 100644
--- a/net/mac80211/aes_gmac.c
+++ b/net/mac80211/aes_gmac.c
@@ -19,17 +19,18 @@
#define GMAC_MIC_LEN 16
#define GMAC_NONCE_LEN 12
-#define AAD_LEN 20
int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
- const u8 *data, size_t data_len, u8 *mic)
+ const u8 *data, size_t data_len, u8 *mic, u8 *zero)
{
struct scatterlist sg[4];
char aead_req_data[sizeof(struct aead_request) +
crypto_aead_reqsize(tfm)]
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *)aead_req_data;
- u8 zero[GMAC_MIC_LEN], iv[AES_BLOCK_SIZE];
+ u8 iv[AES_BLOCK_SIZE];
+
+ printk(KERN_DEBUG "gmac size: %d\n", sizeof(aead_req_data));
if (data_len < GMAC_MIC_LEN)
return -EINVAL;
@@ -38,7 +39,7 @@ int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
memset(zero, 0, GMAC_MIC_LEN);
sg_init_table(sg, 4);
- sg_set_buf(&sg[0], aad, AAD_LEN);
+ sg_set_buf(&sg[0], aad, GMAC_AAD_LEN);
sg_set_buf(&sg[1], data, data_len - GMAC_MIC_LEN);
sg_set_buf(&sg[2], zero, GMAC_MIC_LEN);
sg_set_buf(&sg[3], mic, GMAC_MIC_LEN);
@@ -49,7 +50,7 @@ int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
aead_request_set_tfm(aead_req, tfm);
aead_request_set_crypt(aead_req, sg, sg, 0, iv);
- aead_request_set_ad(aead_req, AAD_LEN + data_len);
+ aead_request_set_ad(aead_req, GMAC_AAD_LEN + data_len);
crypto_aead_encrypt(aead_req);
diff --git a/net/mac80211/aes_gmac.h b/net/mac80211/aes_gmac.h
index d328204d73a8..f06833c9095f 100644
--- a/net/mac80211/aes_gmac.h
+++ b/net/mac80211/aes_gmac.h
@@ -11,10 +11,13 @@
#include <linux/crypto.h>
+#define GMAC_MIC_LEN 16
+#define GMAC_AAD_LEN 20
+
struct crypto_aead *ieee80211_aes_gmac_key_setup(const u8 key[],
size_t key_len);
int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
- const u8 *data, size_t data_len, u8 *mic);
+ const u8 *data, size_t data_len, u8 *mic, u8 *zero);
void ieee80211_aes_gmac_key_free(struct crypto_aead *tfm);
#endif /* AES_GMAC_H */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 103187ca9474..bc2a3282e0a1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1129,6 +1129,14 @@ enum mac80211_scan_state {
SCAN_ABORT,
};
+struct ieee80211_crypto_bufs {
+
+ u8 buf1[32];
+ u8 buf2[16];
+};
+
+extern struct ieee80211_crypto_bufs __percpu *ieee80211_crypto_bufs;
+
struct ieee80211_local {
/* embed the driver visible part.
* don't cast (use the static inlines below), but we keep
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 1075ac24c8c5..6175cde94c53 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -33,6 +33,8 @@
#include "led.h"
#include "debugfs.h"
+struct ieee80211_crypto_bufs __percpu *ieee80211_crypto_bufs;
+
void ieee80211_configure_filter(struct ieee80211_local *local)
{
u64 mc;
@@ -1234,6 +1236,10 @@ static int __init ieee80211_init(void)
BUILD_BUG_ON(offsetof(struct ieee80211_tx_info, driver_data) +
IEEE80211_TX_INFO_DRIVER_DATA_SIZE > sizeof(skb->cb));
+ ieee80211_crypto_bufs = alloc_percpu(struct ieee80211_crypto_bufs);
+ if (!ieee80211_crypto_bufs)
+ return -ENOMEM;
+
ret = rc80211_minstrel_init();
if (ret)
return ret;
@@ -1264,6 +1270,8 @@ static void __exit ieee80211_exit(void)
ieee80211_iface_exit();
+ free_percpu(ieee80211_crypto_bufs);
+
rcu_barrier();
}
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index b48c1e13e281..c02634c4210c 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -405,8 +405,13 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
u8 *pos;
u8 pn[6];
u64 pn64;
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 b_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *b_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
@@ -534,8 +539,14 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
}
if (!(status->flag & RX_FLAG_DECRYPTED)) {
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 b_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *b_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
+
/* hardware didn't decrypt/verify MIC */
ccmp_special_blocks(skb, pn, b_0, aad);
@@ -639,8 +650,13 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
u8 *pos;
u8 pn[6];
u64 pn64;
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 j_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *j_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
@@ -764,8 +780,14 @@ ieee80211_crypto_gcmp_decrypt(struct ieee80211_rx_data *rx)
}
if (!(status->flag & RX_FLAG_DECRYPTED)) {
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 j_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *j_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
+
/* hardware didn't decrypt/verify MIC */
gcmp_special_blocks(skb, pn, j_0, aad);
@@ -935,8 +957,12 @@ ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
struct ieee80211_tx_info *info;
struct ieee80211_key *key = tx->key;
struct ieee80211_mmie *mmie;
- u8 aad[20];
u64 pn64;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
return TX_DROP;
@@ -979,8 +1005,12 @@ ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx)
struct ieee80211_tx_info *info;
struct ieee80211_key *key = tx->key;
struct ieee80211_mmie_16 *mmie;
- u8 aad[20];
u64 pn64;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
return TX_DROP;
@@ -1022,8 +1052,13 @@ ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_key *key = rx->key;
struct ieee80211_mmie *mmie;
- u8 aad[20], mic[8], ipn[6];
+ u8 mic[8], ipn[6];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (!ieee80211_is_mgmt(hdr->frame_control))
return RX_CONTINUE;
@@ -1072,8 +1107,13 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_key *key = rx->key;
struct ieee80211_mmie_16 *mmie;
- u8 aad[20], mic[16], ipn[6];
+ u8 mic[16], ipn[6];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (!ieee80211_is_mgmt(hdr->frame_control))
return RX_CONTINUE;
@@ -1123,9 +1163,15 @@ ieee80211_crypto_aes_gmac_encrypt(struct ieee80211_tx_data *tx)
struct ieee80211_key *key = tx->key;
struct ieee80211_mmie_16 *mmie;
struct ieee80211_hdr *hdr;
- u8 aad[20];
u64 pn64;
u8 nonce[12];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *zero = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < GMAC_AAD_LEN);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < GMAC_MIC_LEN);
if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
return TX_DROP;
@@ -1158,7 +1204,8 @@ ieee80211_crypto_aes_gmac_encrypt(struct ieee80211_tx_data *tx)
/* MIC = AES-GMAC(IGTK, AAD || Management Frame Body || MMIE, 128) */
if (ieee80211_aes_gmac(key->u.aes_gmac.tfm, aad, nonce,
- skb->data + 24, skb->len - 24, mmie->mic) < 0)
+ skb->data + 24, skb->len - 24, mmie->mic,
+ zero) < 0)
return TX_DROP;
return TX_CONTINUE;
@@ -1171,8 +1218,15 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_key *key = rx->key;
struct ieee80211_mmie_16 *mmie;
- u8 aad[20], mic[16], ipn[6], nonce[12];
+ u8 mic[16], ipn[6], nonce[12];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *zero = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < GMAC_AAD_LEN);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < GMAC_MIC_LEN);
if (!ieee80211_is_mgmt(hdr->frame_control))
return RX_CONTINUE;
@@ -1204,7 +1258,7 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
if (ieee80211_aes_gmac(key->u.aes_gmac.tfm, aad, nonce,
skb->data + 24, skb->len - 24,
- mic) < 0 ||
+ mic, zero) < 0 ||
memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
key->u.aes_gmac.icverrors++;
return RX_DROP_UNUSABLE;
^ permalink raw reply related
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 8:47 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476434561.31114.7.camel@sipsolutions.net>
On 14 October 2016 at 09:42, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 09:41 +0100, Ard Biesheuvel wrote:
>
>> > I assume the stack buffer itself is not the problem here, but aad,
>> > which is allocated on the stack one frame up.
>> > Do we really need to revert the whole patch to fix that?
>>
>> Ah never mind, this is about 'odata'. Apologies, should have read
>> first
>
> Right, odata also goes into an sg list and further on.
>
> I think we should wait for Herbert to chime in before we do any further
> work though, perhaps he has any better ideas.
>
Do you have a reference for the sg_set_buf() call on odata?
crypto/ccm.c does not seem to have it (afaict), and the same problem
does not exist in the accelerated arm64 implementation. In the mean
time, I will try and see if we can move aad[] off the stack in the WPA
code.
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:45 UTC (permalink / raw)
To: Sergey Senozhatsky, Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Network Development,
Sergey Senozhatsky, Herbert Xu, David S. Miller,
Linux Wireless List, linux-kernel@vger.kernel.org
In-Reply-To: <20161014083957.GA421@swordfish>
On Fri, 2016-10-14 at 17:39 +0900, Sergey Senozhatsky wrote:
>
> given that we have a known issue shouldn't VMAP_STACK be
> disabled for now, or would you rather prefer to mark MAC80211
> as incompatible: "depends on CFG80211 && !VMAP_STACK"?
Yeah. It's a bit complicated by the fact that most people will probably
have hardware crypto in their wifi NICs, so that they won't actually
hit the software crypto path. As I said in my other email though, we
can't guarantee - even if the driver says it can do hardware crypto -
that it really will do it for all frames (some might not be able to do
for management frames for example), so we also can't really catch this
at runtime ...
Making mac80211 depend on !VMAP_STACK is probably technically best, but
I fear it'll break a lot of people's configurations who don't have a
problem right now (e.g. Linus's, who probably enabled this, but I know
where he uses wifi he uses an Intel NIC that will always do HW crypto).
Andy, what do you think?
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:42 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu_v+4D_7EvRkrSrStheuP1L-oPh=o7Ub+NU7-wgLYks4w@mail.gmail.com>
On Fri, 2016-10-14 at 09:41 +0100, Ard Biesheuvel wrote:
> > I assume the stack buffer itself is not the problem here, but aad,
> > which is allocated on the stack one frame up.
> > Do we really need to revert the whole patch to fix that?
>
> Ah never mind, this is about 'odata'. Apologies, should have read
> first
Right, odata also goes into an sg list and further on.
I think we should wait for Herbert to chime in before we do any further
work though, perhaps he has any better ideas.
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 8:41 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu8hTy8U7FcA58P1YWt2rHU09xP9v1UU0KcnD4uuaK8How@mail.gmail.com>
On 14 October 2016 at 09:39, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 14 October 2016 at 09:28, Johannes Berg <johannes@sipsolutions.net> wrote:
>>
>>> 1. revert that patch (doing so would need some major adjustments now,
>>> since it's pretty old and a number of new things were added in the
>>> meantime)
>>
>> This it will have to be, I guess.
>>
>>> 2. allocate a per-CPU buffer for all the things that we put on the
>>> stack and use in SG lists, those are:
>>> * CCM/GCM: AAD (32B), B_0/J_0 (16B)
>>> * GMAC: AAD (20B), zero (16B)
>>> * (not sure why CMAC isn't using this API, but it would be like GMAC)
>>
>> This doesn't work - I tried to move the mac80211 buffers, but because
>> we also put the struct aead_request on the stack, and crypto_ccm has
>> the "odata" in there, and we can't separate the odata from that struct,
>> we'd have to also put that into a per-CPU buffer, but it's very big -
>> 456 bytes for CCM, didn't measure the others but I'd expect them to be
>> larger, if different.
>>
>> I don't think we can allocate half a kb for each CPU just to be able to
>> possibly use the acceleration here. We can't even make that conditional
>> on not having hardware crypto in the wifi NIC because drivers are
>> always allowed to pass undecrypted frames, regardless of whether or not
>> HW crypto was attempted, so we don't know upfront if we'll have to
>> decrypt anything in software...
>>
>> Given that, I think we have had a bug in here basically since Ard's
>> patch, we never should've put these structs on the stack. Herbert, you
>> also touched this later and converted the API usage, did you see the
>> way the stack is used here and think it should be OK, or did you simply
>> not realize that?
>>
>> Ard, are you able to help out working on a revert of your patch? That
>> would require also reverting a number of other patches (various fixes,
>> API adjustments, etc. to the AEAD usage), but the more complicated part
>> is that in the meantime Jouni introduced GCMP and CCMP-256, both of
>> which we of course need to retain.
>>
>
> I am missing some context here, but could you explain what exactly is
> the problem here?
>
> Look at this code
>
> """
> struct scatterlist sg[3];
>
> char aead_req_data[sizeof(struct aead_request) +
> crypto_aead_reqsize(tfm)]
> __aligned(__alignof__(struct aead_request));
> struct aead_request *aead_req = (void *) aead_req_data;
>
> memset(aead_req, 0, sizeof(aead_req_data));
>
> sg_init_table(sg, 3);
> sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
> sg_set_buf(&sg[1], data, data_len);
> sg_set_buf(&sg[2], mic, mic_len);
>
> aead_request_set_tfm(aead_req, tfm);
> aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
> aead_request_set_ad(aead_req, sg[0].length);
> """
>
> I assume the stack buffer itself is not the problem here, but aad,
> which is allocated on the stack one frame up.
> Do we really need to revert the whole patch to fix that?
Ah never mind, this is about 'odata'. Apologies, should have read first
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Sergey Senozhatsky @ 2016-10-14 8:39 UTC (permalink / raw)
To: Andy Lutomirski, Johannes Berg
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrW8imkGTtR9-mFB=e=Gdr1QXVuhXN3GRs2B6wbmPaaGhA@mail.gmail.com>
On (10/13/16 14:49), Andy Lutomirski wrote:
[..]
> > > FAIL: 00004100002cba02 > ffffc900802cba02 || 1 -> (00004100002cba02
> > > >> 39) == 130
> >
> > Yeah, we already know that in this function the aad variable is on the
> > stack, it explicitly is.
> >
> > The question, though, is why precisely that fails in the crypto code.
> > Can you send the Oops report itself?
> >
>
> It's failing before that. With CONFIG_VMAP_STACK=y, the stack may not
> be physically contiguous and can't be used for DMA, so putting it in a
> scatterlist is bogus in general, and the crypto code mostly wants a
> scatterlist.
>
> There are a couple (faster!) APIs for crypto that don't use
> scatterlists, but I don't think AEAD works with them.
given that we have a known issue shouldn't VMAP_STACK be
disabled for now, or would you rather prefer to mark MAC80211
as incompatible: "depends on CFG80211 && !VMAP_STACK"?
-ss
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 8:39 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476433699.31114.6.camel@sipsolutions.net>
On 14 October 2016 at 09:28, Johannes Berg <johannes@sipsolutions.net> wrote:
>
>> 1. revert that patch (doing so would need some major adjustments now,
>> since it's pretty old and a number of new things were added in the
>> meantime)
>
> This it will have to be, I guess.
>
>> 2. allocate a per-CPU buffer for all the things that we put on the
>> stack and use in SG lists, those are:
>> * CCM/GCM: AAD (32B), B_0/J_0 (16B)
>> * GMAC: AAD (20B), zero (16B)
>> * (not sure why CMAC isn't using this API, but it would be like GMAC)
>
> This doesn't work - I tried to move the mac80211 buffers, but because
> we also put the struct aead_request on the stack, and crypto_ccm has
> the "odata" in there, and we can't separate the odata from that struct,
> we'd have to also put that into a per-CPU buffer, but it's very big -
> 456 bytes for CCM, didn't measure the others but I'd expect them to be
> larger, if different.
>
> I don't think we can allocate half a kb for each CPU just to be able to
> possibly use the acceleration here. We can't even make that conditional
> on not having hardware crypto in the wifi NIC because drivers are
> always allowed to pass undecrypted frames, regardless of whether or not
> HW crypto was attempted, so we don't know upfront if we'll have to
> decrypt anything in software...
>
> Given that, I think we have had a bug in here basically since Ard's
> patch, we never should've put these structs on the stack. Herbert, you
> also touched this later and converted the API usage, did you see the
> way the stack is used here and think it should be OK, or did you simply
> not realize that?
>
> Ard, are you able to help out working on a revert of your patch? That
> would require also reverting a number of other patches (various fixes,
> API adjustments, etc. to the AEAD usage), but the more complicated part
> is that in the meantime Jouni introduced GCMP and CCMP-256, both of
> which we of course need to retain.
>
I am missing some context here, but could you explain what exactly is
the problem here?
Look at this code
"""
struct scatterlist sg[3];
char aead_req_data[sizeof(struct aead_request) +
crypto_aead_reqsize(tfm)]
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *) aead_req_data;
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_table(sg, 3);
sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
sg_set_buf(&sg[1], data, data_len);
sg_set_buf(&sg[2], mic, mic_len);
aead_request_set_tfm(aead_req, tfm);
aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
aead_request_set_ad(aead_req, sg[0].length);
"""
I assume the stack buffer itself is not the problem here, but aad,
which is allocated on the stack one frame up.
Do we really need to revert the whole patch to fix that?
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:28 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jouni Malinen
In-Reply-To: <1476429916.4382.12.camel@sipsolutions.net>
> 1. revert that patch (doing so would need some major adjustments now,
> since it's pretty old and a number of new things were added in the
> meantime)
This it will have to be, I guess.
> 2. allocate a per-CPU buffer for all the things that we put on the
> stack and use in SG lists, those are:
> * CCM/GCM: AAD (32B), B_0/J_0 (16B)
> * GMAC: AAD (20B), zero (16B)
> * (not sure why CMAC isn't using this API, but it would be like GMAC)
This doesn't work - I tried to move the mac80211 buffers, but because
we also put the struct aead_request on the stack, and crypto_ccm has
the "odata" in there, and we can't separate the odata from that struct,
we'd have to also put that into a per-CPU buffer, but it's very big -
456 bytes for CCM, didn't measure the others but I'd expect them to be
larger, if different.
I don't think we can allocate half a kb for each CPU just to be able to
possibly use the acceleration here. We can't even make that conditional
on not having hardware crypto in the wifi NIC because drivers are
always allowed to pass undecrypted frames, regardless of whether or not
HW crypto was attempted, so we don't know upfront if we'll have to
decrypt anything in software...
Given that, I think we have had a bug in here basically since Ard's
patch, we never should've put these structs on the stack. Herbert, you
also touched this later and converted the API usage, did you see the
way the stack is used here and think it should be OK, or did you simply
not realize that?
Ard, are you able to help out working on a revert of your patch? That
would require also reverting a number of other patches (various fixes,
API adjustments, etc. to the AEAD usage), but the more complicated part
is that in the meantime Jouni introduced GCMP and CCMP-256, both of
which we of course need to retain.
johannes
^ permalink raw reply
* [PATCH] brcmfmac: print name of connect status event
From: Rafał Miłecki @ 2016-10-14 7:45 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This simplifies debugging. Format %s (%u) comes from similar debugging
message in brcmf_fweh_event_worker.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ++-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 4 ++--
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h | 2 ++
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b777e1b..1e7c6f0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5506,7 +5506,8 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
u32 reason = e->reason;
struct station_info sinfo;
- brcmf_dbg(CONN, "event %d, reason %d\n", event, reason);
+ brcmf_dbg(CONN, "event %s (%u), reason %d\n",
+ brcmf_fweh_event_name(event), event, reason);
if (event == BRCMF_E_LINK && reason == BRCMF_E_REASON_LINK_BSSCFG_DIS &&
ndev != cfg_to_ndev(cfg)) {
brcmf_dbg(CONN, "AP mode link down\n");
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 79c081f..c79306b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -69,7 +69,7 @@ static struct brcmf_fweh_event_name fweh_event_names[] = {
*
* @code: code to lookup.
*/
-static const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
+const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
{
int i;
for (i = 0; i < ARRAY_SIZE(fweh_event_names); i++) {
@@ -79,7 +79,7 @@ static const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
return "unknown";
}
#else
-static const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
+const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
{
return "nodebug";
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
index 26ff5a9..5fba4b4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
@@ -287,6 +287,8 @@ struct brcmf_fweh_info {
void *data);
};
+const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code);
+
void brcmf_fweh_attach(struct brcmf_pub *drvr);
void brcmf_fweh_detach(struct brcmf_pub *drvr);
int brcmf_fweh_register(struct brcmf_pub *drvr, enum brcmf_fweh_event_code code,
--
2.9.3
^ permalink raw reply related
* Re: [RFC 0/5] ath6kl: non WMI data service support
From: Valo, Kalle @ 2016-10-14 7:34 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1476376769-4708-1-git-send-email-erik.stromdahl@gmail.com>
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> This patch series is intended to prepare the ath6kl driver
> for newer chipsets that doesn't use the current WMI data
> endpoints for data traffic.
>
> The chipset I have been working with (and used for testing)
> is QCA6584. It is SDIO based (at least the variant I have
> been using) with 802.11p WAVE DSRC capabilities.
>
> This chipset is different from the AR600X family in that
> it does not use the WMI data services (service id's 0x101
> to 0x104 ) for data traffic.
> Instead it uses the HTT data service for data and wmi unified
> for control messages.
> It is also different when it comes to mailbox addresses
> and HTC header format as well, but these differences are not
> part of this patch series.
Do you have more patches implemented, like something already working or
have just started?
--=20
Kalle Valo=
^ permalink raw reply
* Re: [PATCH] cfg80211: Add support to update connection parameters
From: Johannes Berg @ 2016-10-14 7:32 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linux-wireless, vamsi krishna
In-Reply-To: <1476373178-31105-1-git-send-email-jouni@qca.qualcomm.com>
In addition to Arend's comments...
> * (invoked with the wireless_dev mutex held)
> + * @update_connect_params: Update the connect parameters while connected to a
> + * BSS. The updated parameters can be used by driver/firmware for
> + * subsequent BSS selection (roaming) decisions and to form the
> + * Authentication/(Re)Association Request frames. This call does not
> + * request an immediate disassociation or reassociation with the current
> + * BSS, i.e., this impacts only subsequence (re)associations.
The other related calls are all invoked with the wireless_dev mutex
held, I think it'd be better for consistency to replicate that here.
> +static int nl80211_update_connect_params(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct cfg80211_connect_params connect;
> + struct cfg80211_connect_params_valid cpv;
> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
> + struct net_device *dev = info->user_ptr[1];
> + struct wireless_dev *wdev = dev->ieee80211_ptr;
> +
> + memset(&connect, 0, sizeof(connect));
> + memset(&cpv, 0, sizeof(cpv));
I tend to prefer (0) C99 initalizers since it makes the code shorter,
but it doesn't really matter much.
> + if (!wdev->current_bss)
> + return -ENOLINK;
Also, regarding the locking, there's no guarantee that this won't
become NULL immediately after the check, if you don't have any locking.
Now, the driver (or more likely firmware!), would still have to protect
against races, say where the firmware disconnected while userspace
called this ... but at least software wise it'd be consistent.
johannes
^ permalink raw reply
* Re: [PATCH] cfg80211: Add support to update connection parameters
From: Johannes Berg @ 2016-10-14 7:27 UTC (permalink / raw)
To: Arend van Spriel, Jouni Malinen; +Cc: linux-wireless, vamsi krishna
In-Reply-To: <61aa0cc9-6485-b206-56ce-cd682d91dd96@broadcom.com>
> > /**
> > + * struct cfg80211_connect_params_valid - Connection parame ters
> > to be updated
>
> What's in a name? Could we just use '_updated' instead of '_valid'
> here and below.
In mac80211 we typically just have a single "u32 changed" argument,
with bits defined in an enum - wouldn't that be sufficient here as
well?
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 7:25 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Ard Biesheuvel
In-Reply-To: <CALCETrW8imkGTtR9-mFB=e=Gdr1QXVuhXN3GRs2B6wbmPaaGhA@mail.gmail.com>
On Thu, 2016-10-13 at 14:49 -0700, Andy Lutomirski wrote:
>
> It's failing before that. With CONFIG_VMAP_STACK=y, the stack may
> not be physically contiguous and can't be used for DMA, so putting it
> in a scatterlist is bogus in general, and the crypto code mostly
> wants a scatterlist.
I see, so all this stuff is getting inlined, and we crash in
sg_set_buf() because it does sg_set_page() and that obviously needs to
do virt_to_page(), which is invalid on this address now.
With CONFIG_DEBUG_SG we'd have hit the BUG_ON there instead.
It does indeed look like AEAD doesn't have any non-SG API.
So ultimately, the bug already goes back to Ard's commit 7ec7c4a9a686
("mac80211: port CCMP to cryptoapi's CCM driver") since that already
potentially used stack space for DMA.
Since we don't have any space in the SKB or anywhere else at this point
(other than the stack that we can't use), I see two ways out of this:
1. revert that patch (doing so would need some major adjustments now,
since it's pretty old and a number of new things were added in the
meantime)
2. allocate a per-CPU buffer for all the things that we put on the
stack and use in SG lists, those are:
* CCM/GCM: AAD (32B), B_0/J_0 (16B)
* GMAC: AAD (20B), zero (16B)
* (not sure why CMAC isn't using this API, but it would be like
GMAC)
Thoughts?
johannes
^ permalink raw reply
* pull-request: wireless-drivers 2016-10-14
From: Kalle Valo @ 2016-10-14 7:18 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
first wireless-drivers pull request for 4.9 and this time we have
unusually many fixes even before -rc1 is released. Most important here
are the wlcore and rtlwifi commits which fix critical regressions,
otherwise smaller impact fixes and one new sdio id for ath6kl.
Please let me know if there are any problems.
Kalle
The following changes since commit 03a1eabc3f54469abd4f1784182851b2e29630cc:
Merge branch 'mlxsw-fixes' (2016-10-04 20:28:10 -0400)
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-2016-10-14
for you to fetch changes up to 1ea2643961b0d1b8d0e4a11af5aa69b0f92d0533:
ath6kl: add Dell OEM SDIO I/O for the Venue 8 Pro (2016-10-13 14:16:33 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 4.9
wlcore
* fix a double free regression causing hard to track crashes
rtl8xxxu
* fix driver reload issues, a memory leak and an endian bug
rtlwifi
* fix a major regression introduced in 4.9 with firmware loading on
certain hardware
ath10k
* fix regression about broken cal_data debugfs file (since 4.7)
ath9k
* revert temperature compensation for AR9003+ devices, it was causing
too much problems
ath6kl
* add Dell OEM SDIO I/O for the Venue 8 Pro
----------------------------------------------------------------
Adam Williamson (1):
ath6kl: add Dell OEM SDIO I/O for the Venue 8 Pro
Felix Fietkau (1):
Revert "ath9k_hw: implement temperature compensation support for AR9003+"
Jes Sorensen (4):
rtl8xxxu: Fix memory leak in handling rxdesc16 packets
rtl8xxxu: Fix big-endian problem reporting mactime
rtl8xxxu: Fix rtl8723bu driver reload issue
rtl8xxxu: Fix rtl8192eu driver reload issue
Larry Finger (1):
rtlwifi: Fix regression caused by commit d86e64768859
Marty Faltesek (1):
ath10k: cache calibration data when the core is stopped
Wei Yongjun (1):
wlcore: sdio: drop kfree for memory allocated with devm_kzalloc
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/debug.c | 75 ++++++++++----------
drivers/net/wireless/ath/ath6kl/sdio.c | 1 +
drivers/net/wireless/ath/ath9k/ar9003_calib.c | 25 +------
drivers/net/wireless/ath/ath9k/hw.h | 1 -
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 4 +-
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 ++-
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 ++
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++-
drivers/net/wireless/realtek/rtlwifi/core.c | 2 +-
.../net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 8 +--
.../net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 13 ++--
.../net/wireless/realtek/rtlwifi/rtl8192cu/sw.c | 12 ++--
.../net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 6 +-
.../net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 8 +--
.../net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 9 +--
.../net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 12 ++--
.../net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 6 +-
.../net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 18 ++---
drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 -
drivers/net/wireless/ti/wlcore/sdio.c | 1 -
21 files changed, 110 insertions(+), 117 deletions(-)
^ permalink raw reply
* Re: BCM43602 firmware reports multiple BRCMF_E_DEAUTH
From: Rafał Miłecki @ 2016-10-14 6:33 UTC (permalink / raw)
To: Arend Van Spriel, linux-wireless@vger.kernel.org,
brcm80211 development
In-Reply-To: <90e69e94-2210-22ec-3aec-67d9605b4b7c@broadcom.com>
On 10/05/2016 11:08 AM, Arend Van Spriel wrote:
> On 4-10-2016 20:15, Rafał Miłecki wrote:
>> On 09/28/2015 11:00 AM, Rafał Miłecki wrote:
>>> I'm using recent brcmfmac and brcmfmac43602-pcie.ap.bin that currently
>>> sits in linux-firmware.git.
>>>
>>> In OpenWrt we have hostapd with a feature of banning STAs. It works in
>>> a quite simple way. Whenever hostapd gets NL80211_CMD_NEW_STATION for
>>> STA that is banned it sends NL80211_CMD_DEL_STATION.
>>>
>>> The problem is that in such case BCM43602 firmware happens to randomly
>>> send more than 1 BRCMF_E_DEAUTH event. It seems it can send random
>>> amount between 1 and 3. Looks a bit like some kind of race. It's
>>> nothing really critical, just makes hostapd log a bit confusing.
>>>
>>> Could someone at Broadcom look at firmware source to see if you can
>>> fix this, please?
>>
>> Hey, I didn't get any reply on this for a year. I just saw similar
>> problem with
>> BCM4366. Below you will find a nice log with my extra comments.
>>
>> Could take a look at this issue this time, please?
>
> Can try.
Thank you and sorry for my late reply! I'm back home now ready to provide any
extra details.
>> I think it may be another problem related to the A-MPDU thing (bug?) I
>> reported
>> in "AMPDU stalls with brcmfmac4366b-pcie.bin triggering WARNINGs" e-mail
>> thread.
>
> So what firmware version do you have? A colleague pointed me to firmware
> fix that may be related so I want to know the target string to build.
> Firmware version is in the bin file:
>
> $ hexdump -C fw.bin | tail -40
Well, I am pretty sure I was using the one released by Broadcom. Also my only
device with 4366b1 is DIR-885L. Once I was looking for 4366c0 firmware as
described in the kernel's bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=135321
but I don't think I replaced them or anything.
Anyway, I repeated my tests paying attention to the firmware file. I'm pretty
sure it's the same one you can find in:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/brcm/brcmfmac4366b-pcie.bin
root@lede:/# md5sum /lib/firmware/brcm/brcmfmac4366b-pcie.bin
596f13d84e0042035cdb41202cfc385a /lib/firmware/brcm/brcmfmac4366b-pcie.bin
root@lede:/# hexdump -C /lib/firmware/brcm/brcmfmac4366b-pcie.bin | tail -40
000f1670 40 00 03 07 07 07 40 00 03 07 07 07 40 00 03 07 |@.....@.....@...|
000f1680 07 07 40 00 72 61 74 65 73 65 6c 00 73 74 66 00 |..@.ratesel.stf.|
000f1690 63 63 6b 5f 6f 6e 65 63 6f 72 65 5f 74 78 00 74 |cck_onecore_tx.t|
000f16a0 65 6d 70 73 5f 70 65 72 69 6f 64 00 74 78 63 68 |emps_period.txch|
000f16b0 61 69 6e 00 72 78 63 68 61 69 6e 00 4d cc 07 00 |ain.rxchain.M...|
000f16c0 69 44 26 00 71 c9 07 00 91 c6 07 00 2d e9 ff 41 |iD&.q.......-..A|
000f16d0 80 46 4f f4 40 70 0d 46 17 46 1e 46 1f f5 4c ff |.FO.@p.F.F.F..L.|
000f16e0 04 46 48 b9 28 46 1f f5 4d ff 02 46 23 49 24 48 |.FH.(F..M..F#I$H|
000f16f0 13 f7 24 fb 1e 20 3e e0 00 21 4f f4 40 72 12 f5 |..$.. >..!O.@r..|
000f1700 9d fd 0a 9b 40 46 00 93 04 f1 f8 03 01 93 04 f1 |....@F..........|
000f1710 fc 03 02 93 29 46 3a 46 33 46 e0 f7 5f fa c4 f8 |....)F:F3F.._...|
000f1720 f4 00 28 b9 17 48 15 49 0b 25 13 f7 07 fb 1e e0 |..(..H.I.%......|
000f1730 00 22 40 f6 12 01 00 25 22 f5 26 fb c4 f8 00 01 |."@....%".&.....|
000f1740 40 f6 12 01 d4 f8 f4 00 22 f5 06 fa c4 f8 e8 00 |@.......".......|
000f1750 20 46 51 f7 8d fb 0c 21 00 22 20 46 51 f7 94 fb | FQ....!." FQ...|
000f1760 20 46 4d f7 69 f8 d4 f8 f4 00 df f7 d1 fe 20 46 | FM.i......... F|
000f1770 1f f5 16 ff 28 46 04 b0 bd e8 f0 81 88 17 2f 00 |....(F......../.|
000f1780 97 1b 2a 00 e2 f1 29 00 77 6c 63 5f 62 6d 61 63 |..*...).wlc_bmac|
000f1790 5f 70 72 6f 63 65 73 73 5f 75 63 6f 64 65 5f 73 |_process_ucode_s|
000f17a0 72 00 00 00 84 73 3b b4 0a 0a 45 ed 3d 22 90 56 |r....s;...E.=".V|
000f17b0 00 00 07 00 00 00 00 00 00 00 00 00 00 00 00 a4 |................|
000f17c0 91 7a c4 13 01 bd 32 08 01 00 34 33 36 36 62 31 |.z....2...4366b1|
000f17d0 2d 72 6f 6d 6c 2f 70 63 69 65 2d 61 67 2d 73 70 |-roml/pcie-ag-sp|
000f17e0 6c 69 74 72 78 2d 66 64 61 70 2d 6d 62 73 73 2d |litrx-fdap-mbss-|
000f17f0 6d 66 70 2d 77 6e 6d 2d 6f 73 65 6e 2d 77 6c 31 |mfp-wnm-osen-wl1|
000f1800 31 6b 2d 77 6c 31 31 75 2d 74 78 62 66 2d 70 6b |1k-wl11u-txbf-pk|
000f1810 74 63 74 78 2d 61 6d 73 64 75 74 78 2d 61 6d 70 |tctx-amsdutx-amp|
000f1820 64 75 72 65 74 72 79 2d 63 68 6b 64 32 68 64 6d |duretry-chkd2hdm|
000f1830 61 2d 70 72 6f 70 74 78 73 74 61 74 75 73 2d 31 |a-proptxstatus-1|
000f1840 31 6e 70 72 6f 70 2d 6f 62 73 73 2d 64 62 77 73 |1nprop-obss-dbws|
000f1850 77 2d 72 69 6e 67 65 72 2d 64 6d 61 69 6e 64 65 |w-ringer-dmainde|
000f1860 78 31 36 2d 62 67 64 66 73 20 56 65 72 73 69 6f |x16-bgdfs Versio|
000f1870 6e 3a 20 31 30 2e 31 30 2e 36 39 2e 32 33 37 20 |n: 10.10.69.237 |
000f1880 43 52 43 3a 20 34 62 63 34 38 63 37 62 20 44 61 |CRC: 4bc48c7b Da|
000f1890 74 65 3a 20 46 72 69 20 32 30 31 36 2d 30 31 2d |te: Fri 2016-01-|
000f18a0 30 38 20 31 32 3a 35 35 3a 32 35 20 50 53 54 20 |08 12:55:25 PST |
000f18b0 55 63 6f 64 65 20 56 65 72 3a 20 31 30 37 33 2e |Ucode Ver: 1073.|
000f18c0 35 33 31 20 46 57 49 44 3a 20 30 31 2d 63 34 37 |531 FWID: 01-c47|
000f18d0 61 39 31 61 34 0a 00 0d 01 |a91a4....|
000f18d9
And there is a log using that very firmware I verified above.
# I got some timeouts, this time without ampdu_dbg or wlc_ampdu_watchdog
Fri Oct 14 06:09:07 2016 kern.err kernel: [ 437.579199] brcmfmac: brcmf_netdev_wait_pend8021x: Timed out waiting for no pending 802.1x packets
Fri Oct 14 06:09:08 2016 kern.err kernel: [ 438.529199] brcmfmac: brcmf_netdev_wait_pend8021x: Timed out waiting for no pending 802.1x packets
# Firmware sends BRCMF_E_DEAUTH and BRCMF_E_DISASSOC_IND events.
# My smartphone never receives deauth/disassoc and it believes it's still
# connected to the AP.
Fri Oct 14 06:09:12 2016 kern.debug kernel: [ 442.276363] brcmfmac: CONSOLE: 027172.113 wl0: Proxy STA 78:d6:f0:9b:ba:bc link is already gone !!??
Fri Oct 14 06:09:12 2016 kern.err kernel: [ 442.276405] brcmfmac: brcmf_notify_connect_status_ap: event 5, reason 3
Fri Oct 14 06:09:12 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
Fri Oct 14 06:09:12 2016 kern.err kernel: [ 442.276447] brcmfmac: brcmf_notify_connect_status_ap: event 5, reason 3
Fri Oct 14 06:09:12 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
# My smartphone starts sending packets. Firmware reacts by sending
# BRCMF_E_DEAUTH events to the driver.
Fri Oct 14 06:10:57 2016 kern.err kernel: [ 547.213534] brcmfmac: brcmf_notify_connect_status_ap: event 5, reason 7
Fri Oct 14 06:10:57 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
Fri Oct 14 06:10:57 2016 kern.err kernel: [ 547.321590] brcmfmac: brcmf_notify_connect_status_ap: event 5, reason 7
Fri Oct 14 06:10:57 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
Fri Oct 14 06:10:57 2016 kern.err kernel: [ 547.321918] brcmfmac: brcmf_notify_connect_status_ap: event 5, reason 7
Fri Oct 14 06:10:57 2016 daemon.info hostapd: wlan1-1: STA 78:d6:f0:9b:ba:bc IEEE 802.11: disassociated
^ 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