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

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 07/20/2018 08:18 PM, George Dunlap wrote:
> Furthermore, imagine the following scenario:
> 
> * dom0 enables altp2m on domain A
> * dom0 switches altp2m to view 1 on domain A
> * dom0 enables #VE on domain A
> * domain A has a vmexit
>   -> At this point, EPTP_INDEX is 0, so the vmexit code will drop a
> reference on altp2m index 1 and increase the reference count on altp2m
> index 0 #
> 
> My patch fixes the above issue, but your patch doesn't (AFAICT).  What
> altp2m_vcpu_destroy() did wasn't fundamentally buggy; it just
> highlighted the issue by doing the equivalent of putting 0xDEADBEEF in
> EPTP_INDEX; and what your patch did was to reverse that, by making
> EPTP_INDEX accidentally correct again the next time you ran your test.
> 
> (Let me know if I'm wrong about that!)

I do prefer your patch, but unless I'm missing something my patch is
doing the same thing (albeit in a slightly more convoluted manner): it's
calling altp2m_vcpu_update_p2m(v) _inside_
altp2m_vcpu_update_vmfunc_ve(v). That's all it does, other than removing
the (now redundant) explicit altp2m_vcpu_update_p2m(v) call from
altp2m_vcpu_destroy():

https://lists.xenproject.org/archives/html/xen-devel/2018-06/msg01898.html

So for every hvm_funcs.altp2m_vcpu_update_vmfunc_ve(v) (i.e. the vmx.c
function) that gets called, I also call altp2m_vcpu_update_p2m(v), which
properly sets EPTP_INDEX (just as your patch does by __vmwrite()ing it
directly in vmx_vcpu_update_vmfunc_ve(), but in a roundabout manner).

Did I misunderstand something?


Thanks,
Razvan

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