On Wed, 14 Mar 2018 17:08:48 +0200 Kalle Valo <kvalo@xxxxxxxxxxxxxx> wrote: > Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx> 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