[PATCH net,stable 0/3] qmi_wwan: working around 3 serious firmware bugs

[PATCH net,stable 0/3] qmi_wwan: working around 3 serious firmware bugs

[Bjørn Mork]

This series adds workarounds for 3 different firmware bugs, each
preventing the affected devices from working at all. I therefore
humbly request that these fixes go to stable-3.8 (if still
maintained) and 3.9 (either via net if still possible, or via
stable if not).

All 3 workarounds are applied to all devices supported by the driver.
Adding quirks for specific devices was considered as an alternative,
but was rejected because we have too little information about the
exact distribution of the buggy firmwares. All we know is that the
same bug shows up in devices from at least 3 different, and presumably
independent, vendors.

The workarounds have instead been designed to automatically apply
when necessary, and to have as little impact as possible on unaffected
devices.  The series has been tested on a number of devices both with
and without these bugs.

The series should apply cleanly to net/master, net-next/master and
stable/linux-3.8.y


Thanks,
Bjørn


Bjørn Mork (3):
  net: qmi_wwan: fixup missing ethernet header (firmware bug
    workaround)
  net: qmi_wwan: fixup destination address (firmware bug workaround)
  net: qmi_wwan: prevent duplicate mac address on link (firmware bug
    workaround)

 drivers/net/usb/qmi_wwan.c |  104 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

-- 
1.7.10.4

[PATCH net,stable 1/3] net: qmi_wwan: fixup missing ethernet header (firmware bug workaround)

[Bjørn Mork]

A number of LTE devices from different vendors all suffer from the
same firmware bug: Most of the packets received from the device while
it is attached to a LTE network will not have an ethernet header. The
devices work as expected when attached to 2G or 3G networks, sending
an ethernet header with all packets.

This driver is not aware of which network the modem attached to, and
even if it were there are still some packet types which are always
received with the header intact.

All devices supported by this driver have severely limited
networking capabilities:
 - can only transmit IPv4, IPv6 and possibly ARP
 - can only support a single host hardware address at any time
 - will only do point-to-point communcation with the host

Because of this, we are able to reliably identify any bogus raw IP
packets by simply looking at the 4 IP version bits.  All we need to
do is to avoid 4 or 6 in the first digit of the mac address.  This
workaround ensures this, and fix up the received packets as necessary.

Given the distribution of the bug, it is believed that the source is
the chipset vendor.  The devices which are verified to be affected are:
 Huawei E392u-12 (Qualcomm MDM9200)
 Pantech UML290  (Qualcomm MDM9600)
 Novatel USB551L (Qualcomm MDM9600)
 Novatel E362    (Qualcomm MDM9600)

It is believed that the bug depend on firmware revision, which means
that possibly all devices based on the above mentioned chipset may be
affected if we consider all available firmware revisions.

The information about affected devices and versions is likely
incomplete.  As the additional overhead for packets not needing this
fixup is very small, it is considered acceptable to apply the
workaround to all devices handled by this driver.

Reported-by: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/qmi_wwan.c |   84 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 968d5d5..d8a50c7 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/ethtool.h>
+#include <linux/etherdevice.h>
 #include <linux/mii.h>
 #include <linux/usb.h>
 #include <linux/usb/cdc.h>
@@ -52,6 +53,82 @@ struct qmi_wwan_state {
 	struct usb_interface *data;
 };
 
