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

[Ben Catterall <Ben.Catterall@citrix.com>]

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@citrix.com>
>> ---
>>   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!


> ~Andrew
>
>> +    }
>> +
>>       HAP_ERROR("Intercepted a guest #PF (%u:%u) with HAP enabled.\n",
>>                 d->domain_id, v->vcpu_id);
>> +
>>       domain_crash(d);
>>       return 0;
>>   }