Thanks for the explanation. Now it is clear why we don't need this in
upstream.
--Pratham
On 4/26/2021 3:47 PM, Felipe Balbi wrote:
Hi,
Pratham Pratap <prathampratap@xxxxxxxxxxxxxx> writes:
Hi,
Let's say a function driver queues a request on ep0 and before the
completion handler could run composition switch/physical disconnect
happens. This request will be in pending list since gadget_giveback is
not done but the composite driver will free the request from
composite_dev_cleanup. Now, once the next connect happens, another ep0
request is queued and while handling the completion of that request,
gadget driver might end up accessing the old/stale request leading to
list_poison since pending list is corrupted.
argh, I'm assuming you're using dwc3. It's always a good idea to Cc
maintainers for the drivers or subsystems in question. You can rely on
scripts/get_maintainer.pl to get an idea who you should Cc.
Anyway, assuming dwc3.
Have you tried to actually reproduce this? Did you collect tracepoints?
Did you read dwc3's documentation, specially in regards to reporting
bugs?
Try to consider what happens when the cable is yanked and you'll quickly
realize what you suggest can't happen. How does USB know that cable is
disconnected? What happens to dwc3 when the cable is disconnected? What
does the driver do about it?
If you really found a bug, please report it correctly, following the
Reporting Bugs section of dwc3 documentation, Cc the relevant people and
make sure to reproduce the problem with *mainline*; downstream kernel is
not acceptable ;-)
Also, please be clear about the setup you're using. The only thing I can
infer is that you're using dwc3 with one of the QC platforms and I can
only infer that due to your email domain. Please be clear, how to
reproduce? Which QC platform are you using? Which kernel version?
To fix this, the function drivers might want to use setup_pending(mark
it to true) flag so that when composite_dev_cleanup is run the requests
are given back from usb_ep_dequeue; clear the setup pending flag in
function driver when completion handler is run successfully. I can see
this issue in almost all the function drivers.
Nah, we don't need that. Please answer the questions above about
handling for cable disconnect and you'll see this is unnecessary.