Re: [Xen-devel] [PATCH V5] x86/altp2m: Fix crash with INVALID_ALTP2M EPTP index

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.06.18 at 16:35, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> 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 <rcojocaru@xxxxxxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel