[Xen-changelog] [xen master] x86/gdbsx: security audit of {, un}pausevcpu and domstatus hypercalls

[patchbot@xxxxxxx]

commit a7d8ce78444dd8a5ee4d987745b6a4582ee43bf9
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri Jul 25 11:55:11 2014 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Jul 25 11:55:11 2014 +0200

    x86/gdbsx: security audit of {,un}pausevcpu and domstatus hypercalls
    
    XEN_DOMCTL_gdbsx_domstatus is already safe.  It loops at most over every 
vcpu
    in a domain and breaks at the first vcpu with an event pending, marking it 
as
    not-pending.
    
    XEN_DOMCTL_gdbsx_pausevcpu had an incorrect bounds check against the vcpu 
id,
    allowing an overflow of d->vcpu[] with an id between d->max_vcpus and
    MAX_VIRT_CPUS.  It was also able to overflow a vcpus pause count by many
    repeated hypercalls.
    
    The bounds check is fixed, and vcpu_pause() has been replaced with
    vcpu_pause_by_systemcontroller() which cuts out at 255 uses.
    
    XEN_DOMCTL_gdbsx_unpausevcpu suffered from the same bounds problems as its
    pause counterpart, and is fixed in exactly the same way.  Despite the
    atomic_read(&v->pause_count), this code didn't successfully prevent against 
an
    underflow of the vcpu pause count.
    
    The vcpu_unpause() has been replaced with vcpu_pause_by_systemcontroller()
    which correctly prevents against underflow.  The printk() is updated to 
have a
    proper guest logging level, and provide more useful information in the XSM
    case of one domain having debugger privileges over another.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Tim Deegan <tim@xxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 docs/misc/xsm-flask.txt |    3 ---
 xen/arch/x86/domctl.c   |   16 ++++++++--------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 31b9d27..9559028 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -96,9 +96,6 @@ __HYPERVISOR_domctl (xen/include/public/domctl.h)
  * XEN_DOMCTL_set_broken_page_p2m
  * XEN_DOMCTL_setnodeaffinity
  * XEN_DOMCTL_gdbsx_guestmemio
- * XEN_DOMCTL_gdbsx_pausevcpu
- * XEN_DOMCTL_gdbsx_unpausevcpu
- * XEN_DOMCTL_gdbsx_domstatus
 
 __HYPERVISOR_sysctl (xen/include/public/sysctl.h)
 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 243f42f..d1517c4 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1030,11 +1030,10 @@ long arch_do_domctl(
         if ( !d->controller_pause_count )
             break;
         ret = -EINVAL;
-        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
+        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
              (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) == NULL )
             break;
-        vcpu_pause(v);
-        ret = 0;
+        ret = vcpu_pause_by_systemcontroller(v);
     }
     break;
 
@@ -1046,13 +1045,14 @@ long arch_do_domctl(
         if ( !d->controller_pause_count )
             break;
         ret = -EINVAL;
-        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
+        if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
              (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) == NULL )
             break;
-        if ( !atomic_read(&v->pause_count) )
-            printk("WARN: Unpausing vcpu:%d which is not paused\n", 
v->vcpu_id);
-        vcpu_unpause(v);
-        ret = 0;
+        ret = vcpu_unpause_by_systemcontroller(v);
+        if ( ret == -EINVAL )
+            printk(XENLOG_G_WARNING
+                   "WARN: d%d attempting to unpause %pv which is not paused\n",
+                   current->domain->domain_id, v);
     }
     break;
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

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