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: Ben Hutchings <ben@decadent.org.uk>

diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index c4cfd1dea88149d56fb1315155d10a6461cfeb9d..75d90407c39e07c7006b75c94ebaefe12ee5f13f 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 db2cb74bf8549620e39e18b20bd69479a4e8fbfe..a2f757976eac77c5e6d64d4d30e5b3e5a5045fde 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 01db4602a39edbc75dadb39dec0d6a4bfb9ad147..c9e32783b00847364d55cd716f6c273205341ca7 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/rndis_host.c b/drivers/net/usb/rndis_host.c
index 255d6a424a6b56f303b93296ef744268a2e4cbd7..13b40e335a10648f53176ac13678a3d7a0f3c674 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 a8e464024fbb0d4d086546b01fe17618a869d479..c41b42ba80c20181249213bb1a738ecac8587e59 100644 (file)
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -1079,6 +1079,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 55b3218c2cd2f363d2aba99dbf1572c0a3eed897..6617325b14c2da86fe612dc761aefeaa84bcde4f 100644 (file)
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1039,6 +1039,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 dc53a8f83001c6d8b69a72499696170098f6246b..3d217421c016f3d43beb08b44f64c7d0b4b24ab0 100644 (file)
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -414,17 +414,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);
 }
@@ -446,13 +448,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