linux-kernel - Re: [PATCH] cpuidle: Avoid possible NULL pointer dereference in __cpuidle_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: <4F79FE8F.1040708@linaro.org>
Date:	Mon, 02 Apr 2012 21:31:27 +0200
From:	Daniel Lezcano <daniel.lezcano@...aro.org>
To:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
CC:	lenb@...nel.org, khilman@...com, deepthi@...ux.vnet.ibm.com,
	g.trinabh@...il.com, arjan@...radead.org,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
	amit.kucheria@...aro.org
Subject: Re: [PATCH] cpuidle: Avoid possible NULL pointer dereference in __cpuidle_register_device()

On 04/02/2012 04:44 PM, Srivatsa S. Bhat wrote:
> In __cpuidle_register_device(), "dev->cpu" is used before checking if dev is
> non-NULL. Fix it.
>
> Signed-off-by: Srivatsa S. Bhat<srivatsa.bhat@...ux.vnet.ibm.com>
> ---

That should be fixed at the caller level. Usually, static function does 
not check the function parameters, it is up to the exported function to 
do that. It is supposed the static functions are called with valid 
parameters.

There are two callers for __cpuidle_register_device:
  * cpuidle_register_device
  * cpuidle_enable_device

Both of them do not check 'dev' is a valid parameter. They should as 
they are exported and could be used by an external module. IMHO, BUG_ON 
could be used here if dev == NULL.


>   drivers/cpuidle/cpuidle.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 87411ce..75b381e 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -372,7 +372,7 @@ EXPORT_SYMBOL_GPL(cpuidle_disable_device);
>   static int __cpuidle_register_device(struct cpuidle_device *dev)
>   {
>   	int ret;
> -	struct device *cpu_dev = get_cpu_device((unsigned long)dev->cpu);
> +	struct device *cpu_dev;
>   	struct cpuidle_driver *cpuidle_driver = cpuidle_get_driver();
>
>   	if (!dev)
> @@ -380,6 +380,7 @@ static int __cpuidle_register_device(struct cpuidle_device *dev)
>   	if (!try_module_get(cpuidle_driver->owner))
>   		return -EINVAL;
>
> +	cpu_dev = get_cpu_device((unsigned long)dev->cpu);
>   	init_completion(&dev->kobj_unregister);
>
>   	per_cpu(cpuidle_devices, dev->cpu) = dev;
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
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/