On Thu, Nov 12, 2020 at 09:01:11AM +0000, Peter Chen wrote: > On 20-11-11 14:47:10, Alan Stern wrote: > > There's still a possible race, although it's a different one: The > > gadget's delayed status reply can race with the host timing out and > > sending a new SETUP packet: > > > > Host sends SETUP packet A > > > > Function receives A and decides > > to send a delayed status reply > > > > Function thread starts to > > process packet A > > > > Host times out waiting for A status > > > > Host sends new SETUP packet B > > > > Composite core receives packet B > > and resets cdev->delayed_status > > resets cdev->delayed_status? Where to do that? Sorry, for some reason I though I read a line that set delayed_status to 0 in composite_setup(). I must have been fantasizing... > Even the host re-try the > control transfer, it should send the same control request, eg, > SET_CONFIGURATION, and increase cdev->delayed_status. There is a description > for possible host sends next control request before receiving status > packet at USB 2.0 Spec, CH 5.5.5 Control Transfer Data Sequences > > If a Setup transaction is received by an endpoint before a previously > initiated control transfer is completed, > the device must abort the current transfer/operation and > handle the new control Setup transaction. A Setup > transaction should not normally be sent before the completion > of a previous control transfer. However, if a > transfer is aborted, for example, due to errors on the bus, > the host can send the next Setup transaction > prematurely from the endpoint’s perspective. Yes. The composite core does not abort the current transfer/operation when a new Setup transaction occurs. But it should -- and it should set delayed_status back to 0. (Maybe this is what I was fantasizing.) > > Function thread finishes and calls > > usb_composite_setup_continue() > > > > The composite core sends a status > > reply for packet A, not packet B > > > > Host receives status for A but thinks > > it is the status for B! > > > > Function thread processes packet B > > > > Function thread finishes and calls > > usb_composite_setup_continue() > > > > The composite core sees > > cdev->delayed_status == 0 and WARNs. > > > > At the moment I don't see how to prevent this sort of race from > > happening. We may need to change the API, giving the composite core a > > way to match up calls to usb_composite_setup_continue() with the > > corresponding call to composite_setup(). But even that wouldn't fix > > the entire problem. > > > > Hi Alan, > > I more think a possible reset or disconnect occurrence between them, and > composite_disconnect is called. That sounds reasonable. This is why I think we will need to change the API. There has to be a way to tell usb_composite_setup_continue() which SETUP it is being called for. A new SETUP or a disconnect should invalidate the old SETUP, and then usb_composite_setup_continue() would ignore any calls that were for the old SETUP packet. Alan Stern
