linux-kernel - RE: [PATCH] cpuidle: Check cpuidle driver before add refcount

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:	Fri, 30 Aug 2013 20:26:34 +0800
From:	Daniel Fu <danifu@...dia.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
CC:	"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Johnny Qiu <joqiu@...dia.com>
Subject: RE: [PATCH] cpuidle: Check cpuidle driver before add refcount

X-NVConfidentiality: public

In SMP, When doing the cpuidle driver registration. There maybe only 1 cpu have register cpuidle(CPU0), 
but In the process of registration cpuidle driver, the scheduler maybe schedule the process to the other CPU (eg. CPU0 have heay load, migrate to CPU1) 
cpuidle_get_driver() --> get_cpu() , will get CPU1, if CPU1 didn't register cpuidle driver, Will get NULL.

I know we should prevent the registration migrate to the other CPU, but we should check drv before add refcount to prevent kernel panic at least, Right?

Best Regards,
Daniel

-----Original Message-----
From: Rafael J. Wysocki [mailto:rjw@...k.pl] 
Sent: 2013年8月30日 20:01
To: Daniel Fu
Cc: daniel.lezcano@...aro.org; linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org; Johnny Qiu
Subject: Re: [PATCH] cpuidle: Check cpuidle driver before add refcount

On Friday, August 30, 2013 07:48:22 PM Daniel Fu wrote:
> If the current CPU has no cpuidle driver, drv will be NULL.
> Check if we get drv successfully before add refount to prevent Kernel 
> panic.

What is the actual scenario that may lead to this panic?

Rafael


> Signed-off-by: Daniel Fu <danifu@...dia.com>
> ---
>  drivers/cpuidle/driver.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c index 
> 3ac499d..6e11701 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -331,7 +331,8 @@ struct cpuidle_driver *cpuidle_driver_ref(void)
>  	spin_lock(&cpuidle_driver_lock);
>  
>  	drv = cpuidle_get_driver();
> -	drv->refcnt++;
> +	if (drv)
> +		drv->refcnt++;
>  
>  	spin_unlock(&cpuidle_driver_lock);
>  	return drv;
> 
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.