Re: [PATCH] cpuidle: Avoid possible NULL pointer dereference in __cpuidle_register_device()

[Daniel Lezcano <daniel.lezcano@linaro.org>]

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@linux.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