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-control-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’t know what they are for. The 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@xxxxxxxxxxxx] Sent: Wednesday, January 31, 2018 3:20 PM To: Rafał Miłecki Cc: Rafał Miłecki; Kalle Valo; Franky Lin; Hante Meuleman; Chi-Hsien Lin; Wright Feng; Pieter-Paul Giesberts; linux-wireless@xxxxxxxxxxxxxxx; brcm80211-dev-list.pdl@xxxxxxxxxxxx; brcm80211-dev-list@xxxxxxxxxxx Subject: Re: [PATCH] brcmfmac: detect & reject faked packet generated by a firmware On 1/31/2018 2:14 PM, Rafał Miłecki wrote: > 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 <rafal@xxxxxxxxxx> >>> >>> 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 == 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); > } >
