Re: [Xen-devel] [RFC 4/4] HVM x86 deprivileged mode: Trap handlers for deprivileged mode

[Ben Catterall <Ben.Catterall@xxxxxxxxxx>]

On 07/08/15 14:19, Andrew Cooper wrote:
> On 07/08/15 13:32, Ben Catterall wrote:
> > On 06/08/15 22:24, Andrew Cooper wrote:
> > > On 06/08/2015 17:45, Ben Catterall wrote:
> > > > Added trap handlers to catch exceptions such as a page fault, general
> > > > protection fault, etc. These handlers will crash the domain as such
> > > > exceptions
> > > > would indicate that either there is a bug in deprivileged mode or it
> > > > has been
> > > > compromised by an attacker.
> > > >
> > > > Signed-off-by: Ben Catterall <Ben.Catterall@xxxxxxxxxx>
> > > > ---
> > > >    xen/arch/x86/mm/hap/hap.c |  9 +++++++++
> > > >    xen/arch/x86/traps.c      | 41
> > > > ++++++++++++++++++++++++++++++++++++++++-
> > > >    2 files changed, 49 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> > > > index abc5113..43bde89 100644
> > > > --- a/xen/arch/x86/mm/hap/hap.c
> > > > +++ b/xen/arch/x86/mm/hap/hap.c
> > > > @@ -685,8 +685,17 @@ static int hap_page_fault(struct vcpu *v,
> > > > unsigned long va,
> > > >    {
> > > >        struct domain *d = v->domain;
> > > >    +    /* If we get a page fault whilst in HVM security user mode */
> > > > +    if( v->user_mode == 1 )
> > > > +    {
> > > > +        printk("HVM: #PF (%u:%u) whilst in user mode\n",
> > > > +                 d->domain_id, v->vcpu_id);
> > > %pv is your friend.  Like Linux, we have custom printk formats.  In this
> > > case, passing 'v' as a parameter to %pv will cause d$Xv$Y to be
> > > printed.  (The example below predates %pv being introduced).
> > ok, will do. thanks!
> > > > +        domain_crash_synchronous();
> > > No need for _synchronous() here.  _synchronous() should only be used
> > > when you can't safely recover.  It ends up spinning in a tight loop
> > > waiting for the next timer interrupt, is anything up to 30ms away.
> > I'm not sure if we can safely recover from this. This will only be
> > triggered if there is a bug in depriv mode
> > or if the mode has been compromised and an attacker has tried to
> > access unavailable memory.
> >  From my understanding (am I missing something?): domain_crash
> > effectively sets flags to tell the scheduler that
> > it should be killed the next time the scheduler runs and then returns.
> > In which case, if we don't do a
> > synchronous crash, this return path would return back into the
> > deprivileged mode, we would not
> > have mapped in the page (as we shouldn't), and then we get another fault.
> >
> > What do you think is the best way forward? Thanks!
> Given that there is a use of domain_crash(d) in context below, it is
> clearly safe to use from here.  (Although my general point about hap vs
> shadow code still applies, meaning that hap_page_fault() is not the
> correct function to hook like this.)
>
> domain_crash() sets a flag, but exiting out from a fault handler heading
> back towards ring3 code should check for pending softirqs.  However,
> because of the way you have hooked return-to-depriv, you might have
> broken this.
Understood, I'll move the handler, make this change and examine the 
return path. Thanks!
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel