Return-path: Received: from 2.mo68.mail-out.ovh.net ([46.105.52.162]:45194 "EHLO 2.mo68.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751581AbeAaNxY (ORCPT ); Wed, 31 Jan 2018 08:53:24 -0500 Received: from player763.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo68.mail-out.ovh.net (Postfix) with ESMTP id 44495BE90E for ; Wed, 31 Jan 2018 14:15:07 +0100 (CET) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Date: Wed, 31 Jan 2018 14:14:55 +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 , linux-wireless@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com, brcm80211-dev-list@cypress.com Subject: Re: [PATCH] brcmfmac: detect & reject faked packet generated by a firmware In-Reply-To: <5A705B5E.5070906@broadcom.com> References: <20180130090922.30346-1-zajec5@gmail.com> <5A705B5E.5070906@broadcom.com> Message-ID: (sfid-20180131_145329_845074_6607FD5E) Sender: linux-wireless-owner@vger.kernel.org List-ID: On 2018-01-30 12:47, Arend van Spriel wrote: > On 1/30/2018 10:09 AM, Rafał Miłecki wrote: >> From: Rafał Miłecki >> >> When using 4366b1 and 4366c0 chipsets with more recent firmwares >> 1) 10.10 (TOB) (r663589) >> 2) 10.10.122.20 (r683106) >> respectively, it is impossible to use brcmfmac with interface in AP >> mode. With the AP interface bridged and multicast used, no STA will be >> able to associate; the STA will be immediately disassociated when >> attempting to associate. >> >> Debugging revealed this to be caused by a "faked" packet (generated by >> firmware), that is passed to the networking subsystem and then back to >> the firmware. Fortunately this packet is easily identified and can be >> detected and ignored as a workaround for misbehaving firmware. > > I am actually wondering what this packet is. Have you checked in > brcmf_msgbuf_process_rx_complete(). I am curious what buflen is there > and what eth_type_trans() will do to the packet, ie. what protocol. As > everything should be 802.3 we could/should add a length check of 14 > bytes. Did you find anything? I got some debugging info, hopefully this is what you expected [ 144.356648] brcmfmac: [brcmf_msgbuf_process_rx_complete] msg.msgtype: 0x12 [ 144.363559] brcmfmac: [brcmf_msgbuf_process_rx_complete] msg.ifidx: 0x00 [ 144.370374] brcmfmac: [brcmf_msgbuf_process_rx_complete] msg.flags: 0x80 [ 144.377179] brcmfmac: [brcmf_msgbuf_process_rx_complete] msg.rsvd0: 0x00 [ 144.383986] brcmfmac: [brcmf_msgbuf_process_rx_complete] msg.request_id: 0x00000041 [ 144.391661] brcmfmac: [brcmf_msgbuf_process_rx_complete] compl_hdr.status: 0x0000 [ 144.399156] brcmfmac: [brcmf_msgbuf_process_rx_complete] compl_hdr.flow_ring_id: 0x0000 [ 144.407179] brcmfmac: [brcmf_msgbuf_process_rx_complete] metadata_len: 0x0000 [ 144.414334] brcmfmac: [brcmf_msgbuf_process_rx_complete] data_len: 0x0014 [ 144.421227] brcmfmac: [brcmf_msgbuf_process_rx_complete] data_offset: 0x0000 [ 144.428288] brcmfmac: [brcmf_msgbuf_process_rx_complete] flags: 0x0001 [ 144.434918] brcmfmac: [brcmf_msgbuf_process_rx_complete] rx_status_0: 0x00000000 [ 144.442334] brcmfmac: [brcmf_msgbuf_process_rx_complete] rx_status_1: 0x00000000 [ 144.449750] brcmfmac: [brcmf_msgbuf_process_rx_complete] rsvd0: 0x00000001 [ 144.456724] brcmfmac: [brcmf_msgbuf_process_rx_complete] skb->data: ff ff ff ff ff ff ec 10 7b 5f ?? ?? 00 06 00 01 af 81 01 00 [ 144.467883] brcmfmac: [brcmf_msgbuf_process_rx_complete] skb->protocol: 0x0400 (just masked 2 bytes of my MAC) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index 1bd4b96..08cdcaf 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -1172,7 +1172,43 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) return; } + if (skb->len == ETH_HLEN + 6) { + uint8_t *data; + int i; + + pr_info("[%s] msg.msgtype:\t0x%02x\n", __func__, rx_complete->msg.msgtype); + pr_info("[%s] msg.ifidx:\t\t0x%02x\n", __func__, rx_complete->msg.ifidx); + pr_info("[%s] msg.flags:\t\t0x%02x\n", __func__, rx_complete->msg.flags); + pr_info("[%s] msg.rsvd0:\t\t0x%02x\n", __func__, rx_complete->msg.rsvd0); + pr_info("[%s] msg.request_id:\t0x%08x\n", __func__, le32_to_cpu(rx_complete->msg.request_id)); + + pr_info("[%s] compl_hdr.status:\t0x%04x\n", __func__, le16_to_cpu(rx_complete->compl_hdr.status)); + pr_info("[%s] compl_hdr.flow_ring_id:\t0x%04x\n", __func__, le16_to_cpu(rx_complete->compl_hdr.flow_ring_id)); + + pr_info("[%s] metadata_len:\t0x%04x\n", __func__, le16_to_cpu(rx_complete->metadata_len)); + pr_info("[%s] data_len:\t\t0x%04x\n", __func__, le16_to_cpu(rx_complete->data_len)); + pr_info("[%s] data_offset:\t0x%04x\n", __func__, le16_to_cpu(rx_complete->data_offset)); + pr_info("[%s] flags:\t\t0x%04x\n", __func__, le16_to_cpu(rx_complete->flags)); + pr_info("[%s] rx_status_0:\t0x%08x\n", __func__, le32_to_cpu(rx_complete->rx_status_0)); + pr_info("[%s] rx_status_1:\t0x%08x\n", __func__, le32_to_cpu(rx_complete->rx_status_1)); + pr_info("[%s] rsvd0:\t\t0x%08x\n", __func__, le32_to_cpu(rx_complete->rsvd0)); + + data = skb->data; + pr_info("[%s] skb->data:\t\t", __func__); + for (i = 0; i < 32 && i < skb->len; i++) { + pr_cont("%02x ", data[i]); + if (i % 4 == 3) + pr_cont(" "); + } + pr_cont("\n"); + } + skb->protocol = eth_type_trans(skb, ifp->ndev); + + if (skb->len == 6) { + pr_info("[%s] skb->protocol:\t0x%04x\n", __func__, skb->protocol); + } + brcmf_netif_rx(ifp, skb); }