Re: [PATCH] acpi: Make sure valid CPU is passed to do_pm_op()

[Boris Ostrovsky <boris.ostrovsky@amd.com>]

On 08/14/2012 12:55 PM, Jan Beulich wrote:
>>>> On 14.08.12 at 18:37, Boris Ostrovsky <boris.ostrovsky@amd.com> wrote:
>> acpi: Make sure valid CPU is passed to do_pm_op()
>>
>> Passing invalid CPU value to do_pm_op() will cause assertion
>> in cpu_online().
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
>
> I'd like to propose the below extension to you change instead.

Yes, thanks, I didn't notice these.

-boris


>
> Jan
>
> Subject: acpi: Make sure valid CPU is passed to do_pm_op()
>
> Passing invalid CPU value to do_pm_op() will cause assertion
> in cpu_online().
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
>
> Such checks would, at a first glance, then also be missing at the top
> of various helper functions, but these check really were already
> redundant with the check in do_pm_op(). Remove the redundant checks
> for clarity and brevity.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -201,8 +201,6 @@ static int get_cpufreq_para(struct xen_s
>       struct list_head *pos;
>       uint32_t cpu, i, j = 0;
>
> -    if ( !op || !cpu_online(op->cpuid) )
> -        return -EINVAL;
>       pmpt = processor_pminfo[op->cpuid];
>       policy = per_cpu(cpufreq_cpu_policy, op->cpuid);
>
> @@ -305,9 +303,6 @@ static int set_cpufreq_gov(struct xen_sy
>   {
>       struct cpufreq_policy new_policy, *old_policy;
>
> -    if ( !op || !cpu_online(op->cpuid) )
> -        return -EINVAL;
> -
>       old_policy = per_cpu(cpufreq_cpu_policy, op->cpuid);
>       if ( !old_policy )
>           return -EINVAL;
> @@ -326,8 +321,6 @@ static int set_cpufreq_para(struct xen_s
>       int ret = 0;
>       struct cpufreq_policy *policy;
>
> -    if ( !op || !cpu_online(op->cpuid) )
> -        return -EINVAL;
>       policy = per_cpu(cpufreq_cpu_policy, op->cpuid);
>
>       if ( !policy || !policy->governor )
> @@ -404,22 +397,12 @@ static int set_cpufreq_para(struct xen_s
>       return ret;
>   }
>
> -static int get_cpufreq_avgfreq(struct xen_sysctl_pm_op *op)
> -{
> -    if ( !op || !cpu_online(op->cpuid) )
> -        return -EINVAL;
> -
> -    op->u.get_avgfreq = cpufreq_driver_getavg(op->cpuid, USR_GETAVG);
> -
> -    return 0;
> -}
> -
>   int do_pm_op(struct xen_sysctl_pm_op *op)
>   {
>       int ret = 0;
>       const struct processor_pminfo *pmpt;
>
> -    if ( !op || !cpu_online(op->cpuid) )
> +    if ( !op || op->cpuid >= nr_cpu_ids || !cpu_online(op->cpuid) )
>           return -EINVAL;
>       pmpt = processor_pminfo[op->cpuid];
>
> @@ -455,7 +438,7 @@ int do_pm_op(struct xen_sysctl_pm_op *op
>
>       case GET_CPUFREQ_AVGFREQ:
>       {
> -        ret = get_cpufreq_avgfreq(op);
> +        op->u.get_avgfreq = cpufreq_driver_getavg(op->cpuid, USR_GETAVG);
>           break;
>       }
>
>
>
>