linux-kernel - RE: [PATCH] usb: dwc3: gadget: Fix controller get stuck when kicking extra transfer in wrong case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Jan 2020 02:38:18 +0000
From:   Ran Wang <ran.wang_1@....com>
To:     Tejas Joglekar <Tejas.Joglekar@...opsys.com>,
        Felipe Balbi <balbi@...nel.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        Jun Li <jun.li@....com>, Peter Chen <peter.chen@....com>
CC:     "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Subject: RE: [PATCH] usb: dwc3: gadget: Fix controller get stuck when kicking
 extra transfer in wrong case

Hi Tejas,


On Tuesday, January 07, 2020 18:16, Tejas Joglekar wrote:
> Hi,
> On 1/7/2020 12:44 PM, Ran Wang wrote:
> > According to original commit c96e6725db9d6a ("usb: dwc3: gadget:
> > Correct the logic for queuing sgs"), we would only kick off another
> > transfer in case of
> > req->num_pending_sgs > 0.
> >
> The commit 8c7d4b7b3d43 ("usb: dwc3: gadget: Fix logical condition") fixes
> the commit f38e35dd84e2 (usb: dwc3: gadget: split
> dwc3_gadget_ep_cleanup_completed_requests()).

Yes, actually I have tried to dig the history of this implementation as much as I can.
and the commit I mentioned looks like the first one to add this function.
 
> > However, current logic will do this as long as req->remaining > 0,
> > this will include the case of non-sgs (both
> > dwc3_gadget_ep_request_completed(req) and
> > req->num_pending_sgs are 0) that we did not want to.
> >
> > Without this fix, we observed dwc3 got stuck on Layerscape plaftorms
> > (such as
> > LS1088ARDB) when enabling gadget (mass storage function) as below:
> >
> Similar issue was reported by Thinh after my fix, and he has submitted a
> patch for the same. You can refer the discussion
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fpatch%2F11292087%2F&amp;data=02%7C01%7Cran.wan
> g_1%40nxp.com%7C97345f38c19849e5a82608d7935a8d31%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C637139889495749889&amp;sdata=tcV6
> FWjqvYYO7BCByACqAg%2FB%2B%2Fz3NC%2B20%2BIEVuyfJc4%3D&amp;reser
> ved=0.

Thanks for this information. I will follow that thread.

Regards,
Ran

> > [   27.923959] Mass Storage Function, version: 2009/09/11
> > [   27.929115] LUN: removable file: (no medium)
> > [   27.933432] LUN: file: /run/media/sda1/419/test
> > [   27.937963] Number of LUNs=1
> > [   27.941042] g_mass_storage gadget: Mass Storage Gadget, version:
> 2009/09/11
> > [   27.948019] g_mass_storage gadget: userspace failed to provide
> iSerialNumber
> > [   27.955069] g_mass_storage gadget: g_mass_storage ready
> > [   28.411188] g_mass_storage gadget: super-speed config #1: Linux File-
> Backed Storage
> > [   48.319766] g_mass_storage gadget: super-speed config #1: Linux File-
> Backed Storage
> > [   68.320794] g_mass_storage gadget: super-speed config #1: Linux File-
> Backed Storage
> > [   88.319898] g_mass_storage gadget: super-speed config #1: Linux File-
> Backed Storage
> > [  108.320808] g_mass_storage gadget: super-speed config #1: Linux
> > File-Backed Storage [  128.323419] g_mass_storage gadget: super-speed
> > config #1: Linux File-Backed Storage [  148.320857] g_mass_storage
> > gadget: super-speed config #1: Linux File-Backed Storage [
> > 148.362023] g_mass_storage gadget: super-speed config #0: unconfigured
> >
> > Fixes: 8c7d4b7b3d43 ("usb: dwc3: gadget: Fix logical condition")
> >
> > Cc: stable@...r.kernel.org
> > Signed-off-by: Ran Wang <ran.wang_1@....com>
> > ---
> >  drivers/usb/dwc3/gadget.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 0c960a9..5b0f02f 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -2491,7 +2491,7 @@ static int
> > dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
> >
> >  	req->request.actual = req->request.length - req->remaining;
> >
> > -	if (!dwc3_gadget_ep_request_completed(req) ||
> > +	if (!dwc3_gadget_ep_request_completed(req) &&
> >  			req->num_pending_sgs) {
> >  		__dwc3_gadget_kick_transfer(dep);
> >  		goto out;
> >
> 
> Thanks & Regards,
>  Tejas Joglekar