[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [Patch v2 3/3] xen/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> CC: Keir Fraser <keir@xxxxxxx> CC: Jan Beulich <JBeulich@xxxxxxxx> CC: Tim Deegan <tim@xxxxxxx> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> CC: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> --- v2: (re)correct vpu id bounds check --- 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; -- 1.7.10.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |