linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <zajec5@gmail.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: "Arend van Spriel" <arend.vanspriel@broadcom.com>,
	"Franky Lin" <franky.lin@broadcom.com>,
	"Hante Meuleman" <hante.meuleman@broadcom.com>,
	"Chi-Hsien Lin" <chi-hsien.lin@cypress.com>,
	"Wright Feng" <wright.feng@cypress.com>,
	"Pieter-Paul Giesberts" <pieter-paul.giesberts@broadcom.com>,
	"James Hughes" <james.hughes@raspberrypi.org>,
	linux-wireless@vger.kernel.org,
	brcm80211-dev-list.pdl@broadcom.com,
	brcm80211-dev-list@cypress.com, netdev@vger.kernel.org,
	"Linus Lüssing" <linus.luessing@c0d3.blue>,
	"Felix Fietkau" <nbd@nbd.name>,
	bridge@lists.linux-foundation.org,
	"Rafał Miłecki" <rafal@milecki.pl>
Subject: [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default
Date: Wed, 14 Mar 2018 12:01:19 +0100	[thread overview]
Message-ID: <20180314110119.13631-1-zajec5@gmail.com> (raw)

From: Rafał Miłecki <rafal@milecki.pl>

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.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 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.
+
+	  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.
+
 config BRCM_TRACING
 	bool "Broadcom device tracing"
 	depends on BRCMSMAC || BRCMFMAC
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 19048526b4af..db6987015fb1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -230,6 +230,34 @@ static void brcmf_netdev_set_multicast_list(struct net_device *ndev)
 	schedule_work(&ifp->multicast_work);
 }
 
+/**
+ * brcmf_skb_is_iapp - checks if skb is an IAPP packet
+ *
+ * @skb: skb to check
+ */
+static bool brcmf_skb_is_iapp(struct sk_buff *skb)
+{
+	const u8 iapp_l2_update_packet[6] __aligned(2) = {
+		0x00, 0x01, 0xaf, 0x81, 0x01, 0x00,
+	};
+	unsigned char *eth_data = skb_mac_header(skb) + ETH_HLEN;
+#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+	const u16 *a = (const u16 *)eth_data;
+	const u16 *b = (const u16 *)iapp_l2_update_packet;
+#endif
+
+	if (skb->len - skb->mac_len != 6 ||
+	    !is_multicast_ether_addr(eth_hdr(skb)->h_dest))
+		return false;
+
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+	return !(((*(const u32 *)eth_data) ^ (*(const u32 *)iapp_l2_update_packet)) |
+		 ((*(const u16 *)(eth_data + 4)) ^ (*(const u16 *)(iapp_l2_update_packet + 4))));
+#else
+	return !((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2]));
+#endif
+}
+
 static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 					   struct net_device *ndev)
 {
@@ -250,6 +278,12 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 		goto done;
 	}
 
+	if (!IS_ENABLED(CONFIG_BRCMFMAC_IAPP) && brcmf_skb_is_iapp(skb)) {
+		dev_kfree_skb(skb);
+		ret = -EINVAL;
+		goto done;
+	}
+
 	/* Make sure there's enough writeable headroom */
 	if (skb_headroom(skb) < drvr->hdrlen || skb_header_cloned(skb)) {
 		head_delta = max_t(int, drvr->hdrlen - skb_headroom(skb), 0);
@@ -325,6 +359,11 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
 
 void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
 {
+	if (!IS_ENABLED(CONFIG_BRCMFMAC_IAPP) && brcmf_skb_is_iapp(skb)) {
+		brcmu_pkt_buf_free_skb(skb);
+		return;
+	}
+
 	if (skb->pkt_type == PACKET_MULTICAST)
 		ifp->ndev->stats.multicast++;
 
-- 
2.11.0

             reply	other threads:[~2018-03-14 11:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-14 11:01 Rafał Miłecki [this message]
2018-03-14 12:58 ` [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default Arend van Spriel
2018-03-14 15:39   ` Rafał Miłecki
2018-03-14 15:40     ` Rafał Miłecki
2018-03-14 15:57     ` Rafał Miłecki
2018-03-14 20:44       ` Arend van Spriel
2018-03-14 14:24 ` Kalle Valo
2018-03-14 14:44   ` Arend van Spriel
2018-03-14 15:08     ` Kalle Valo
2018-03-14 15:27       ` Stephen Hemminger
2018-03-14 15:28       ` Rafał Miłecki
2018-03-14 15:44   ` Rafał Miłecki
2018-03-14 16:10     ` Kalle Valo
2018-03-14 20:55       ` Arend van Spriel
2018-03-15  9:23         ` Kalle Valo
2018-03-14 15:08 ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180314110119.13631-1-zajec5@gmail.com \
    --to=zajec5@gmail.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=brcm80211-dev-list@cypress.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=chi-hsien.lin@cypress.com \
    --cc=franky.lin@broadcom.com \
    --cc=hante.meuleman@broadcom.com \
    --cc=james.hughes@raspberrypi.org \
    --cc=kvalo@codeaurora.org \
    --cc=linus.luessing@c0d3.blue \
    --cc=linux-wireless@vger.kernel.org \
    --cc=nbd@nbd.name \
    --cc=netdev@vger.kernel.org \
    --cc=pieter-paul.giesberts@broadcom.com \
    --cc=rafal@milecki.pl \
    --cc=wright.feng@cypress.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).