linux-kernel - RE: [PATCH] usb: dwc2: 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]

Message-ID: <A2CA0424C0A6F04399FB9E1CD98E0304845072F6@US01WEMBX2.internal.synopsys.com>
Date:	Tue, 13 Jan 2015 21:12:27 +0000
From:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>
To:	Robert Baldyga <r.baldyga@...sung.com>
CC:	"balbi@...com" <balbi@...com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dinguyen@...nsource.altera.com" <dinguyen@...nsource.altera.com>,
	"yousaf.kaukab@...el.com" <yousaf.kaukab@...el.com>,
	"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>
Subject: RE: [PATCH] usb: dwc2: call dwc2_is_controller_alive() under
 spinlock

> From: Robert Baldyga [mailto:r.baldyga@...sung.com]
> Sent: Tuesday, January 13, 2015 2:58 AM
> 
> This patch fixes bug described here:
> https://lkml.org/lkml/2014/12/22/185
> 
> Signed-off-by: Robert Baldyga <r.baldyga@...sung.com>
> ---
>  drivers/usb/dwc2/core_intr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index ad43c5b..668c8dd 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -476,13 +476,13 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
>  	u32 gintsts;
>  	irqreturn_t retval = IRQ_NONE;
> 
> +	spin_lock(&hsotg->lock);
> +
>  	if (!dwc2_is_controller_alive(hsotg)) {
>  		dev_warn(hsotg->dev, "Controller is dead\n");
>  		goto out;

This isn't right, now the spinlock isn't released if we take this path.

Besides, this patch doesn't really make sense. How could taking the
spinlock affect the value returned from dwc2_is_controller_alive? All
it does is read from the GSNPSID register, and that register is never
written to.

Are you absolutely sure this is the fix?

-- 
Paul

>  	}
> 
> -	spin_lock(&hsotg->lock);
> -
>  	gintsts = dwc2_read_common_intr(hsotg);
>  	if (gintsts & ~GINTSTS_PRTINT)
>  		retval = IRQ_HANDLED;
> --
> 1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/