Boris Ostrovsky
2012-Aug-14 16:37 UTC
[PATCH] acpi: Make sure valid CPU is passed to do_pm_op()
# 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];
Jan Beulich
2012-Aug-14 16:55 UTC
Re: [PATCH] acpi: Make sure valid CPU is passed to do_pm_op()
>>> 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; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Boris Ostrovsky
2012-Aug-14 17:04 UTC
Re: [PATCH] acpi: Make sure valid CPU is passed to do_pm_op()
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; > } > > > >