Re: [Xen-devel] [Patch v2 3/3] xen/gdbsx: Security audit of {, un}pausevcpu and domstatus hypercalls

[Tim Deegan <tim@xxxxxxx>]

At 12:16 +0100 on 24 Jul (1406200565), Andrew Cooper wrote:
> 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>

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