xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: [PATCH 2/2] x86/cpufreq: Avoid using processor_pminfo[cpu] when it is NULL
Date: Fri, 12 Aug 2016 11:35:19 +0100	[thread overview]
Message-ID: <1470998119-14965-2-git-send-email-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <1470998119-14965-1-git-send-email-andrew.cooper3@citrix.com>

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 ]----
  <snip>
  (XEN)    [<ffff82d0801c4231>] cpufreq_add_cpu+0x161/0xdc0
  (XEN)    [<ffff82d0801c6610>] cpufreq.c#cpu_callback+0x20/0x30
  (XEN)    [<ffff82d0804eefad>] cpufreq.c#cpufreq_presmp_init+0x2d/0x50
  (XEN)    [<ffff82d0804c5942>] do_presmp_initcalls+0x22/0x30
  (XEN)    [<ffff82d08051852d>] __start_xen+0x378d/0x42f0
  (XEN)    [<ffff82d080100073>] __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 <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 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 = &processor_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 = &processor_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 = &processor_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 = &processor_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 = &processor_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 = &processor_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(&cpu_nfb, CPU_ONLINE, cpu);
     register_cpu_notifier(&cpu_nfb);
     return 0;
 }
-- 
2.1.4


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

  reply	other threads:[~2016-08-12 10:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-12 10:35 [PATCH 1/2] x86/boot: Align e820 and video data in the boot trampoline Andrew Cooper
2016-08-12 10:35 ` Andrew Cooper [this message]
2016-08-12 12:14   ` [PATCH 2/2] x86/cpufreq: Avoid using processor_pminfo[cpu] when it is NULL Jan Beulich
2016-08-12 12:08 ` [PATCH 1/2] x86/boot: Align e820 and video data in the boot trampoline Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1470998119-14965-2-git-send-email-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).