[Xen-devel] RE: [PATCH v2] Enable SMEP CPU feature support for XEN hypervisor

["Li, Xin" <xin.li@xxxxxxxxx>]

One last comment from Jan, he suggested to use
    page_user = (read_cr4() & X86_CR4_SMEP) ? _PAEG_USER : 0;
thanks!
-Xin

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@xxxxxxxxx] On Behalf Of Keir Fraser
> Sent: Saturday, June 04, 2011 3:22 AM
> To: Li, Xin
> Cc: xen-devel; Jan Beulich
> Subject: Re: [PATCH v2] Enable SMEP CPU feature support for XEN hypervisor
> 
> Hi Xen,
> 
> New patch attached and comments in-line.
> 
> On 03/06/2011 19:49, "Li, Xin" <xin.li@xxxxxxxxx> wrote:
> 
> >> 1. The initialisation in cpu/common.c is misguided. Features like
> >> set_in_cr4(), setup_clear_cpu_cap(), and cpu_has_xxx, are designed to allow
> >> the feature initialisation to happen close in code to where the feature is
> >> used. Hence I have moved the initialisation into traps.c:trap_init().
> >
> > It's the right way to move.
> >
> > But trap_init() is called before Xen does leaf 7.0 detection, thus we also
> > need to add leaf 7.0 detection in early_cpu_detect to get
> > boot_cpu_data.x86_capability[7] initialized before trap_init().
> 
> Oooo good point. Fixed in the attached patch by moving SMEP setup into
> setup.c, explicitly and immediately after identify_cpu(). I was actually in
> two minds whether to fix this by extending early_cpu_detect(). Overall I
> decided I don't want to grow e_c_d() if not absolutely necessary. If the CPU
> setup stuff in setup.c grows too big and ugly I'd rather refactor it another
> way.
> 
> >> 3. I have pushed interpretation of the pf_type enumeration out to the
> >> callers of spurious_page_fault(). This is because a SMEP fault while Xen is
> >> executing should *crash Xen*. So that's what I do. Further, when it is a
> >
> > I'm still wondering is it overkill to kill Xen?  An evil guest can crash 
> > Xen?
> 
> An evil guest has to penetrate Xen before it can crash it in this way. If
> Xen has been subverted, and is lucky enough to notice, what should it do?
> The only sane answer is to shoot itself in the head. This kind of issue
> would require immediate developer attention to fix whatever Xen bug had been
> exploited to trigger SMEP.
> 
> > 32bit pv guest should be able to make use of SMEP.  When it is from Xen,
> > we crash Xen.  When it's is from guest kernel executing user code, we
> > can inject to guest to let it kill the current process.  Of course such 
> > cases
> > the guest should be able to do SMEP handling.
> 
> Haha, give over on this idea that unexplainable behaviour should make you
> only crash the process/guest. If your behaviour is unexplainable, and you
> have pretensions of security, you fail-stop.
> 
> > We can't consistently handle it for 64bit and 32bit guest.
> 
> Well yeah, but that ignores my actual question, which was...
> """I wonder whether SMEP should be enabled only for guests (even PV guests)
> which detect it via CPUID and proactively enable it via their virtualised
> CR4? I mean, it is off in real hardware by default for a reason: backward
> compatibility. Furthermore, we only detect spurious page faults for buggy
> old PV guests, the rest will get the SMEP fault punted up to them, which
> seems odd if they don't understand SMEP."""
> 
> I mean, I know we may as well just hide the feature from PV 64b guests
> totally. That's obvious. Let's stop talking about PV 64b guests already! The
> question is: what to do about PV 32b guests?
> 
>  -- Keir
> 
> > Thanks!
> > -Xin


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel