[3.5.y.z extended stable] Patch "usbnet: remove generic hard_header_len check" has been added to staging queue
Luís Henriques
luis.henriques at canonical.com
Wed Feb 26 15:28:14 UTC 2014
On Wed, Feb 26, 2014 at 03:32:09PM +0100, Emil Goode wrote:
> Hello Luis,
>
> For the 3.5.y kernel, the below patch is also needed in order to fix all
> bugs in the asix module related to packets crossing urb boundaries.
> It was included in the v3.8-rc3 mainline kernel.
>
> commit 8b5b6f5413e97c3e8bafcdd67553d508f4f698cd
> Author: Lucas Stach <dev at lynxeye.de>
> Date: Wed Jan 16 04:24:07 2013 +0000
>
> net: asix: handle packets crossing URB boundaries
>
> The best thing is if this patch also could be applied before the next
> release of 3.5.y.
>
> I can send a backport of the above patch that applies to 3.5.y this
> evening.
Obviously, this patch doesn't apply cleanly to the 3.5 kernel -- the driver
has been split into several files, and some of the changes don't seem to be
applicable to this kernel.
But I would be more than happy to take a backport from you, specially if
you're able to give it some testing ;-)
Cheers,
--
Luís
> Best regards,
>
> Emil Goode
>
> On Wed, Feb 26, 2014 at 01:25:38PM +0000, Luis Henriques wrote:
> > This is a note to let you know that I have just added a patch titled
> >
> > usbnet: remove generic hard_header_len check
> >
> > to the linux-3.5.y-queue branch of the 3.5.y.z extended stable tree
> > which can be found at:
> >
> > http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.5.y-queue
> >
> > If you, or anyone else, feels it should not be added to this tree, please
> > reply to this email.
> >
> > For more information about the 3.5.y.z tree, see
> > https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
> >
> > Thanks.
> > -Luis
> >
> > ------
> >
> > From af183b6b0e06a6238c257167aec3ed4f18d78fc5 Mon Sep 17 00:00:00 2001
> > From: Emil Goode <emilgoode at gmail.com>
> > Date: Thu, 13 Feb 2014 17:50:19 +0100
> > Subject: usbnet: remove generic hard_header_len check
> >
> > commit eb85569fe2d06c2fbf4de7b66c263ca095b397aa upstream.
> >
> > 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 at gmail.com>
> > Reported-by: Igor Gnatenko <i.gnatenko.brain at gmail.com>
> > Signed-off-by: David S. Miller <davem at davemloft.net>
> > [ luis: backported to 3.5: dropped changes to drivers/net/usb/ax88179_178a.c
> > and drivers/net/usb/sr9800.c ]
> > Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> > ---
> > drivers/net/usb/gl620a.c | 4 ++++
> > drivers/net/usb/mcs7830.c | 5 +++--
> > drivers/net/usb/net1080.c | 4 ++++
> > drivers/net/usb/qmi_wwan.c | 8 ++++----
> > drivers/net/usb/rndis_host.c | 4 ++++
> > drivers/net/usb/smsc75xx.c | 4 ++++
> > drivers/net/usb/smsc95xx.c | 4 ++++
> > drivers/net/usb/usbnet.c | 25 ++++++++++---------------
> > 8 files changed, 37 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
> > index db3c802..f14e8de 100644
> > --- 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 03c2d8d..3acdf1d 100644
> > --- 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 28c4d51..389afcf 100644
> > --- 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 7589509..6546f21 100644
> > --- 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 4a433583..38e6d25 100644
> > --- a/drivers/net/usb/rndis_host.c
> > +++ b/drivers/net/usb/rndis_host.c
> > @@ -495,6 +495,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 c241ae7..a4370d2 100644
> > --- a/drivers/net/usb/smsc75xx.c
> > +++ b/drivers/net/usb/smsc75xx.c
> > @@ -1097,6 +1097,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 b1112e7..86b6fda 100644
> > --- 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 a936201..49d4e9a 100644
> > --- 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
> > --
> > 1.9.0
> >
More information about the kernel-team
mailing list