usbnet: remove generic hard_header_len check

[ Upstream commit eb85569fe2d06c2fbf4de7b66c263ca095b397aa ]

This patch removes a generic hard_header_len check from the usbnet
module that is causing dropped packages under certain circumstances
for devices that send rx packets that cross urb boundaries.

One example is the AX88772B which occasionally send rx packets that
cross urb boundaries where the remaining partial packet is sent with
no hardware header. When the buffer with a partial packet is of less
number of octets than the value of hard_header_len the buffer is
discarded by the usbnet module.

With AX88772B this can be reproduced by using ping with a packet
size between 1965-1976.

The bug has been reported here:

https://bugzilla.kernel.org/show_bug.cgi?id=29082

This patch introduces the following changes:
- Removes the generic hard_header_len check in the rx_complete
  function in the usbnet module.
- Introduces a ETH_HLEN check for skbs that are not cloned from
  within a rx_fixup callback.
- For safety a hard_header_len check is added to each rx_fixup
  callback function that could be affected by this change.
  These extra checks could possibly be removed by someone
  who has the hardware to test.
- Removes a call to dev_kfree_skb_any() and instead utilizes the
  dev->done list to queue skbs for cleanup.

The changes place full responsibility on the rx_fixup callback
functions that clone skbs to only pass valid skbs to the
usbnet_skb_return function.

Signed-off-by: Emil Goode <emilgoode@gmail.com>
Reported-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index 38266bdae26b9ced95c79c19742b3458a55414f2..71e0b99386020deb33503e53b5b4cb6894647d7c 100644 (file)
--- a/drivers/net/usb/gl620a.c
+++ b/drivers/net/usb/gl620a.c
@@ -86,6 +86,10 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
        u32                     size;
        u32                     count;
 
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        header = (struct gl_header *) skb->data;
 
        // get the packet count of the received skb

diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index c434b6ba033793457581f5aa07b41d175698a057..21a93775271ed0d2398cf05033a81cc24970c37c 100644 (file)
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -601,8 +601,9 @@ static int mcs7830_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
        u8 status;
 
-       if (skb->len == 0) {
-               dev_err(&dev->udev->dev, "unexpected empty rx frame\n");
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len) {
+               dev_err(&dev->udev->dev, "unexpected tiny rx frame\n");
                return 0;
        }
 

diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 83f965cb69e74fc233d03fdd93538a05304e6670..006a14244a3261c3b8586ef4f9771f4cf33a29bc 100644 (file)
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -419,6 +419,10 @@ static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
        struct nc_trailer       *trailer;
        u16                     hdr_len, packet_len;
 
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        if (!(skb->len & 0x01)) {
 #ifdef DEBUG
                struct net_device       *net = dev->net;

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 3b6e932890bb77a414912fb68543cfa6212d6f37..387ececab6e84d3a4ed518b79d67f8fb046046b0 100644 (file)
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -202,10 +202,10 @@ 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
-        */
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        switch (skb->data[0] & 0xf0) {
        case 0x40:
                proto = htons(ETH_P_IP);

diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index c8f1b5b3aff3e13c85bf8c528a59eebc0a7cf8c1..bf4c0979c484e995b2ada2165a00d8eba123bbab 100644 (file)
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -490,6 +490,10 @@ EXPORT_SYMBOL_GPL(rndis_unbind);
  */
 int rndis_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        /* peripheral may have batched packets to us... */
        while (likely(skb->len)) {
                struct rndis_data_hdr   *hdr = (void *)skb->data;

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index d89747aa3c4b401c1d60ef331e40fb90eec03e5b..5a82fcdd206b902496ccfbcf88976213c7d670db 100644 (file)
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -1093,6 +1093,10 @@ static void smsc75xx_rx_csum_offload(struct usbnet *dev, struct sk_buff *skb,
 
 static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        while (skb->len > 0) {
                u32 rx_cmd_a, rx_cmd_b, align_count, size;
                struct sk_buff *ax_skb;

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 94ae66999f592a4a8a1c45fdc24505050271d086..13f50713bf7e14fae07e27f9242085d95d988dfe 100644 (file)
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1041,6 +1041,10 @@ static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
 
 static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        while (skb->len > 0) {
                u32 header, align_count;
                struct sk_buff *ax_skb;

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 174aece3b90a573a0165207c6b2581b472dbda34..1283de5b31f31ef3d72c083ef3825394f41026c8 100644 (file)
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -415,17 +415,19 @@ static inline void rx_process (struct usbnet *dev, struct sk_buff *skb)
        }
        // else network stack removes extra byte if we forced a short packet
 
-       if (skb->len) {
-               /* all data was already cloned from skb inside the driver */
-               if (dev->driver_info->flags & FLAG_MULTI_PACKET)
-                       dev_kfree_skb_any(skb);
-               else
-                       usbnet_skb_return(dev, skb);
+       /* all data was already cloned from skb inside the driver */
+       if (dev->driver_info->flags & FLAG_MULTI_PACKET)
+               goto done;
+
+       if (skb->len < ETH_HLEN) {
+               dev->net->stats.rx_errors++;
+               dev->net->stats.rx_length_errors++;
+               netif_dbg(dev, rx_err, dev->net, "rx length %d\n", skb->len);
+       } else {
+               usbnet_skb_return(dev, skb);
                return;
        }
 
-       netif_dbg(dev, rx_err, dev->net, "drop\n");
-       dev->net->stats.rx_errors++;
 done:
        skb_queue_tail(&dev->done, skb);
 }
@@ -447,13 +449,6 @@ static void rx_complete (struct urb *urb)
        switch (urb_status) {
        /* success */
        case 0:
-               if (skb->len < dev->net->hard_header_len) {
-                       state = rx_cleanup;
-                       dev->net->stats.rx_errors++;
-                       dev->net->stats.rx_length_errors++;
-                       netif_dbg(dev, rx_err, dev->net,
-                                 "rx length %d\n", skb->len);
-               }
                break;
 
        /* stalls need manual reset. this is rare ... except that