Re: [Xen-devel] [PATCH 2/2] x86/cpufreq: Avoid using processor_pminfo[cpu] when it is NULL

[Jan Beulich]

>>> On 12.08.16 at 12:35,  wrote:
> The undefined behaviour sanitiser shows that it really is NULL via the
> pre_initcall path.
> 
>   (XEN) 
> 
>   (XEN) UBSAN: Undefined behaviour in cpufreq.c:158:66
>   (XEN) member access within null pointer of type 'struct processor_pminfo'
>   (XEN) [ Xen-4.8-unstable  x86_64  debug=y  Not tainted ]
>   
>   (XEN)[] cpufreq_add_cpu+0x161/0xdc0
>   (XEN)[] cpufreq.c#cpu_callback+0x20/0x30
>   (XEN)[] cpufreq.c#cpufreq_presmp_init+0x2d/0x50
>   (XEN)[] do_presmp_initcalls+0x22/0x30
>   (XEN)[] __start_xen+0x378d/0x42f0
>   (XEN)[] __high_start+0x53/0x60
> 
> Fix two other occurances of the same buggy logic.
> 
> The processor_pminfo[] objects are only allocated as a result of
> XENPF_set_processor_pminfo hypercalls, which means that this early cpu
> callback will always hit the early NULL check, and is therefore pointless.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/2] x86/cpufreq: Avoid using processor_pminfo[cpu] when it is NULL

[Andrew Cooper]

The undefined behaviour sanitiser shows that it really is NULL via the
pre_initcall path.

  (XEN) 

  (XEN) UBSAN: Undefined behaviour in cpufreq.c:158:66
  (XEN) member access within null pointer of type 'struct processor_pminfo'
  (XEN) [ Xen-4.8-unstable  x86_64  debug=y  Not tainted ]
  
  (XEN)[] cpufreq_add_cpu+0x161/0xdc0
  (XEN)[] cpufreq.c#cpu_callback+0x20/0x30
  (XEN)[] cpufreq.c#cpufreq_presmp_init+0x2d/0x50
  (XEN)[] do_presmp_initcalls+0x22/0x30
  (XEN)[] __start_xen+0x378d/0x42f0
  (XEN)[] __high_start+0x53/0x60

Fix two other occurances of the same buggy logic.

The processor_pminfo[] objects are only allocated as a result of
XENPF_set_processor_pminfo hypercalls, which means that this early cpu
callback will always hit the early NULL check, and is therefore pointless.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/drivers/cpufreq/cpufreq.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index f19b403..fd82ef5 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -126,7 +126,7 @@ int __init cpufreq_register_governor(struct 
cpufreq_governor *governor)
 
 int cpufreq_limit_change(unsigned int cpu)
 {
-struct processor_performance *perf = _pminfo[cpu]->perf;
+struct processor_performance *perf;
 struct cpufreq_policy *data;
 struct cpufreq_policy policy;
 
@@ -134,6 +134,8 @@ int cpufreq_limit_change(unsigned int cpu)
 !processor_pminfo[cpu])
 return -ENODEV;
 
+perf = _pminfo[cpu]->perf;
+
 if (perf->platform_limit >= perf->state_count)
 return -EINVAL;
 
@@ -155,12 +157,15 @@ int cpufreq_add_cpu(unsigned int cpu)
 struct cpufreq_dom *cpufreq_dom = NULL;
 struct cpufreq_policy new_policy;
 struct cpufreq_policy *policy;
-struct processor_performance *perf = _pminfo[cpu]->perf;
+struct processor_performance *perf;
 
 /* to protect the case when Px was not controlled by xen */
-if (!processor_pminfo[cpu]  ||
-!(perf->init & XEN_PX_INIT) ||
-!cpu_online(cpu))
+if ( !processor_pminfo[cpu] || !cpu_online(cpu) )
+return -EINVAL;
+
+perf = _pminfo[cpu]->perf;
+
+if ( !(perf->init & XEN_PX_INIT) )
 return -EINVAL;
 
 if (!cpufreq_driver)
@@ -310,12 +315,15 @@ int cpufreq_del_cpu(unsigned int cpu)
 struct list_head *pos;
 struct cpufreq_dom *cpufreq_dom = NULL;
 struct cpufreq_policy *policy;
-struct processor_performance *perf = _pminfo[cpu]->perf;
+struct processor_performance *perf;
 
 /* to protect the case when Px was not controlled by xen */
-if (!processor_pminfo[cpu]  ||
-!(perf->init & XEN_PX_INIT) ||
-!cpu_online(cpu))
+if ( !processor_pminfo[cpu] || !cpu_online(cpu) )
+return -EINVAL;
+
+perf = _pminfo[cpu]->perf;
+
+if ( !(perf->init & XEN_PX_INIT) )
 return -EINVAL;
 
 if (!per_cpu(cpufreq_cpu_policy, cpu))
@@ -637,8 +645,6 @@ static struct notifier_block cpu_nfb = {
 
 static int __init cpufreq_presmp_init(void)
 {
-void *cpu = (void *)(long)smp_processor_id();
-cpu_callback(_nfb, CPU_ONLINE, cpu);
 register_cpu_notifier(_nfb);
 return 0;
 }
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel