Return-path: Received: from mail-lf0-f45.google.com ([209.85.215.45]:38469 "EHLO mail-lf0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751001AbeAaQOl (ORCPT ); Wed, 31 Jan 2018 11:14:41 -0500 Received: by mail-lf0-f45.google.com with SMTP id g72so21531216lfg.5 for ; Wed, 31 Jan 2018 08:14:40 -0800 (PST) From: Hante Meuleman References: <20180130090922.30346-1-zajec5@gmail.com> <5A705B5E.5070906@broadcom.com> <5A71D08B.7090905@broadcom.com> In-Reply-To: <5A71D08B.7090905@broadcom.com> MIME-Version: 1.0 Date: Wed, 31 Jan 2018 17:14:38 +0100 Message-ID: <4f6223b8083ed69432493a37d4f45b69@mail.gmail.com> (sfid-20180131_171447_098883_B22E1B4E) Subject: RE: [PATCH] brcmfmac: detect & reject faked packet generated by a firmware To: Arend Van Spriel , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Cc: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Kalle Valo , Franky Lin , Chi-Hsien Lin , Wright Feng , Pieter-Paul Giesberts , linux-wireless@vger.kernel.org, "BRCM80211-DEV-LIST,PDL" , brcm80211-dev-list@cypress.com Content-Type: text/plain; charset="UTF-8" Sender: linux-wireless-owner@vger.kernel.org List-ID: It is an 802.2 frame, more specifically a LLC XID frames. So why it exists? And more over, why would we crash as an result? Decoding info can be found here: https://www.cisco.com/c/en/us/support/docs/ibm-technologies/logical-link-co= ntrol-llc/12247-45.html#con3 The frame was likely sent by the stack from remote site PC, should be possible to capture with tcpdump. I've seen these frames before, but don=E2=80=99t know what they are for. Th= e frame appears to be correctly encoded. The ethertype, is not a type, but a len field. The only protocol with such a short len allowed is llc, see also https://www.savvius.com/networking-glossary/ethernet/frame_formats/ So it is 802.2 (also known as LLC) Regads, Hante -----Original Message----- From: Arend van Spriel [mailto:arend.vanspriel@broadcom.com] Sent: Wednesday, January 31, 2018 3:20 PM To: Rafa=C5=82 Mi=C5=82ecki Cc: Rafa=C5=82 Mi=C5=82ecki; Kalle Valo; Franky Lin; Hante Meuleman; Chi-Hs= ien 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 On 1/31/2018 2:14 PM, Rafa=C5=82 Mi=C5=82ecki wrote: > On 2018-01-30 12:47, Arend van Spriel wrote: >> On 1/30/2018 10:09 AM, Rafa=C5=82 Mi=C5=82ecki wrote: >>> From: Rafa=C5=82 Mi=C5=82ecki >>> >>> 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 was going to say no, but below I see I misinterpreted your commit message and thought we were getting 6 bytes from firmware, but it is 6 bytes + ETH_HLEN. > I got some debugging info, hopefully this is what you expected and more ... :-) > [ 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 Not sure what protocol that is. Can not find it in if_ether.h. Will look in our firmware repo for it. Thanks, Arend > (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 =3D=3D 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 =3D skb->data; > + pr_info("[%s] skb->data:\t\t", __func__); > + for (i =3D 0; i < 32 && i < skb->len; i++) { > + pr_cont("%02x ", data[i]); > + if (i % 4 =3D=3D 3) > + pr_cont(" "); > + } > + pr_cont("\n"); > + } > + > skb->protocol =3D eth_type_trans(skb, ifp->ndev); > + > + if (skb->len =3D=3D 6) { > + pr_info("[%s] skb->protocol:\t0x%04x\n", __func__, > skb->protocol); > + } > + > brcmf_netif_rx(ifp, skb); > } >