Re: [PATCH 2/3] usb: dwc2: host: do not delay retries after successful transfer
From: Doug Anderson
Date: Tue Jul 17 2018 - 15:59:01 EST
Hi,
On Fri, Jun 15, 2018 at 4:49 PM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> Hi,
>
> On Fri, Jun 15, 2018 at 3:01 PM, Dmitry Torokhov
> <dmitry.torokhov@xxxxxxxxx> wrote:
>> When handling split transactions we should not be delaying retrying
>> SSPLIT/CSPLIT after we successfully communicate with the device, so
>> let's reset dtd->num_naks counter when handling XFERCOMPL.
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> ---
>> drivers/usb/dwc2/hcd_intr.c | 1 +
>> 1 file changed, 1 insertion(+)
>
> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
Though this looked good, I just tested this myself and found that it
corrupts memory. Please do not merge it.
Specifically slub_debug complains of a use after free and sure enough
I can see that in one case we call dwc2_xfercomp_isoc_split_in() and
it can call dwc2_release_channel() and that can free the qtd. Setting
the qtd->num_naks to 0 at the end of the function is a use after free.
I've just tested the same USB peripheral that Dmitry was testing with.
I found two things:
* This patch is important for making that peripheral work
* When I move the "qtd->num_naks" to the beginning it's all good.
Dmitry is OOTO at the moment, so I'll re-post this patch with that change.
-Doug