Re: [Xen-devel] [PATCH] vm_event: make sure the domain is paused in key domctls

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jan 28, 2016 at 2:05 PM, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> wrote:

On 01/28/2016 10:09 PM, Tamas K Lengyel wrote:
> On Thu, Jan 28, 2016 at 6:52 AM, Razvan Cojocaru
> <rcojocaru@xxxxxxxxxxxxxxx <mailto:rcojocaru@xxxxxxxxxxxxxxx>> wrote:
>
>Â Â ÂThis patch pauses the domain for all writes through the 'ad'
>Â Â Âpointer in monitor_domctl(), defers a domain_unpause() call until
>Â Â Âafter the CRs are updated for the MONITOR_EVENT_WRITE_CTRLREG
>Â Â Âcase, and makes sure that the domain is paused for both vm_event
>Â Â Âenable and disable cases in vm_event_domctl().
>Â Â ÂThanks go to Andrew Cooper for his review and suggestions.
>
>
> For vm_event_enable the domain is already paused by libxc before the
> domctl is issued. I don't see a problem in doing another pause in Xen,
> but given we have XSA-99, just doing this pause in Xen would not be
> enough. So is it really necessary/fixes anything?

This isn't about XSA-99, the problem here is related to my previous
patch "x86 vm_event: reset monitor in vm_event_cleanup_domain()". While
that improves matters and greatly reduces the chances of crashes due to
hvm_msr_write_intercept() or hvm_set_crX() dereferencing a NULL
v->arch.vm_event that's assumed to be OK, when the corresponding
v->domain->arch.monitor is non-zero, the foolproof way is to make sure
that functions such as vm_event_cleanup_domain() are always being called
only while the domain has been paused. So there should be a
domain_pause() call somewhere on the call path before that.

Sure, but that's the disable case. I was only wondering about the enable case where the domain is already paused.

Tamas

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