Re: Fwd: [DWC3][Gadget] Question regarding the unmapping of request as part of queue failure

[Felipe Balbi <balbi@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2406 bytes --]


Hi,

Sriharsha Allenki <sallenki@codeaurora.org> writes:
>> Sriharsha Allenki <sallenki@codeaurora.org> writes:
>>> I was looking at the code flow for ep_queue and came across the
>>> following piece of code.
>>>
>>> __dwc3_gadget_kick_transfer {
>>>  
>>>     dwc3_prepare_trbs(dep);
>>>     req = next_request(&dep->started_list);
>>>     if (!req) {
>>>             dep->flags |= DWC3_EP_PENDING_REQUEST;
>>>             return 0;
>>>     }
>>> }
>>>
>>> As part of dwc3_prepare_trbs(dep), we get a request from the pending_list
>>> and queue to the tail of the started_list. But here we get the head of
>>> the started_list, now if there is any failure in issuing UPDATE_TRANSFER
>>> to the core, we unmap this request using "dwc3_gadget_del_and_unmap_request".
>>>
>>> But if this kick_transfer was part of the ep_queue and we have failed
>>> to issue update transfer, instead of unmapping the request we are trying
>>> to queue, we will be unmapping a different request (first in the started_list)
>>> which the core could have already started processing. I believe we should unmap
>>> the request we are trying to queue but not any other.
>> no, we have to start requests in order and dequeue them in order as
>> well. There's no way to verify that the request is already processed by
>> the HW, other than checking HWO bit which is set during
>> dwc3_prepare_trbs(). This is a HW-SW race condition that we can't really
>> fix.
>>
>> It is minimized, however, by the fact that, at least for non-isoc
>> endpoints, we use No Status Update Transfer commands, which means the
>> command can't fail.
> Thanks Felipe for the reply. I see that this is a trick race condition
> between HW-SW, I have seen one occurrence where ep_queue from f_fs has
> failed (at kick_transfer).And since Asynchronous IO has been enabled,

what the reason to failure? Capture the debug data and send as a reply
to this message. Method for reporting bugs on dwc3 is documented.

> the request was freed leading to thecorruption of started_list because
> the list_del and unmap was happened on the requestat the head, but the
> request freed by the f_fs is at the tail of the started_list. This
> caused a use after free issue.
>
> Please let me know your comments.

No comments, really. I need to see debug data from dwc3.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]