Skip to contentSkip to conversation

brcmfmac: drop Inter-Access Point Protocol packets by default

PATCH In review Rafał Miłecki 4 participants 5 messages Mar 15, 2018 – Mar 16, 2018 1 message not in the archive

Subsystems (4)
Networking Network drivers Ethernet bridge Broadcom BRCM80211 IEEE802.11 wireless drivers
lore
Share & tools
lore

Rafał Miłecki

author
#0 lore raw

PATCH v2 1/1 brcmfmac: drop Inter-Access Point Protocol packets by default

From: Rafał Miłecki <[email protected]>

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.

Older firmwares were only generating 802.11f frames. Newer ones like:
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
for 4366b1 and 4366c0 respectively seem to also /respect/ 802.11f frames in the Tx path by performing a STA disassociation.

This 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 using a new module param.

This change results in only two more comparisons (check for module param and check for Ethernet packet length) for 99.9% of packets. Its overhead should be very minimal.

Signed-off-by Rafał Miłecki <[email protected]>
---
 .../wireless/broadcom/brcm80211/brcmfmac/common.c  |  5 ++
 .../wireless/broadcom/brcm80211/brcmfmac/common.h  |  1 +
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 57 ++++++++++++++++++++++
 3 files changed, 63 insertions(+)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +5 -0
index 70ef9835b647..5532ef39439f 100644
@@ -75,6 +75,10 @@ static int brcmf_roamoff;
 module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
 MODULE_PARM_DESC(roamoff, "Do not use internal roaming engine");
 
+static int brcmf_iapp_enable;
+module_param_named(iapp, brcmf_iapp_enable, int, 0);
+MODULE_PARM_DESC(iapp, "Enable partial support for the obsoleted Inter-Access Point Protocol");
+
 #ifdef DEBUG
 /* always succeed brcmf_bus_started() */
 static int brcmf_ignore_probe_fail;

@@ -438,6 +442,7 @@ struct brcmf_mp_device *brcmf_get_module_param(struct device *dev,
 	settings->feature_disable = brcmf_feature_disable;
 	settings->fcmode = brcmf_fcmode;
 	settings->roamoff = !!brcmf_roamoff;
+	settings->iapp = !!brcmf_iapp_enable;
 #ifdef DEBUG
 	settings->ignore_probe_fail = !!brcmf_ignore_probe_fail;
 #endif
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h +1 -0
index a62f8e70b320..ef914619e8e1 100644
@@ -58,6 +58,7 @@ struct brcmf_mp_device {
 	unsigned int	feature_disable;
 	int		fcmode;
 	bool		roamoff;
+	bool		iapp;
 	bool		ignore_probe_fail;
 	struct brcmfmac_pd_cc *country_codes;
 	union {
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +57 -0

75 lines not on this page · raw message

Signature · 1 line
2.11.0

A message that is not in the archive (sent off-list or not yet received): 20180315072909.1512-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

author
#2 lore raw
Quoted 27 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

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.

Older firmwares were only generating 802.11f frames. Newer ones like:
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
for 4366b1 and 4366c0 respectively seem to also /respect/ 802.11f frames in the Tx path by performing a STA disassociation.

This 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 using a new module param.

Quoted 6 lines

This change results in only two more comparisons (check for module param and check for Ethernet packet length) for 99.9% of packets. Its overhead should be very minimal.

Signed-off-by Rafał Miłecki <[email protected]>

---

I forgot to include the changelog, sorry.

V2: Use module param to don't /abuse/ Kconfig
Slightly optimize brcmf_skb_is_iapp
Move some description from Kconfig to the code
Update commit description: specify affected fws & mention impact

Acked-by
#3 lore raw
In reply to a message not in the archive
Quoted 23 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

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.

Older firmwares were only generating 802.11f frames. Newer ones like:
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
for 4366b1 and 4366c0 respectively seem to also /respect/ 802.11f frames in the Tx path by performing a STA disassociation.

This 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).

Quoted 7 lines

It appears there are some Broadcom's clients/users who care about this feature despite the drawbacks. They can switch it on using a new module param.

This change results in only two more comparisons (check for module param and check for Ethernet packet length) for 99.9% of packets. Its overhead should be very minimal.

Hi Rafał,

Thanks for this patch.

Acked-by Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/[email protected]>
Quoted 6 lines
Signed-off-by Rafał Miłecki <[email protected]>
---
  .../wireless/broadcom/brcm80211/brcmfmac/common.c  |  5 ++
  .../wireless/broadcom/brcm80211/brcmfmac/common.h  |  1 +
  .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 57 ++++++++++++++++++++++
  3 files changed, 63 insertions(+)
#4 lore raw
In reply to a message not in the archive
Quoted 27 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

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.

Older firmwares were only generating 802.11f frames. Newer ones like:
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
for 4366b1 and 4366c0 respectively seem to also /respect/ 802.11f frames in the Tx path by performing a STA disassociation.

This 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 using a new module param.

Quoted 6 lines

This change results in only two more comparisons (check for module param and check for Ethernet packet length) for 99.9% of packets. Its overhead should be very minimal.

Signed-off-by Rafał Miłecki <[email protected]>
Acked-by Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/[email protected]>

Patch applied to wireless-drivers.git, thanks.

125905517028 brcmfmac: drop Inter-Access Point Protocol packets by default

Signature · 3 lines
https://patchwork.kernel.org/patch/10283971/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
#5 lore raw
In reply to Rafał Miłecki
Quoted 27 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

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.

Older firmwares were only generating 802.11f frames. Newer ones like:
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
for 4366b1 and 4366c0 respectively seem to also /respect/ 802.11f frames in the Tx path by performing a STA disassociation.

This 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 using a new module param.

Quoted 6 lines

This change results in only two more comparisons (check for module param and check for Ethernet packet length) for 99.9% of packets. Its overhead should be very minimal.

Signed-off-by Rafał Miłecki <[email protected]>
Acked-by Arend van Spriel <[email protected]>

Patch applied to wireless-drivers.git, thanks.

125905517028 brcmfmac: drop Inter-Access Point Protocol packets by default

Signature · 3 lines
https://patchwork.kernel.org/patch/10283971/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches