[Xen-changelog] [xen master] x86/idle: Fix get_cpu_idle_time()'s interaction with offline pcpus

[patchbot@xxxxxxx]

commit 0aa27ce3351f7eb09d13e863a1d5f303086aa32a
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri Oct 4 12:23:23 2013 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Oct 4 12:23:23 2013 +0200

    x86/idle: Fix get_cpu_idle_time()'s interaction with offline pcpus
    
    Checking for "idle_vcpu[cpu] != NULL" is insufficient protection against
    offline pcpus.  From a hypercall, vcpu_runstate_get() will determine "v !=
    current", and try to take the vcpu_schedule_lock().  This will try to look 
up
    per_cpu(schedule_data, v->processor) and promptly suffer a NULL structure
    deference as v->processors' __per_cpu_offset is INVALID_PERCPU_AREA.
    
    One example might look like this:
    
    ...
    Xen call trace:
       [<ffff82c4c0126ddb>] vcpu_runstate_get+0x50/0x113
       [<ffff82c4c0126ec6>] get_cpu_idle_time+0x28/0x2e
       [<ffff82c4c012b5cb>] do_sysctl+0x3db/0xeb8
       [<ffff82c4c023280d>] compat_hypercall+0xbd/0x116
    
    Pagetable walk from 0000000000000040:
     L4[0x000] = 0000000186df8027 0000000000028207
     L3[0x000] = 0000000188e36027 00000000000261c9
     L2[0x000] = 0000000000000000 ffffffffffffffff
    
    ****************************************
    Panic on CPU 11:
    ...
    
    get_cpu_idle_time() has been updated to correctly deal with offline pcpus
    itself by returning 0, in the same way as it would if it was missing the
    idle_vcpu[] pointer.
    
    In doing so, XENPF_getidletime needed updating to correctly retain its
    described behaviour of clearing bits in the cpumap for offline pcpus.
    
    As this crash can only be triggered with toolstack hypercalls, it is not a
    security issue and just a simple bug.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Keir Fraser <keir@xxxxxxx>
---
 xen/arch/x86/platform_hypercall.c |    8 ++++++--
 xen/common/schedule.c             |    9 ++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/platform_hypercall.c 
b/xen/arch/x86/platform_hypercall.c
index 7175a82..2162811 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -355,10 +355,14 @@ ret_t 
do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
 
         for_each_cpu ( cpu, cpumap )
         {
-            if ( idle_vcpu[cpu] == NULL )
-                cpumask_clear_cpu(cpu, cpumap);
             idletime = get_cpu_idle_time(cpu);
 
+            if ( !idletime )
+            {
+                cpumask_clear_cpu(cpu, cpumap);
+                continue;
+            }
+
             if ( copy_to_guest_offset(idletimes, cpu, &idletime, 1) )
             {
                 ret = -EFAULT;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index a8398bd..1ddfb22 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -176,13 +176,12 @@ void vcpu_runstate_get(struct vcpu *v, struct 
vcpu_runstate_info *runstate)
 
 uint64_t get_cpu_idle_time(unsigned int cpu)
 {
-    struct vcpu_runstate_info state;
-    struct vcpu *v;
+    struct vcpu_runstate_info state = { 0 };
+    struct vcpu *v = idle_vcpu[cpu];
 
-    if ( (v = idle_vcpu[cpu]) == NULL )
-        return 0;
+    if ( cpu_online(cpu) && v )
+        vcpu_runstate_get(v, &state);
 
-    vcpu_runstate_get(v, &state);
     return state.time[RUNSTATE_running];
 }
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog