Re: WARNING in usb_composite_setup_continue

Alan Stern
2020-11-16T16:00:56Z

On Mon, Nov 16, 2020 at 10:02:22AM +0000, Peter Chen wrote:
> On 20-11-13 12:00:15, Alan Stern wrote:
> > That's right. Unfortunately, I think fixing this will require changes
> > to the UDC drivers as well as to the composite core. Similar to what I
> > said earlier, there will have to be a way for the composite core to tell
> > the UDC driver which SETUP packet the zero-length status reply is meant
> > for.
>
> It needs the composite layer to support multiple requests for EP0, the
> effort is big. It is better the real problem is reported, then, it has
> environment to test the solution. The reported FuzzUSB issue is not this issue.

WE don't really need to support multiple ep0 requests. We just need to
know whether a particular response was meant for the most recent
ep0 transfer or for an earlier one.

For example, the UDC driver could keep a counter of SETUP packets, and
it could pass this counter to the composite core's setup routine. The
status reply request could include the corresponding counter value, and
the UDC driver could then ignore replies that are not meant for the most
recent SETUP.

Of course, this would be a big API change, affecting all UDC and gadget
drivers. Maybe you can think of a better way to fix the problem.

> > Suppose the host sends a Set-Interface request, and the function driver
> > is not able to handle it (maybe a memory allocation fails). The gadget
> > should report this failure to the host by STALLing ep0. But there is no
> > way for the function driver to tell the composite core that the failure
> > occurred!
> >
> > You can see this problem in f_mass_storage.c. If do_set_interface()
> > encounters an error, it will return a negative error code. But the
> > caller (handle_exception()) ignores the return code!
> >
> > When Dave Brownell designed the Gadget and Composite APIs, he really did
> > not give sufficient thought to the issues involved in delayed responses
> > to control-OUT transfers.
> >
>
> We could add one parameter for usb_composite_setup_continue to indicate
> error occurred during function's deferred operation, and stall the ep0
> at usb_composite_setup_continue, do you think so?

That would work only if the function driver also had a way to tell the
composite core which control transfer failed.

The basic problem is the race between a control status response and
arrival of a new SETUP packet. This race exists between the UDC driver
and the gadget driver (that is, the composiste core), and between the
composite core and the function drivers.

Alan Stern