* 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