+/* Make up an ethernet header if the packet doesn't have 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
+ * 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.
+ *
+ * This means that this function will reliably add the appropriate
+ * header iff necessary, provided our hardware address does not start
+ * with 4 or 6.
+ */
+static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+	__be16 proto;
+
+	/* usbnet rx_complete guarantees that skb->len is at least
+	 * hard_header_len, so we can inspect the dest address without
+	 * checking skb->len
+	 */
+	switch (skb->data[0] & 0xf0) {
+	case 0x40:
+		proto = htons(ETH_P_IP);
+		break;
+	case 0x60:
+		proto = htons(ETH_P_IPV6);
+		break;
+	default:
+		/* pass along other packets without modifications */
+		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;
+	memset(eth_hdr(skb)->h_source, 0, ETH_ALEN);
+	memcpy(eth_hdr(skb)->h_dest, dev->net->dev_addr, ETH_ALEN);
+	return 1;
+}
+
+/* very simplistic detection of IPv4 or IPv6 headers */
+static bool possibly_iphdr(const char *data)
+{
+	return (data[0] & 0xd0) == 0x40;
+}
+
+/* disallow addresses which may be confused with IP headers */
+static int qmi_wwan_mac_addr(struct net_device *dev, void *p)
+{
+	int ret;
+	struct sockaddr *addr = p;
+
+	ret = eth_prepare_mac_addr_change(dev, p);
+	if (ret < 0)
+		return ret;
+	if (possibly_iphdr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+	eth_commit_mac_addr_change(dev, p);
+	return 0;
+}
+
+static const struct net_device_ops qmi_wwan_netdev_ops = {
+	.ndo_open		= usbnet_open,
+	.ndo_stop		= usbnet_stop,
+	.ndo_start_xmit		= usbnet_start_xmit,
+	.ndo_tx_timeout		= usbnet_tx_timeout,
+	.ndo_change_mtu		= usbnet_change_mtu,
+	.ndo_set_mac_address	= qmi_wwan_mac_addr,
+	.ndo_validate_addr	= eth_validate_addr,
+};
+
 /* using a counter to merge subdriver requests with our own into a combined state */
 static int qmi_wwan_manage_power(struct usbnet *dev, int on)
 {
@@ -229,6 +306,12 @@ next_desc:
 		usb_driver_release_interface(driver, info->data);
 	}
 
+	/* make MAC addr easily distinguishable from an IP header */
+	if (possibly_iphdr(dev->net->dev_addr)) {
+		dev->net->dev_addr[0] |= 0x02;	/* set local assignment bit */
+		dev->net->dev_addr[0] &= 0xbf;	/* clear "IP" bit */
+	}
+	dev->net->netdev_ops = &qmi_wwan_netdev_ops;
 err:
 	return status;
 }
@@ -307,6 +390,7 @@ static const struct driver_info	qmi_wwan_info = {
 	.bind		= qmi_wwan_bind,
 	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
+	.rx_fixup       = qmi_wwan_rx_fixup,
 };
 
 #define HUAWEI_VENDOR_ID	0x12D1
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net,stable 2/3] net: qmi_wwan: fixup destination address (firmware bug workaround)

[Bjørn Mork]

Received packets are sometimes addressed to 00:a0:c6:00:00:00
instead of the address the device firmware should have learned
from the host:

321.224126 77.16.85.204 -> 148.122.171.134 ICMP 98 Echo (ping) request  id=0x4025, seq=64/16384, ttl=64

0000  82 c0 82 c9 f1 67 82 c0 82 c9 f1 67 08 00 45 00   .....g.....g..E.
0010  00 54 00 00 40 00 40 01 57 cc 4d 10 55 cc 94 7a   .T..@.@.W.M.U..z
0020  ab 86 08 00 62 fc 40 25 00 40 b2 bc 6e 51 00 00   ....b.@%.@..nQ..
0030  00 00 6b bd 09 00 00 00 00 00 10 11 12 13 14 15   ..k.............
0040  16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25   .......... !"#$%
0050  26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35   &'()*+,-./012345
0060  36 37                                             67

321.240607 148.122.171.134 -> 77.16.85.204 ICMP 98 Echo (ping) reply    id=0x4025, seq=64/16384, ttl=55

0000  00 a0 c6 00 00 00 02 50 f3 00 00 00 08 00 45 00   .......P......E.
0010  00 54 00 56 00 00 37 01 a0 76 94 7a ab 86 4d 10   .T.V..7..v.z..M.
0020  55 cc 00 00 6a fc 40 25 00 40 b2 bc 6e 51 00 00   U...j.@%.@..nQ..
0030  00 00 6b bd 09 00 00 00 00 00 10 11 12 13 14 15   ..k.............
0040  16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25   .......... !"#$%
0050  26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35   &'()*+,-./012345
0060  36 37                                             67

The bogus address is always the same, and matches the address
suggested by many devices as a default address.  It is likely a
hardcoded firmware default.

The circumstances where this bug has been observed indicates that
the trigger is related to timing or some other factor the host
cannot control. Repeating the exact same configuration sequence
that caused it to trigger once, will not necessarily cause it to
trigger the next time. Reproducing the bug is therefore difficult.
This opens up a possibility that the bug is more common than we can
confirm, because affected devices often will work properly again
after a reset.  A procedure most users are likely to try out before
reporting a bug.

