On 2026-10-01 21:47, Peter Maydell wrote:
The OHCI USB controller requires software to set up an Endpoint
Descriptor data structure. This specifies (among other things) both
an EndpointNumber telling the controller which USB endpoint to talk
to, and a Format (F) bit which is 1 for Isochronous transfers and 0
for General transfers.
The specification requires that software matches the F bit with the
endpoint it's setting up the descriptor for, so that Control, Bulk
and Interrupt endpoints use F=0, and Isochronous endpoints use F=1.
It doesn't require the controller to check whether software gets this
right.
The spec isn't totally clear about what happens if software creates
an ED with a mismatch, but I believe that the controller just trusts
the F bit, and tries to talk to the endpoint with the wrong sequence
of USB tokens (notably, isochronous transactions have no handshaking
and the others do). This probably results in the transfer failing.
In hcd-ehci.c we check for the "iso transfer to non-iso endpoint" and
make it fail, but we don't do this in hcd-ohci.c. The result is that
a guest that deliberately sets up this situation can cause us to hit
the assertion added in commit 7b1eba96aeca, where the endpoint is not
isochronous and returns USB_RET_ASYNC to an iso TD handling function
that isn't prepared to deal with that.
Add checks for both "iso transfer to non-iso endpoint" and "non-iso
transfer to iso endpoint" and make those TDs fail rather than blindly
sending them to the endpoint.
Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3729
Signed-off-by: Peter Maydell <[email protected]>
---
The diff looks a little complex because of the re-indenting;
looking at an "ignore whitespace changes" diff might be clearer
Only testing here has been running the repro in the bug tracker
plus "make check-functional", which probably doesn't exercise
USB very much.
---
hw/usb/hcd-ohci.c | 61 +++++++++++++++++++++++++++------------------
hw/usb/trace-events | 2 ++
2 files changed, 39 insertions(+), 24 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
These functions are huge...
Side note, I don't get why we copy packet data *then* return
error if device not found (trace_usb_ohci_td_dev_error), this
is sub-optimal.
