From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=W40shKthK1fdQXT251tWbQgxlMNBACBkMQukZwjI3nw=; b=WWH3pkPuAXbgZqoLgv07Tl3YZdJ3BMreGOUnVc6a0HiCyigbv27Qqp9V0tiNbh1/xI KwHQzfi84c/g7rANP/dtVo+3Ofe+noLppX3nD1dqaixHYGUy3f5sY+WEJu+zZSJ9tce9 1J9RgTfDjOpJUEz4oYBTvoswKV5BhItM89B2uvsbyslJOq1RAjgZ5p8oKjc8Z0CHM1sH hk5E3wpSde6Y58QLTGByy/Z+RHP8Rc46tneAEouh4VZEDQW2fjSH1Ki5kkb5x+ejYuab Vq8cuC7jauBbTL2vetaPmS3hpKuyqP5MOAUQm0y6iNmBGnWjvgIcE9nRcKq7i1OBGQ5H ggcA== Date: Wed, 14 Mar 2018 08:27:15 -0700 From: Stephen Hemminger Message-ID: <20180314082715.13deee1f@xeon-e3> In-Reply-To: <87o9jq66f3.fsf@codeaurora.org> References: <20180314110119.13631-1-zajec5@gmail.com> <878tau7n23.fsf@codeaurora.org> <5AA93530.5040001@broadcom.com> <87o9jq66f3.fsf@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Bridge] [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kalle Valo Cc: James Hughes , Arend van Spriel , bridge@lists.linux-foundation.org, netdev@vger.kernel.org, Chi-Hsien Lin , =?UTF-8?B?UmFmYcWC?= =?UTF-8?B?IE1pxYJlY2tp?= , linux-wireless@vger.kernel.org, Hante Meuleman , Pieter-Paul Giesberts , brcm80211-dev-list.pdl@broadcom.com, Wright Feng , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Felix Fietkau , brcm80211-dev-list@cypress.com, Franky Lin On Wed, 14 Mar 2018 17:08:48 +0200 Kalle Valo wrote: > Arend van Spriel writes: > > > On 3/14/2018 3:24 PM, Kalle Valo wrote: > >>> +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. > >>> >+ > >>> >+ It's important to understand this behavior can lead to a local > >>> >+ DoS security issue. Attacker may trigger disassociation of any > >>> >+ STA by sending a proper Ethernet frame to the wireless > >>> >+ interface. > >>> >+ > >>> >+ Moreover this feature may break AP interfaces in some specific > >>> >+ setups. This applies e.g. to the bridge with hairpin mode > >>> >+ enabled and IFLA_BRPORT_MCAST_TO_UCAST set. IAPP packet > >>> >+ generated by a firmware will get passed back to the wireless > >>> >+ interface and cause immediate disassociation of just-connected > >>> >+ STA. > >> Sorry for jumping late, but does it really make sense to have a Kconfig > >> option for this? I don't think we should add a Kconfig option for every > >> strange feature, there should be stronger reasons (size savings etc) > >> before adding a Kconfig option. > >> > >> And in this case the size savings can't be much. Wouldn't a module > >> parameter be simpler for a functionality change like this? > > > > Hi Kalle, > > > > Good to be wary about Kconfig option. > > I think Linus doesn't like pointless Kconfig options, me neither for > that matter, so I try to make sure the justifications are really there > before adding anything new. > > > So my reason for asking a Kconfig option is that this is directly in > > the datapaths (tx and rx) so I prefer to disable/enable it compile > > time rather then runtime. > > I'm no cpu profile expert but is really one (or two?) if checks of a > cached variable in the datapath really measurable? My guess is that it's > just noise in the results. > > But I'm not going to argue about it, if you think it's still needed I'm > fine with that. Just mention in the commit log the justification the new > Kconfig option. If you have to disable it a module parameter is not a complete disaster From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-pl0-f65.google.com ([209.85.160.65]:37772 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751289AbeCNP1T (ORCPT ); Wed, 14 Mar 2018 11:27:19 -0400 Received: by mail-pl0-f65.google.com with SMTP id w12-v6so1885314plp.4 for ; Wed, 14 Mar 2018 08:27:18 -0700 (PDT) Date: Wed, 14 Mar 2018 08:27:15 -0700 From: Stephen Hemminger To: Kalle Valo Cc: Arend van Spriel , =?UTF-8?B?UmFmYcWC?= =?UTF-8?B?IE1pxYJlY2tp?= , 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, Linus =?UTF-8?B?TMO8c3Npbmc=?= , Felix Fietkau , bridge@lists.linux-foundation.org, =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Subject: Re: [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default Message-ID: <20180314082715.13deee1f@xeon-e3> (sfid-20180314_162725_142531_86FFF162) In-Reply-To: <87o9jq66f3.fsf@codeaurora.org> References: <20180314110119.13631-1-zajec5@gmail.com> <878tau7n23.fsf@codeaurora.org> <5AA93530.5040001@broadcom.com> <87o9jq66f3.fsf@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-wireless-owner@vger.kernel.org List-ID: On Wed, 14 Mar 2018 17:08:48 +0200 Kalle Valo wrote: > Arend van Spriel writes: > > > On 3/14/2018 3:24 PM, Kalle Valo wrote: > >>> +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. > >>> >+ > >>> >+ It's important to understand this behavior can lead to a local > >>> >+ DoS security issue. Attacker may trigger disassociation of any > >>> >+ STA by sending a proper Ethernet frame to the wireless > >>> >+ interface. > >>> >+ > >>> >+ Moreover this feature may break AP interfaces in some specific > >>> >+ setups. This applies e.g. to the bridge with hairpin mode > >>> >+ enabled and IFLA_BRPORT_MCAST_TO_UCAST set. IAPP packet > >>> >+ generated by a firmware will get passed back to the wireless > >>> >+ interface and cause immediate disassociation of just-connected > >>> >+ STA. > >> Sorry for jumping late, but does it really make sense to have a Kconfig > >> option for this? I don't think we should add a Kconfig option for every > >> strange feature, there should be stronger reasons (size savings etc) > >> before adding a Kconfig option. > >> > >> And in this case the size savings can't be much. Wouldn't a module > >> parameter be simpler for a functionality change like this? > > > > Hi Kalle, > > > > Good to be wary about Kconfig option. > > I think Linus doesn't like pointless Kconfig options, me neither for > that matter, so I try to make sure the justifications are really there > before adding anything new. > > > So my reason for asking a Kconfig option is that this is directly in > > the datapaths (tx and rx) so I prefer to disable/enable it compile > > time rather then runtime. > > I'm no cpu profile expert but is really one (or two?) if checks of a > cached variable in the datapath really measurable? My guess is that it's > just noise in the results. > > But I'm not going to argue about it, if you think it's still needed I'm > fine with that. Just mention in the commit log the justification the new > Kconfig option. If you have to disable it a module parameter is not a complete disaster From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default Date: Wed, 14 Mar 2018 08:27:15 -0700 Message-ID: <20180314082715.13deee1f@xeon-e3> References: <20180314110119.13631-1-zajec5@gmail.com> <878tau7n23.fsf@codeaurora.org> <5AA93530.5040001@broadcom.com> <87o9jq66f3.fsf@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Arend van Spriel , =?UTF-8?B?UmFmYcWC?= =?UTF-8?B?IE1pxYJlY2tp?= , Franky Lin , Hante Meuleman , Chi-Hsien Lin , Wright Feng , Pieter-Paul Giesberts , James Hughes , linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w@public.gmane.org, brcm80211-dev-list-+wT8y+m8/X5BDgjK7y7TUQ@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linus =?UTF-8?B?TMO8c3Npbmc=?= , Felix Fietkau , bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= To: Kalle Valo Return-path: In-Reply-To: <87o9jq66f3.fsf-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Sender: linux-wireless-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: netdev.vger.kernel.org On Wed, 14 Mar 2018 17:08:48 +0200 Kalle Valo wrote: > Arend van Spriel writes: > > > On 3/14/2018 3:24 PM, Kalle Valo wrote: > >>> +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. > >>> >+ > >>> >+ It's important to understand this behavior can lead to a local > >>> >+ DoS security issue. Attacker may trigger disassociation of any > >>> >+ STA by sending a proper Ethernet frame to the wireless > >>> >+ interface. > >>> >+ > >>> >+ Moreover this feature may break AP interfaces in some specific > >>> >+ setups. This applies e.g. to the bridge with hairpin mode > >>> >+ enabled and IFLA_BRPORT_MCAST_TO_UCAST set. IAPP packet > >>> >+ generated by a firmware will get passed back to the wireless > >>> >+ interface and cause immediate disassociation of just-connected > >>> >+ STA. > >> Sorry for jumping late, but does it really make sense to have a Kconfig > >> option for this? I don't think we should add a Kconfig option for every > >> strange feature, there should be stronger reasons (size savings etc) > >> before adding a Kconfig option. > >> > >> And in this case the size savings can't be much. Wouldn't a module > >> parameter be simpler for a functionality change like this? > > > > Hi Kalle, > > > > Good to be wary about Kconfig option. > > I think Linus doesn't like pointless Kconfig options, me neither for > that matter, so I try to make sure the justifications are really there > before adding anything new. > > > So my reason for asking a Kconfig option is that this is directly in > > the datapaths (tx and rx) so I prefer to disable/enable it compile > > time rather then runtime. > > I'm no cpu profile expert but is really one (or two?) if checks of a > cached variable in the datapath really measurable? My guess is that it's > just noise in the results. > > But I'm not going to argue about it, if you think it's still needed I'm > fine with that. Just mention in the commit log the justification the new > Kconfig option. If you have to disable it a module parameter is not a complete disaster