Unconditionally rewriting the destination address if the first digit
of the received packet is 0, is considered an acceptable compromise
since we already have to inspect this digit.  The simplification will
cause unnecessary rewrites if the real address starts with 0, but this
is still better than adding additional tests for this particular case.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index d8a50c7..cff0bbd 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -68,6 +68,10 @@ struct qmi_wwan_state {
  * This means that this function will reliably add the appropriate
  * header iff 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.
  */
 static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
@@ -84,6 +88,12 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	case 0x60:
 		proto = htons(ETH_P_IPV6);
 		break;
+	case 0x00:
+		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:
 		/* pass along other packets without modifications */
 		return 1;
@@ -94,6 +104,7 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	skb_reset_mac_header(skb);
 	eth_hdr(skb)->h_proto = proto;
 	memset(eth_hdr(skb)->h_source, 0, ETH_ALEN);
+fix_dest:
 	memcpy(eth_hdr(skb)->h_dest, dev->net->dev_addr, ETH_ALEN);
 	return 1;
 }
-- 
1.7.10.4

[PATCH net,stable 3/3] net: qmi_wwan: prevent duplicate mac address on link (firmware bug workaround)

[Bjørn Mork]

We normally trust and use the CDC functional descriptors provided by a
number of devices.  But some of these will erroneously list the address
reserved for the device end of the link.  Attempting to use this on
both the device and host side will naturally not work.

Work around this bug by ignoring the functional descriptor and assign a
random address instead in this case.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/qmi_wwan.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index cff0bbd..2a3579f 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -53,6 +53,9 @@ struct qmi_wwan_state {
 	struct usb_interface *data;
 };
 
+/* default ethernet address used by the modem */
+static const u8 default_modem_addr[ETH_ALEN] = {0x02, 0x50, 0xf3};
+
 /* Make up an ethernet header if the packet doesn't have one.
  *
  * A firmware bug common among several devices cause them to send raw
@@ -317,6 +320,12 @@ next_desc:
 		usb_driver_release_interface(driver, info->data);
 	}
 
+	/* Never use the same address on both ends of the link, even
+	 * if the buggy firmware told us to.
+	 */
+	if (!compare_ether_addr(dev->net->dev_addr, default_modem_addr))
+		eth_hw_addr_random(dev->net);
+
 	/* make MAC addr easily distinguishable from an IP header */
 	if (possibly_iphdr(dev->net->dev_addr)) {
 		dev->net->dev_addr[0] |= 0x02;	/* set local assignment bit */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net,stable 0/3] qmi_wwan: working around 3 serious firmware bugs

[David Miller]

From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Fri, 19 Apr 2013 00:57:08 +0200

> This series adds workarounds for 3 different firmware bugs, each
> preventing the affected devices from working at all. I therefore
> humbly request that these fixes go to stable-3.8 (if still
> maintained) and 3.9 (either via net if still possible, or via
> stable if not).
> 
> All 3 workarounds are applied to all devices supported by the driver.
> Adding quirks for specific devices was considered as an alternative,
> but was rejected because we have too little information about the
> exact distribution of the buggy firmwares. All we know is that the
> same bug shows up in devices from at least 3 different, and presumably
> independent, vendors.
> 
> The workarounds have instead been designed to automatically apply
> when necessary, and to have as little impact as possible on unaffected
> devices.  The series has been tested on a number of devices both with
> and without these bugs.
> 
> The series should apply cleanly to net/master, net-next/master and
> stable/linux-3.8.y

Series applied and queued up for -stable, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net,stable 0/3] qmi_wwan: working around 3 serious firmware bugs

[Dan Williams]

On Fri, 2013-04-19 at 17:51 -0400, David Miller wrote:
> From: Bjørn Mork <bjorn@mork.no>
> Date: Fri, 19 Apr 2013 00:57:08 +0200
> 
> > This series adds workarounds for 3 different firmware bugs, each
> > preventing the affected devices from working at all. I therefore
> > humbly request that these fixes go to stable-3.8 (if still
> > maintained) and 3.9 (either via net if still possible, or via
> > stable if not).
> > 
> > All 3 workarounds are applied to all devices supported by the driver.
> > Adding quirks for specific devices was considered as an alternative,
> > but was rejected because we have too little information about the
> > exact distribution of the buggy firmwares. All we know is that the
> > same bug shows up in devices from at least 3 different, and presumably
> > independent, vendors.
> > 
> > The workarounds have instead been designed to automatically apply
> > when necessary, and to have as little impact as possible on unaffected
> > devices.  The series has been tested on a number of devices both with
> > and without these bugs.
> > 
> > The series should apply cleanly to net/master, net-next/master and
> > stable/linux-3.8.y
> 
> Series applied and queued up for -stable, thanks!

You're fast :)  But in any case:

Tested-by: Dan Williams <dcbw@redhat.com>

Tested on Gobi 1K (known not to exhibit the raw-ip bug), Novatel E362,
and Pantech UML290.

Dan