linux-kernel - Re: [PATCHv4 6/7] hwspinlock/omap: enable module before reading SYSSTATUS register

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: <20140114131052.GA10818@saruman.home>
Date:	Tue, 14 Jan 2014 07:10:52 -0600
From:	Felipe Balbi <balbi@...com>
To:	Suman Anna <s-anna@...com>
CC:	Ohad Ben-Cohen <ohad@...ery.com>,
	Mark Rutland <mark.rutland@....com>,
	Tony Lindgren <tony@...mide.com>,
	Kumar Gala <galak@...eaurora.org>,
	<linux-kernel@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	<devicetree@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCHv4 6/7] hwspinlock/omap: enable module before reading
 SYSSTATUS register

On Mon, Jan 13, 2014 at 06:19:23PM -0600, Suman Anna wrote:
> The number of hwspinlocks are determined based on the value read
> from the IP block's SYSSTATUS register. However, the module may
> not be enabled and clocked, and the read may result in a bus error.
> 
> This particular issue is seen rather easily on AM33XX, since the
> module wakeup is software controlled, and it is disabled out of
> reset. Make sure the module is enabled and clocked before reading
> the SYSSTATUS register.
> 
> Signed-off-by: Suman Anna <s-anna@...com>
> ---
>  drivers/hwspinlock/omap_hwspinlock.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c
> index 9f56fb2..194886e 100644
> --- a/drivers/hwspinlock/omap_hwspinlock.c
> +++ b/drivers/hwspinlock/omap_hwspinlock.c
> @@ -101,10 +101,23 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
>  	if (!io_base)
>  		return -ENOMEM;
>  
> +	/*
> +	 * make sure the module is enabled and clocked before reading
> +	 * the module SYSSTATUS register
> +	 */
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
> +
>  	/* Determine number of locks */
>  	i = readl(io_base + SYSSTATUS_OFFSET);
>  	i >>= SPINLOCK_NUMLOCKS_BIT_OFFSET;
>  
> +	/*
> +	 * runtime PM will make sure the clock of this module is
> +	 * enabled again iff at least one lock is requested
> +	 */
> +	pm_runtime_put(&pdev->dev);

there is a small race here (which was already present previously) where
you could return from probe() in a failure case before your PM runtime
put request then you would disable pm_runtime while the device was still
alive.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)