On 06/21/2013 05:47 PM, Ming Lei wrote:
The bug can be easily reproduced with Gadget Zero in full_speed mode
connected to a host with UHCI controller by the standard test from
tools/usb/ with command line
testusb -a -t 7 -c 2000 -s 4096 -v 41
I am wondering if it is a valid test, since the built sg list has one middle sg
which size is not maxp aligned.
Did you run the test on ehci controller? and is the transfer OK on ehci
controller?
Yes, for ehci the test succeded.
This test exists inside linux kernel source tree for long time. It is
important to check usb infrastructure integrity.
The test utility 'testusb' uses ioctl call to a driver 'usbtest', and
actually the driver run a test (in the kernel space) on the host. The
hardware (Gadget Zero device) verifies the test transfer.
The test crashes with SIGBUS.
IMO, the test may cause buffer crash.
No buffer crash. The driver signals about the test failed in such way.
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -977,6 +977,9 @@ static int uhci_submit_common(struct uhci_hcd *uhci, struct urb *urb,
for (;;) { /* Allow zero length packets */
int pktsze = maxsze;
+ if (this_sg_len < pktsze) /* The short packet */
+ pktsze = this_sg_len;
This will build a short packet transfer descriptor which is not the last
one, and it isn't correct.
Why do you think so? Could you please refer to the specification of UHCI.
The attached patch may avoid the memory crash, but I am wondering if
we need to consider the situation, or a warning should be needed.
diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index 041c6dd..5c98044 100644
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -969,6 +969,13 @@ static int uhci_submit_common(struct uhci_hcd
*uhci, struct urb *urb,
pktsze = len;
if (!(urb->transfer_flags & URB_SHORT_NOT_OK))
status &= ~TD_CTRL_SPD;
+ } else {
+ /* skip the middle short transfer */
+ if (this_sg_len < maxsze) {
+ len -= this_sg_len;
+ this_sg_len = 0;
+ goto skip;
+ }
}
if (plink) {
@@ -989,6 +996,7 @@ static int uhci_submit_common(struct uhci_hcd
*uhci, struct urb *urb,
data += pktsze;
this_sg_len -= pktsze;
len -= maxsze;
+skip:
if (this_sg_len <= 0) {
if (--i <= 0 || len <= 0)
break;
Your patch skips a chunk of data to transfer. This is a corruption of
data. It's unacceptable. We can return an error code if we treat the
request as malformed, but we can't transfer data selectively.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
