]> xenbits.xensource.com Git - people/dariof/xen.git/commitdiff
xen/domctl: Fix Xen heap leak via XEN_DOMCTL_getvcpucontext
authorAndrew Cooper <andrew.cooper3@citrix.com>
Sun, 8 Oct 2017 14:12:18 +0000 (15:12 +0100)
committerAndrew Cooper <andrew.cooper3@citrix.com>
Mon, 9 Oct 2017 11:43:21 +0000 (12:43 +0100)
The backing structure for XEN_DOMCTL_getvcpucontext is only zeroed in the x86
HVM case.  At the very least, this means that ARM returns junk through its
flags field (as it is only ever conditionally or'd into), and x86 PV leaks
data through gdt_frames[14...15].  (An exhaustive search for other leaks
hasn't been performed).

Unconditionally zero the memory upon allocation, and forgo the double clear
for x86 HVM.  These hypercalls are not on hotpaths.

Note that this does not qualify for an XSA.  Per XSA-77,
XEN_DOMCTL_getvcpucontext is unsafe for disaggregation, meaning that only the
control domain can use this hypercall.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Julien Grall <julien.grall@linaro.org>
xen/arch/x86/domctl.c
xen/common/domctl.c

index 540ba089d70e1e60cac29de2404374a4b19f3603..1b208f9f958baea76f0f32be8dc5a67c8e6f6602 100644 (file)
@@ -1530,8 +1530,6 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
     bool compat = is_pv_32bit_domain(d);
 #define c(fld) (!compat ? (c.nat->fld) : (c.cmp->fld))
 
-    if ( !is_pv_domain(d) )
-        memset(c.nat, 0, sizeof(*c.nat));
     memcpy(&c.nat->fpu_ctxt, v->arch.fpu_ctxt, sizeof(c.nat->fpu_ctxt));
     c(flags = v->arch.vgc_flags & ~(VGCF_i387_valid|VGCF_in_kernel));
     if ( v->fpu_initialised )
index d03bbf20a5883ad4c9fe5e53192a4e8f8f7aa4ae..3c6fa4ec67f1b37fe422a80d6bad871b5c75eae2 100644 (file)
@@ -869,7 +869,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
                      < sizeof(struct compat_vcpu_guest_context));
 #endif
         ret = -ENOMEM;
-        if ( (c.nat = xmalloc(struct vcpu_guest_context)) == NULL )
+        if ( (c.nat = xzalloc(struct vcpu_guest_context)) == NULL )
             goto getvcpucontext_out;
 
         vcpu_pause(v);