Opened 5 years ago

Closed 5 years ago

#10867 closed bug (fixed)

EHCI Control Transfers : Hang if length of buffer > size of input data

Reported by: akshay1994 Owned by: phoudoin
Priority: normal Milestone: R1
Component: Drivers/USB Version: R1/Development
Keywords: USB EHCI Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

EHCI control transfers hang indefinitely, if the length of buffer (wLength) specified, is greater than the size of the object received. This is repeat-able, by performing a control transfer asking for any descriptor, with length set to (size of descriptor + 1).

Attachments (3)

0001-Fix-EHCI-Control-Transfers.patch (933 bytes ) - added by akshay1994 5 years ago.
Patch (updated)
0001-Fix-Short-Packet-EHCI-Control-Transfers.patch (1.6 KB ) - added by akshay1994 5 years ago.
0001-Fix-Short-Packet-EHCI-Transfers.patch (2.2 KB ) - added by akshay1994 5 years ago.
Corrected Patch

Download all attachments as: .zip

Change History (14)

comment:1 by akshay1994, 5 years ago

Has a Patch: set

comment:2 by anevilyak, 5 years ago

Component: - GeneralDrivers/USB
Owner: changed from nobody to mmlr

comment:3 by jessicah, 5 years ago

Style: missing the != 0 for the first part of the if condition.

by akshay1994, 5 years ago

Patch (updated)

comment:4 by akshay1994, 5 years ago

Updated :)

comment:5 by akshay1994, 5 years ago

Updated the patch. In case of a Short Packet Control Transfer, EHCI falls back to Alternate_Next_qTD. This made it bypass the Status Stage, due to which an interrupt was not generated on completion of the transfer, causing a delay. :)

comment:6 by pulkomandy, 5 years ago

Owner: changed from mmlr to phoudoin
Status: newassigned

comment:7 by korli, 5 years ago

Thanks for the patch and the bug report. It seems like this isn't the correct fix for the problem. Active transfers are active for a reason, the spec implies that the software shouldn't mess with them, and other implementations actually don't.

comment:8 by akshay1994, 5 years ago

Thanks a lot for the response. Yes. You are right. Even on a short packet transaction, the HC does set the active bit to 0. (EHCI Spec, Pg 83, Point 4)

(I read the specification wrong, taking a zero length transfer by Host Controller, to be a Short Packet transfer. Sorry!!)

Okay, so why does this still anyway fail. Upon encountering a Short Packet, the HC goes to the Alternate_Next_qTD, which is set to a stray descriptor. This has its active bit set to zero, making the HC take a horizontal jump to the next QueueHead, completing this transaction. Now while processing this in FinishTransfers thread, after checking the short packet qTD, we go to the next qTD, which was never processed by the HC, and thus has its Active Bit set.

Solution I propose : 1) Instead of setting Alternate_Next_qTD to the stray descriptor, we set it to the status descriptor (for all the data descriptors), completing the requirement of a status stage after every control request, and also generating the required interrupt. 2) Upon encountering a short packet descriptor, while processing, we skip all descriptors following this, till we reach the status descriptor, which was next processed by the HC.

Please review this solution, while I make a patch for this. Thanks :)

by akshay1994, 5 years ago

Corrected Patch

comment:9 by akshay1994, 5 years ago

I have modified and corrected the patch.

Changes :

  • The Alternate_Next_qTD for all the data descriptors in a control request point to the Status Descriptor of that request. For all others, it points to the Stray Descriptor, as before.
  • In the Finishing Transfers Worker, if we detect a Short Packet, we mark the transfer as done, if it is a Bulk/Interrupt transfer (as no further descriptors were processed, and a horizontal jump to the next queue head took place); and jump to the Status descriptor, in case of a Control Transfer.

in reply to:  7 comment:10 by akshay1994, 5 years ago

Replying to korli:

Please review this also! This will make libusb work with EHCI :)

comment:11 by pulkomandy, 5 years ago

Resolution: fixed
Status: assignedclosed

Applied in hrev47630.

Note: See TracTickets for help on using tickets.