* [PATCH] acpi: Make sure valid CPU is passed to do_pm_op()
@ 2012-08-14 16:37 Boris Ostrovsky
2012-08-14 16:55 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Boris Ostrovsky @ 2012-08-14 16:37 UTC (permalink / raw)
To: xen-devel; +Cc: boris.ostrovsky
# HG changeset patch
# User Boris Ostrovsky <boris.ostrovsky@amd.com>
# Date 1344962129 -7200
# Node ID 4ebf248d3aa1423da340d6900dd5f21072e519b3
# Parent 33d596f46521ea852e90cf6dbdbf3680d104134c
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>
diff -r 33d596f46521 -r 4ebf248d3aa1 xen/drivers/acpi/pmstat.c
--- a/xen/drivers/acpi/pmstat.c Mon Aug 13 18:09:33 2012 +0100
+++ b/xen/drivers/acpi/pmstat.c Tue Aug 14 18:35:29 2012 +0200
@@ -419,7 +419,7 @@ 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];
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] acpi: Make sure valid CPU is passed to do_pm_op()
2012-08-14 16:37 [PATCH] acpi: Make sure valid CPU is passed to do_pm_op() Boris Ostrovsky
@ 2012-08-14 16:55 ` Jan Beulich
2012-08-14 17:04 ` Boris Ostrovsky
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2012-08-14 16:55 UTC (permalink / raw)
To: boris.ostrovsky; +Cc: xen-devel
[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]
>>> 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.
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;
}
[-- Attachment #2: x86-acpi-pm-op-cpu-check.patch --]
[-- Type: text/plain, Size: 2304 bytes --]
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;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] acpi: Make sure valid CPU is passed to do_pm_op()
2012-08-14 16:55 ` Jan Beulich
@ 2012-08-14 17:04 ` Boris Ostrovsky
0 siblings, 0 replies; 3+ messages in thread
From: Boris Ostrovsky @ 2012-08-14 17:04 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
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;
> }
>
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-08-14 17:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-14 16:37 [PATCH] acpi: Make sure valid CPU is passed to do_pm_op() Boris Ostrovsky
2012-08-14 16:55 ` Jan Beulich
2012-08-14 17:04 ` Boris Ostrovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).