MdeModulePkg/Bus/Pci: Fix Descriptor Misalignment in USB Config Handling #6442
Conversation
The issue with locating the expected interface and endpoint descriptors arises because `configDesc` (USB_CONFIG_DESCRIPTOR) and `IfDesc` (USB_INTERFACE_DESCRIPTOR) are incremented by structure size rather than by actual descriptor length. Specifically: - `configDesc` should be incremented by its actual length. - `IfDesc` should be incremented by its actual length. This incorrect increment causes misalignment, preventing access to the subsequent interface and endpoint descriptors. [Suggested Solution] Update the code to increment the pointers by the actual descriptor lengths, ensuring proper access to all descriptors in the USB configuration. Signed-off-by: Aniket Surekar <[email protected]>
| @@ -2848,7 +2848,7 @@ XhcInitializeEndpointContext ( | |||
| MaxDci = 1; | |||
| } | |||
|
|
|||
| EpDesc = (USB_ENDPOINT_DESCRIPTOR *)(IfDesc + 1); | |||
| EpDesc = (USB_ENDPOINT_DESCRIPTOR *)((UINTN)IfDesc + IfDesc->Length); | |||
There was a problem hiding this comment.
Since IfDesc and ConfigDesc are pointer values, it would be better to type cast to another pointer type such as UINT8 * instead of an integer type of UINTN.
There was a problem hiding this comment.
Yes, I had a similar thought, but another file with existing code uses UINTN typecasting for IfDesc , ConfigDesc and EpDesc, so I kept it the same to maintain consistency. I referenced some of the existing code in
edk2/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c:
XhcInitializeEndpointContext() API: lines 2854, 2858, 2935, 2993, 3008
XhcInitializeEndpointContext64() API: lines 3057, 3061, 3138, 3196, 3211 And so on....
Similarly, in edk2/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c:
XhcPeiSetConfigCmd API: lines 1765, 1833, 1880, 1895
XhcPeiSetConfigCmd64 API: lines 1991, 2060, 2107, 2124 And so on....
For uniformity, I chose to retain the same typecasting.
There was a problem hiding this comment.
@mdkinney @lgao4 @niruiyu If you are satisfied with the changes, could you please approve them?
These changes address an issue where multiple third-party EFTPOS USB devices caused the system to hang in preboot after being connected to the SUT.
Detail analysis:
Analysis of the Issue: (same analysis is applicable for End point descriptor)
The issue occurs because the Interface/Endpoint Descriptor does not receive the expected value, and the length of the interface descriptor is being read as 0 (9th byte of data is 0x0 mentioned in hex dump snap). Consequently, the system hangs in a while loop.
The root cause is that the configDesc (USB_CONFIG_DESCRIPTOR) structure has a fixed length of 9 bytes. However, recent logs from the dump show that a third-party USB device reports a configuration length of 0x0A (10 bytes). In the code, the length is incremented by 9 bytes, causing the interface descriptor to be incorrectly updated from 9 bytes instead of 10 bytes. This mismatch results in the interface descriptor and End Point descriptor type and length being incorrect.
Snap hex dump of config/interface/end point descriptor (Red highlighted config descriptor , Blue highlighted Interface descriptor and remaining dump for end point descriptor )
So fix as mentioned :
Update the code to increment the pointers by the actual descriptor lengths,
ensuring proper access to all descriptors in the USB configuration.
The issue with locating the expected interface and endpoint descriptors
arises because configDesc (USB_CONFIG_DESCRIPTOR) and IfDesc
(USB_INTERFACE_DESCRIPTOR) are incremented by structure size rather than
by actual descriptor length.
Specifically:
configDesc should be incremented by its actual length.
IfDesc should be incremented by its actual length.
This incorrect increment causes misalignment, preventing access to the
subsequent interface and endpoint descriptors.
[Suggested Solution]
Update the code to increment the pointers by the actual descriptor lengths,
ensuring proper access to all descriptors in the USB configuration.
Bugzilla ID 4856