From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@amd.com>
Subject: Re: [PATCH] acpi: Make sure valid CPU is passed to
	do_pm_op()
Date: Tue, 14 Aug 2012 13:04:13 -0400
Message-ID: <502A850D.8050304@amd.com>
References: <4ebf248d3aa1423da340.1344962277@localhost.localdomain>
	<502A9F3C0200007800094E3E@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <502A9F3C0200007800094E3E@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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;
>       }
>
>
>
>