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

[Sriharsha Allenki <sallenki@codeaurora.org>]

Hi,


On 3/28/2020 2:04 PM, Felipe Balbi wrote:
> Hi,
>
> 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,
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.

Thanks and Regards,
Sriharsha