* [PATCH] Fixes buffer allocation size and the actual packet length;
@ 2016-04-26 18:50 Petko Manolov
2016-04-26 18:50 ` Petko Manolov
2016-04-26 19:45 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Petko Manolov @ 2016-04-26 18:50 UTC (permalink / raw)
To: netdev; +Cc: davem, petkan
As noticed by Lincoln Ramsay <a1291762@gmail.com> some old (usb 1.1) Pegasus
based devices may actually return more bytes than the specified in the datasheet
amount. That would not be a problem if the allocated space for the SKB was
equal to the parameter passed to usb_fill_bulk_urb(). Some poor bugger (i
really hope it was not me, but 'git blame' is useless in this case, so anyway)
decided to add '+ 8' to the buffer length parameter. Sometimes the usb transfer
overflows and corrupts the socket structure, leading to kernel panic.
The above doesn't seem to happen for newer (Pegasus2 based) devices which did
help this bug to hide for so long.
Nearly all Pegasus devices may append the RX status to the end of the received
packet. It is the default setup for the driver. The actual ethernet packet is
4 bytes shorter. Why and when 'pkt_len -= 4' became 'pkt_len -= 8' is again
hidden in the mists of time. There might have been a good reason to do so, but
multiple reads of the datasheet did not point me to any.
The patch is against v4.6-rc5 and was tested on ADM8515 device by transferring
multiple gigabytes of data over a couple of days without any complains from the
kernel.
Petko Manolov (1):
Fixes buffer allocation size and the actual packet length;
drivers/net/usb/pegasus.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--
2.8.0.rc3
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Fixes buffer allocation size and the actual packet length;
2016-04-26 18:50 [PATCH] Fixes buffer allocation size and the actual packet length; Petko Manolov
@ 2016-04-26 18:50 ` Petko Manolov
2016-04-26 19:45 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Petko Manolov @ 2016-04-26 18:50 UTC (permalink / raw)
To: netdev; +Cc: davem, petkan
usb_fill_bulk_urb() receives buffer length parameter 8 bytes larger
than what's allocated by alloc_skb(); This seems to be a problem with
older (pegasus usb-1.1) devices, which may silently return more data
than the maximal packet length.
Going through the chip's documentation i figured out the ethernet
packet is appended with 4 bytes of status data. That's why we now
subtract 4 instead of 8 bytes from the reported packet length.
Reported-by: Lincoln Ramsay <a1291762@gmail.com>
Signed-off-by: Petko Manolov <petkan@mip-labs.com>
---
drivers/net/usb/pegasus.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index f840802..780c217 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -497,7 +497,7 @@ static void read_bulk_callback(struct urb *urb)
pkt_len = buf[count - 3] << 8;
pkt_len += buf[count - 4];
pkt_len &= 0xfff;
- pkt_len -= 8;
+ pkt_len -= 4;
}
/*
@@ -528,7 +528,7 @@ static void read_bulk_callback(struct urb *urb)
goon:
usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
usb_rcvbulkpipe(pegasus->usb, 1),
- pegasus->rx_skb->data, PEGASUS_MTU + 8,
+ pegasus->rx_skb->data, PEGASUS_MTU,
read_bulk_callback, pegasus);
rx_status = usb_submit_urb(pegasus->rx_urb, GFP_ATOMIC);
if (rx_status == -ENODEV)
@@ -569,7 +569,7 @@ static void rx_fixup(unsigned long data)
}
usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
usb_rcvbulkpipe(pegasus->usb, 1),
- pegasus->rx_skb->data, PEGASUS_MTU + 8,
+ pegasus->rx_skb->data, PEGASUS_MTU,
read_bulk_callback, pegasus);
try_again:
status = usb_submit_urb(pegasus->rx_urb, GFP_ATOMIC);
@@ -823,7 +823,7 @@ static int pegasus_open(struct net_device *net)
usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
usb_rcvbulkpipe(pegasus->usb, 1),
- pegasus->rx_skb->data, PEGASUS_MTU + 8,
+ pegasus->rx_skb->data, PEGASUS_MTU,
read_bulk_callback, pegasus);
if ((res = usb_submit_urb(pegasus->rx_urb, GFP_KERNEL))) {
if (res == -ENODEV)
--
2.8.0.rc3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Fixes buffer allocation size and the actual packet length;
2016-04-26 18:50 [PATCH] Fixes buffer allocation size and the actual packet length; Petko Manolov
2016-04-26 18:50 ` Petko Manolov
@ 2016-04-26 19:45 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2016-04-26 19:45 UTC (permalink / raw)
To: petkan; +Cc: netdev
Submitting a cover letter and the patch itself with identical Subject lines
is not correct.
You must also use proper "[PATCH $(TREE) X/Y] " prefixes in your Subject
lines as well.
Please format your Subject lines correctly and resubmit, thank you.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-26 19:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-26 18:50 [PATCH] Fixes buffer allocation size and the actual packet length; Petko Manolov
2016-04-26 18:50 ` Petko Manolov
2016-04-26 19:45 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.