Return-path: Received: from 4.mo173.mail-out.ovh.net ([46.105.34.219]:58622 "EHLO 4.mo173.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751481AbeCNQRx (ORCPT ); Wed, 14 Mar 2018 12:17:53 -0400 Received: from player774.ha.ovh.net (unknown [10.109.122.233]) by mo173.mail-out.ovh.net (Postfix) with ESMTP id EC958B2138 for ; Wed, 14 Mar 2018 16:40:40 +0100 (CET) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Date: Wed, 14 Mar 2018 16:40:22 +0100 From: =?UTF-8?Q?Rafa=C5=82_Mi=C5=82ecki?= To: Arend van Spriel Cc: =?UTF-8?Q?Rafa=C5=82_Mi=C5=82ecki?= , Kalle Valo , Franky Lin , Hante Meuleman , Chi-Hsien Lin , Wright Feng , Pieter-Paul Giesberts , James Hughes , linux-wireless@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com, brcm80211-dev-list@cypress.com, netdev@vger.kernel.org, =?UTF-8?Q?Linus_L=C3=BCssing?= , Felix Fietkau , bridge@lists.linux-foundation.org Subject: Re: [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default In-Reply-To: References: <20180314110119.13631-1-zajec5@gmail.com> <5AA91C67.90001@broadcom.com> Message-ID: <9499f70ecb942df0df95512565a8f428@milecki.pl> (sfid-20180314_171758_388844_5C85CCE5) Sender: linux-wireless-owner@vger.kernel.org List-ID: On 2018-03-14 16:39, Rafał Miłecki wrote: > On 2018-03-14 13:58, Arend van Spriel wrote: >> On 3/14/2018 12:01 PM, Rafał Miłecki wrote: >>> From: Rafał Miłecki >>> >>> Testing brcmfmac with more recent firmwares resulted in AP interfaces >>> not working in some specific setups. Debugging resulted in >>> discovering >>> support for IAPP in Broadcom's firmwares. This is an obsoleted >>> standard >>> and its implementation is something that: >>> 1) Most people don't need / want to use >>> 2) Can allow local DoS attacks >>> 3) Breaks AP interfaces in some specific bridge setups >>> >>> To solve issues it can cause this commit modifies brcmfmac to drop >>> IAPP >>> packets. If affects: >>> 1) Rx path: driver won't be sending these unwanted packets up. >>> 2) Tx path: driver will reject packets that would trigger STA >>> disassociation perfromed by a firmware (possible local DoS >>> attack). >>> >>> It appears there are some Broadcom's clients/users who care about >>> this >>> feature despite the drawbacks. They can switch it on by a newly added >>> Kconfig option. >> >> Thanks for taking this approach. Looks fine except for .... (see >> below) >> >> Reviewed-by: Arend van Spriel >>> Signed-off-by: Rafał Miłecki >>> --- >>> drivers/net/wireless/broadcom/brcm80211/Kconfig | 20 +++++++++++ >>> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 39 >>> ++++++++++++++++++++++ >>> 2 files changed, 59 insertions(+) >>> >>> diff --git a/drivers/net/wireless/broadcom/brcm80211/Kconfig >>> b/drivers/net/wireless/broadcom/brcm80211/Kconfig >>> index 9d99eb42d917..876787ef991a 100644 >>> --- a/drivers/net/wireless/broadcom/brcm80211/Kconfig >>> +++ b/drivers/net/wireless/broadcom/brcm80211/Kconfig >>> @@ -68,6 +68,26 @@ config BRCMFMAC_PCIE >>> IEEE802.11ac embedded FullMAC WLAN driver. Say Y if you want to >>> use the driver for an PCIE wireless card. >>> >>> +config BRCMFMAC_IAPP >>> + bool "Partial support for obsoleted Inter-Access Point Protocol" >>> + depends on BRCMFMAC >>> + ---help--- >>> + Most of Broadcom's firmwares can send 802.11f ADD frame every >>> + time new STA connects to the AP interface. Some recent ones >>> + can also disassociate STA when they receive such a frame. >> >> I do not see any evidence that this would occur only for recent >> firmware. That stuff is old and not touched recently. > > My evidence is comparing firmwares for 4366b1: 10.10.69.3309 (r610991) > vs. 10.10 (TOB) (r663589). > > The first one is from linux-firmware.git and it doesn't implement IAPP > in the TX path. The later one is what I got from you privately and it > implements it. > > Also a firmware for 4366c0: 10.10.122.20 (r683106) which is relatively > new implements IAPP in the TX path. Please also take a look at my original patch [PATCH] brcmfmac: detect & reject faked packet generated by a firmware https://patchwork.kernel.org/patch/10191451/