linux-kernel - Re: [PATCH] usb: dwc2: hcd: call dwc2_is_controller

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:   Thu, 14 Feb 2019 11:43:46 +0000
From:   Minas Harutyunyan <minas.harutyunyan@...opsys.com>
To:     Jaehoon Chung <jh80.chung@...sung.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
CC:     "minas.harutyunyan@...opsys.com" <minas.harutyunyan@...opsys.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "m.szyprowski@...sung.com" <m.szyprowski@...sung.com>
Subject: Re: [PATCH] usb: dwc2: hcd: call dwc2_is_controller_alive under
 spinlock

Hi Jaehoon Chung,

On 2/14/2019 2:04 PM, Jaehoon Chung wrote:
> This patch is referred to Robert's patch
> commit cf54772b913b ("usb: dwc2: call dwc2_is_controller_alive() under spinlock")
> 
> During running sdb with otg mode, the usb is hung sometime.
> 
> The one of SDB hang issues should be fixed with this patch.
> After hang, it doesn't never trigger the usb interrupt.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@...sung.com>
> ---
>   drivers/usb/dwc2/hcd_intr.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> index 88b5dcf..8d3b155 100644
> --- a/drivers/usb/dwc2/hcd_intr.c
> +++ b/drivers/usb/dwc2/hcd_intr.c
> @@ -2222,13 +2222,13 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg)
>   	u32 gintsts, dbg_gintsts;
>   	irqreturn_t retval = IRQ_NONE;
>   
> +	spin_lock(&hsotg->lock);
> +
>   	if (!dwc2_is_controller_alive(hsotg)) {
>   		dev_warn(hsotg->dev, "Controller is dead\n");
> -		return retval;
> +		goto out;
>   	}
>   
> -	spin_lock(&hsotg->lock);
> -
>   	/* Check if HOST Mode */
>   	if (dwc2_is_host_mode(hsotg)) {
>   		gintsts = dwc2_read_core_intr(hsotg);
> @@ -2276,6 +2276,7 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg)
>   		}
>   	}
>   
> +out:
>   	spin_unlock(&hsotg->lock);
>   
>   	return retval;
> 

1. Checking core alive or not was introduced in our internal reference 
driver for our setups, because sometime AHB-PCI bridge can hung, not 
core itself. If it happen then only way to overcome it - reboot core 
setup and PC.
2. Actually this issue was fixed on our setups and currently no need to 
check access via bridge to core.
3. Any case if it still happen on some platforms then first need to 
check and fix HW.

On asserted any interrupt for dwc2, handlers call sequence is follow: 
common->gadget->host handlers. If core alive checked in common handler 
then checking same stuff in host handler again not needed at all.

So, core alive checking in host handler create issue for your test, I 
suggest to not fix alive checking in host interrupt handler as you 
suggested and done in common handler, but just remove it.

Thanks,
Minas