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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 7 Jan 2020 10:15:35 +0000
From:   Tejas Joglekar <Tejas.Joglekar@...opsys.com>
To:     Ran Wang <ran.wang_1@....com>, Felipe Balbi <balbi@...nel.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        Tejas Joglekar <Tejas.Joglekar@...opsys.com>,
        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,
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()).

> 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://patchwork.kernel.org/patch/11292087/.
 
> [   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