Re: [PATCH 4.14,4.19,4.20] usb: dwc3: gadget: Clear req->unaligned and zero flags on cleanup

[Sasha Levin <sashal@kernel.org>]

On Fri, Jan 25, 2019 at 05:51:53PM -0800, Jack Pham wrote:
>commit bd6742249b9ca918565e4e3abaa06665e587f4b5 upstream.
>
>OUT endpoint requests may somtimes have one of these flags set
>when preparing to be submitted to HW indicating that there is an
>additional TRB chained to the request for alignment purposes.
>If that request is removed before the controller can execute the
>transfer (e.g. ep_dequeue/ep_disable), the request will not go
>through the dwc3_gadget_ep_cleanup_completed_request() handler
>and will not have its unaligned or zero flag cleared when
>dwc3_gadget_giveback() is called.  This same request could be
>later requeued for a new transfer that does not require an
>extra TRB and if it is successfully completed, the cleanup
>and TRB reclamation will incorrectly process the additional TRB
>which belongs to the next request, and incorrectly advances the
>TRB dequeue pointer, thereby messing up calculation of the next
>requeust's actual/remaining count when it completes.
>
>The right thing to do here is to ensure that the flags are cleared
>before it is given back to the function driver.  A good place
>to do that is in dwc3_gadget_del_and_unmap_request().
>
>Fixes: c6267a51639b ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
>Signed-off-by: Jack Pham <jackp@codeaurora.org>
>Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>[jackp: backport to <= 4.20: replaced 'needs_extra_trb' with 'unaligned'
>        and 'zero' members in patch and reworded commit text]
>Signed-off-by: Jack Pham <jackp@codeaurora.org>
>---
> drivers/usb/dwc3/gadget.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>index 9f92ee03dde7..2a4ea9a1b1e3 100644
>--- a/drivers/usb/dwc3/gadget.c
>+++ b/drivers/usb/dwc3/gadget.c
>@@ -177,6 +177,8 @@ static void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep,
> 	req->started = false;
> 	list_del(&req->list);
> 	req->remaining = 0;
>+	req->unaligned = false;
>+	req->zero = false;
>
> 	if (req->request.status == -EINPROGRESS)
> 		req->request.status = status;

Thanks! I'll let Greg pick it up (as the upstream patch was explicitly
tagged for stable).

--
Thanks,
Sasha