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/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index cea1f3d0311b12bdbfb18d2011a774c9e7e200ab..d33c3ae2fcea8612a848640f994879e21829df0f 100644 (file)
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1109,6 +1109,10 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
        u16 hdr_off;
        u32 *pkt_hdr;
 
+       /* This check is no longer done by usbnet */
+       if (skb->len < dev->net->hard_header_len)
+               return 0;
+
        skb_trim(skb, skb->len - 4);
        memcpy(&rx_hdr, skb_tail_pointer(skb), 4);
        le32_to_cpus(&rx_hdr);

diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index a7e3f4e55bf3651b3874710ffcf4cfcdeb37e299..82ab61d62804818827157a0daf48524e72718784 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 03832d3780aa6134059dc4768bb7614d71075b82..9237c45883cd2fc44af92da1c1da10dad7eb5507 100644 (file)
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -529,8 +529,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 93e0716a118c309f90859e4ee913d2e2e32b41e0..7f4a3a41c4f800f042360369f7851341ca5a3611 100644 (file)
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -366,6 +366,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)) {
                netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
                           skb->len, dev->net->hard_header_len, dev->hard_mtu,

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 401d9d740d5fde2e717b520f32e8ab53ec5b8cad..37d9785974fc4d46a3bca4069909b37103ae4abe 100644 (file)
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -80,10 +80,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 cc49aac70224910f63fc6c5ed6b0b2fc720ff674..691fca4e4c2d25b19b8896a63368eabeceb87362 100644 (file)
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -494,6 +494,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 66ebbacf066f6692ba21d2fa07538535827074b9..12afae0451e6932dd2b86067dfd1903d21b0b49b 100644 (file)
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2108,6 +2108,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 3f38ba868f6182152093e8efad0a864439c5cef0..9375b8c6410b78605ae0bf998e36f8bda8adb1b4 100644 (file)
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1725,6 +1725,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 28f16ed6422dfbf9a18d42ffa67cfe5a83d4a972..f6dce4765de4a332aed3bba244fbdfd193c2a9fb 100644 (file)
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -517,17 +517,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);
 }
@@ -549,13 +551,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