Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error

Alan Stern
2026-04-22T15:35:47Z

On Wed, Apr 22, 2026 at 04:51:30PM +0300, Mathias Nyman wrote:
> On 4/22/26 08:30, Michal Pecio wrote:
> > On Tue, 21 Apr 2026 22:11:41 -0400, Alan Stern wrote:
> > > Can you confirm that it's also possible for xhci-hcd to prevent
> > > control endpoints from restarting when a transaction error (-EPROTO)
> > > occurs? Up until usb_reset_endpoint() or a new callback?
> >
> > Doable. They halt like any other and it's SW's choice how to restart.
>
> Yes, doable.
>
> Would this be used in cases where all hope is lost and we want to reset the
> device, canceling all pending control URBs before restarting the ring,
> thus avoiding sending any extra URBs to the device just to wait for
> them to fail or timeout?

That wasn't my intention. The purpose was to avoid getting into a tight
loop such as in fuzzing tests with dummy-hcd. When a device is
disconnected, if the class driver's completion handler immediately
resubmits after a -EPROTO error, and the resubmitted URB immediately
fails with -EPROTO status, the resulting loop can use enough of a CPU's
time to cause a "CPU hung" error.

The same sort of thing can happen with real host controllers, although
the loop won't be quite so tight. But if we prevent resubmitted URBs
from failing until the endpoint is reset or restarted, that will slow
down the loop a lot. Especially with a series of increasing delays
between the restarts.

> In most cases I can think of it would make sense to keep the control endpoint
> running. Just let the hcd move to the SETUP stage of the next control transfer
> URB and continue.

I'd be in favor of that except for this tight-loop problem.

> For example EMF causing transaction error (-EPROTO) on active IN and OUT bulk
> transfers. Two clear-halt requests are queued, one for each endpoint.
> If first clear-halt request fails with -EPROTO we still want to continue with
> the next request.

Actually no, we don't. If the first clear-halt request fails, it means
the EMF is still present. We want to wait for a short time before
sending the next request. If we don't, and the next request gets sent
right away, it will probably fail also.

> Shouldn't be any toggle/seq-nr issues here on the control endpoint.

Indeed.

> Most control transfer STALL (EPIPE) cases are protocol stalls, and we should
> just continue running. Exception here might be a STALL response
> to a clear-halt request. I assume (didn't check) device must support those.
> So in that case we may want to reset the device.

As I said to Michal, we can treat this the same as a transaction error:
delay and retry, then eventually reset if needed.

In theory there's a risk of racing with a Set-Interface or Set-Config
request, so that the clear-halt refers to an endpoint that doesn't exist
any more. This race can be prevented by making usb_set_interface() and
usb_set_configuration() wait until all pending error recovery is done.

> There's also a risk that the control endpoint isn't started when it should.
> For example class driver could potentially call usb_clear_halt() on the control
> endpoint, synchronously waiting for the clear-halt request to complete before
> calling usb_reset_endpoint(). If endpoint restarts at usb_reset_endpoint() then
> the clear-halt control transfer would time out.

You're suggesting that the clear-halt request might not be sent to the
device (and would therefore time out) because the endpoint queue isn't
running, and it won't start running until usb_reset_endpoint() is
called? That objection applies only to ep0. Other control endpoints
don't have this problem because the clear-halt request is always sent to
ep0.

ep0 is a special case. If it ever does halt in such an extreme fashion
that it can't even accept SETUP packets then there's no way to
communicate with it at all. The USB 2.0 spec mentions this possibility
in section 5.5.5; it says the only way to recover is then to reset the
device. There should never be any reason to send a clear-halt request
for ep0.

Alan Stern