>>> On 16.07.18 at 10:30, <[email protected]> wrote: > On 07/02/2018 08:48 AM, Tian, Kevin wrote: >>> From: Razvan Cojocaru [mailto:[email protected]] >>> Sent: Thursday, June 28, 2018 10:35 PM >>> >>> A VM exit handler executed immediately after enabling #VE might >>> find a stale __vmsave()d EPTP_INDEX, stored by calling >>> altp2m_vcpu_destroy() when >>> SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS >>> had been enabled by altp2m_vcpu_update_vmfunc_ve(). >>> >>> vmx_vmexit_handler() __vmread()s EPTP_INDEX as soon as >>> SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS is set, so if an >>> application enables altp2m on a domain, succesfully calls >>> xc_altp2m_set_vcpu_enable_notify(), then disables altp2m and >>> exits, a second run of said application will likely read the >>> INVALID_ALTP2M EPTP_INDEX set when disabling altp2m in the first >>> run, and crash the host with the BUG_ON(idx >= MAX_ALTP2M), >>> between xc_altp2m_set_vcpu_enable_notify() and >>> xc_altp2m_set_domain_state(..., false). >>> >>> The problem is not restricted to an INVALID_ALTP2M EPTP_INDEX >>> (which can only sanely happen on altp2m uninit), but applies >>> to any stale index previously saved - which means that all >>> altp2m_vcpu_update_vmfunc_ve() calls must also call >>> altp2m_vcpu_update_p2m() after setting >>> SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS, in order to make sure >>> that the stored EPTP_INDEX is always valid at >>> vmx_vmexit_handler() time. >>> >>> I don't however fold the two functions into one everywhere, >>> since in p2m_switch_domain_altp2m_by_id() and >>> p2m_switch_vcpu_altp2m_by_id() the extra work done by >>> altp2m_vcpu_update_vmfunc_ve() is unnecessary and has side >>> effects (such as __vmwrite(VM_FUNCTION_CONTROL, ...)). >>> >>> Signed-off-by: Razvan Cojocaru <[email protected]> >>> >> >> Reviewed-by: Kevin Tian <[email protected]>. >> >> btw next time please use more descriptive words and less >> function names in commit message. > > Fair enough, thanks for the review! Is further action required for this > patch to go in (perhaps George's ack)?
George's ack, indeed. Jan _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
