Re: [Xen-devel] [PATCH V3 1/6] x86/xsaves: enable xsaves/xrstors for pv guest

[Shuai Ruan <shuai.ruan@xxxxxxxxxxxxxxx>]

On Tue, Aug 11, 2015 at 11:24:24AM +0100, Andrew Cooper wrote:
> On 11/08/15 08:50, Shuai Ruan wrote:
> > On Fri, Aug 07, 2015 at 01:44:41PM +0100, Andrew Cooper wrote:
> >> On 07/08/15 09:00, Shuai Ruan wrote:
> >>>>> +                    goto skip;
> >>>>> +                }
> >>>>> +
> >>>>> +                if ( !guest_kernel_mode(v, regs) || (regs->edi & 0x3f) 
> >>>>> )
> >>>> What does edi have to do with xsaves?  only edx:eax are special
> >>>> according to the manual.
> >>>>
> >>> regs->edi is the guest_linear_address
> >> Whyso?  xsaves takes an unconditional memory parameter,  not a pointer
> >> in %rdi.  (regs->edi is only correct for ins/outs because the pointer is
> >> architecturally required to be in %rdi.)
> > You are right. The linear_address should be decoded from the instruction.
> >> There is nothing currently in emulate_privileged_op() which does ModRM
> >> decoding for memory references, nor SIB decoding.  xsaves/xrstors would
> >> be the first such operations.
> >>
> >> I am also not sure that adding arbitrary memory decode here is sensible.
> >>
> >> In an ideal world, we would have what is currently x86_emulate() split
> >> in 3 stages.
> >>
> >> Stage 1 does straight instruction decode to some internal representation.
> >>
> >> Stage 2 does an audit to see whether the decoded instruction is
> >> plausible for the reason why an emulation was needed.  We have had a
> >> number of security issues with emulation in the past where guests cause
> >> one instruction to trap for emulation, then rewrite the instruction to
> >> be something else, and exploit a bug in the emulator.
> >>
> >> Stage 3 performs the actions required for emulation.
> >>
> >> Currently, x86_emulate() is limited to instructions which might
> >> legitimately fault for emulation, but with the advent of VM
> >> introspection, this is proving to be insufficient.  With my x86
> >> maintainers hat on, I would like to avoid the current situation we have
> >> with multiple bits of code doing x86 instruction decode and emulation
> >> (which are all different).
> >>
> >> I think the 3-step approach above caters suitably to all usecases, but
> >> it is a large project itself.  It allows the introspection people to
> >> have a full and complete x86 emulation infrastructure, while also
> >> preventing areas like the shadow paging from being opened up to
> >> potential vulnerabilities in unrelated areas of the x86 architecture.
> >>
> >> I would even go so far as to say that it is probably ok not to support
> >> xsaves/xrestors in PV guests until something along the above lines is
> >> sorted.  The first feature in XSS is processor trace which a PV guest
> >> couldn't use anyway.  I suspect the same applies to most of the other
> > Why PV guest couldn't use precessor trace?
> 
> After more consideration, Xen should not expose xsaves/xrstors to PV
> guests at all.
> 
OK. In next version, I will follow your suggestion not to add support for PV 
guest.
> >> XSS features, or they wouldn't need to be privileged in the first place.
> >>
> > Thanks for your such detail suggestions.
> > For xsaves/xrstors would also bring other benefits for PV guest such as
> > saving memory of XSAVE area. If we do not support xsaves/xrstors in PV , 
> > PV guest would lose these benefits. What's your opinions toward this?
> 
> PV guests running under Xen are exactly the same as regular user
> processes running under Linux.
> 
> There is a reason everything covered by xsaves/xrstors is restricted to
> ring0; it would be a security hole to allow guests to configure the
> features themselves.
> 
> Features such as Processor Trace would need a hypercall interface for
> guests to use.
> 
Ok. Thanks. 
> ~Andrew
Thnaks for your review ,Andrew
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

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