Re: [Xen-devel] [PATCH v2] x86/vmx: correctly gather gs_shadow value for current vCPU

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 5/2/19 11:45 AM, Andrew Cooper wrote:
> On 02/05/2019 07:20, Razvan Cojocaru wrote:
> > On 5/2/19 2:52 AM, Tamas K Lengyel wrote:
> > > Currently the gs_shadow value is only cached when the vCPU is being scheduled
> > > out by Xen. Reporting this (usually) stale value through vm_event is incorrect,
> > > since it doesn't represent the actual state of the vCPU at the time the event
> > > was recorded. This prevents vm_event subscribers from correctly finding kernel
> > > structures in the guest when it is trapped while in ring3.
> > >
> > > Refresh shadow_gs value when the context being saved is for the current vCPU.
> > >
> > > Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > > Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> > > Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > > ---
> > > v2: move fix to hvm so vm_event doesn't have to know specifics
> > > ---
> > >   xen/arch/x86/hvm/vmx/vmx.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > >
> > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> > > index 283eb7b34d..5154ecc2a8 100644
> > > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > > @@ -779,12 +779,17 @@ static void vmx_load_cpu_state(struct vcpu *v, struct 
> > > hvm_hw_cpu *data)
> > >   
> > >   static void vmx_save_vmcs_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)
> > >   {
> > > +    if ( v == current )
> > > +        vmx_save_guest_msrs(v);
> > vmx_save_guest_msrs() is simple enough that the if is probably not
> > necessary here (we can just call the function directly, regardless of
> > what v holds).
> Why?  Doing that would fully corrupt v's state when called in remote
> context, as you'd clobber v's gs_shadow which whatever the value was
> from the current vcpu.
Good point, I've missed that.


Thanks,
Razvan

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