lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161114002218.GW2745@pokute.pelzi.net>
Date:   Mon, 14 Nov 2016 02:22:18 +0200
From:   Jussi Peltola <plz@....fi>
To:     Bjørn Mork <bjorn@...k.no>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] Fixup packets with incorrect ethertype sent by ZTE MF821D

So here's another stab. The comments and the current implementation are
not in sync: any non-multicast address starting with a null octet gets
rewritten, while the comment specifically mentions 00:a0:c6:00:00:00. It
is certainly not elegant but re-writing all unicast destinations with
our address does come to mind instead of special cases.

This patch fails to handle the invalid destinations in either way so I
will send another one if you think it's worthwhile to go on. And it
seems I forgot htons but I need this device for work now so a better
patch must wait :)

commit 35d3a46b7f1ece70e24386acbdd16af4507cb5f3
Author: Jussi Peltola <plz@....fi>
Date:   Mon Nov 14 01:45:32 2016 +0200

    Attempt to fix up packets with a broken ethernet header
    
    Signed-off-by: Jussi Peltola <plz@....fi>

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 3ff76c6..7308d6b 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -153,25 +153,57 @@ static const u8 default_modem_addr[ETH_ALEN] = {0x02, 0x50, 0xf3};
 
 static const u8 buggy_fw_addr[ETH_ALEN] = {0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00};
 
-/* Make up an ethernet header if the packet doesn't have one.
+/* Check if the ethernet header has an unknown ethertype, and return a
+ * guess of the correct one based on the L3 header, or zero if the type was
+ * known or detection failed.
+ */
+static __be16 detect_bogus_header(struct sk_buff *skb) {
+       struct ethhdr *eth_hdr = (struct ethhdr*) skb->data;
+
+       switch (eth_hdr->h_proto) {
+       case ETH_P_IP:
+       case ETH_P_IPV6:
+       case ETH_P_ARP:
+               return 0;
+       default:
+               switch (skb->data[14] & 0xf0) {
+               case 0x40:
+                       return htons(ETH_P_IP);
+               case 0x60:
+                       return htons(ETH_P_IPV6);
+               default:
+                       /* pass on undetectable packets */
+                       return 0;
+               }
+       }
+       /*NOTREACHED*/
+       return 0;
+}
+
+/* Make up an ethernet header if the packet doesn't have a correct one.
  *
  * A firmware bug common among several devices cause them to send raw
  * IP packets under some circumstances.  There is no way for the
  * driver/host to know when this will happen.  And even when the bug
  * hits, some packets will still arrive with an intact header.
  *
- * The supported devices are only capably of sending IPv4, IPv6 and
+ * The supported devices are only capable of sending IPv4, IPv6 and
  * ARP packets on a point-to-point link. Any packet with an ethernet
  * header will have either our address or a broadcast/multicast
- * address as destination.  ARP packets will always have a header.
+ * address as destination. ARP packets will always have a header.
  *
  * This means that this function will reliably add the appropriate
- * header iff necessary, provided our hardware address does not start
+ * header if necessary, provided our hardware address does not start
  * with 4 or 6.
  *
  * Another common firmware bug results in all packets being addressed
  * to 00:a0:c6:00:00:00 despite the host address being different.
- * This function will also fixup such packets.
+ *
+ * Some devices will send packets with garbage source/destination MACs and
+ * ethertypes.
+ *
+ * This function will try to fix up all such packets.
+ *
  */
 static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
@@ -179,8 +211,8 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
        bool rawip = info->flags & QMI_WWAN_FLAG_RAWIP;
        __be16 proto;
 
-       /* This check is no longer done by usbnet */
-       if (skb->len < dev->net->hard_header_len)
+       /* Shorter is definitely invalid and breaks subsequent tests */
+       if (skb->len < 15)
                return 0;
 
        switch (skb->data[0] & 0xf0) {
@@ -190,17 +222,17 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
        case 0x60:
                proto = htons(ETH_P_IPV6);
                break;
-       case 0x00:
+       default:
                if (rawip)
                        return 0;
                if (is_multicast_ether_addr(skb->data))
                        return 1;
-               /* possibly bogus destination - rewrite just in case */
-               skb_reset_mac_header(skb);
-               goto fix_dest;
-       default:
-               if (rawip)
-                       return 0;
+               proto = detect_bogus_header(skb);
+               if (proto) {
+                       /* remove terminally broken header */
+                       skb_pull(skb, ETH_HLEN);
+                       break;
+               }
                /* pass along other packets without modifications */
                return 1;
        }
@@ -208,17 +240,17 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
                skb->dev = dev->net; /* normally set by eth_type_trans */
                skb->protocol = proto;
                return 1;
+       } else {
+               /* push a made-up header */
+               if (skb_headroom(skb) < ETH_HLEN)
+                       return 0;
+               skb_push(skb, ETH_HLEN);
+               skb_reset_mac_header(skb);
+               eth_hdr(skb)->h_proto = proto;
+               eth_zero_addr(eth_hdr(skb)->h_source);
+               memcpy(eth_hdr(skb)->h_dest, dev->net->dev_addr, ETH_ALEN);
+               return 1;
        }
-
-       if (skb_headroom(skb) < ETH_HLEN)
-               return 0;
-       skb_push(skb, ETH_HLEN);
-       skb_reset_mac_header(skb);
-       eth_hdr(skb)->h_proto = proto;
-       eth_zero_addr(eth_hdr(skb)->h_source);
-fix_dest:
-       memcpy(eth_hdr(skb)->h_dest, dev->net->dev_addr, ETH_ALEN);
-       return 1;
 }
 
 /* very simplistic detection of IPv4 or IPv6 headers */

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