From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Catterall Subject: Re: [RFC 4/4] HVM x86 deprivileged mode: Trap handlers for deprivileged mode Date: Tue, 11 Aug 2015 11:33:33 +0100 Message-ID: <55C9CF7D.5070500@citrix.com> References: <1438879519-564-1-git-send-email-Ben.Catterall@citrix.com> <1438879519-564-5-git-send-email-Ben.Catterall@citrix.com> <20150810100755.GD3094@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150810100755.GD3094@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: xen-devel@lists.xensource.com, keir@xen.org, ian.campbell@citrix.com, george.dunlap@eu.citrix.com, andrew.cooper3@citrix.com, jbeulich@suse.com List-Id: xen-devel@lists.xenproject.org On 10/08/15 11:07, Tim Deegan wrote: > Hi, > >> @@ -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); >> + domain_crash_synchronous(); >> + } >> + > > This should happen in paging_fault() so it can guard the > shadow-pagetable paths too. Once it's there, it'll need a check for > is_hvm_vcpu() as well as for user_mode. Maybe have a helper function > 'is_hvm_deprivileged_vcpu()' to do both checks, also used in > hvm_deprivileged_check_trap() &c. > Ok, I'll make this change. >> HAP_ERROR("Intercepted a guest #PF (%u:%u) with HAP enabled.\n", >> d->domain_id, v->vcpu_id); >> + >> domain_crash(d); >> return 0; >> } >> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c >> index 9f5a6c6..19d465f 100644 >> --- a/xen/arch/x86/traps.c >> +++ b/xen/arch/x86/traps.c >> @@ -74,6 +74,7 @@ >> #include >> #include >> #include >> +#include >> >> /* >> * opt_nmi: one of 'ignore', 'dom0', or 'fatal'. >> @@ -500,6 +501,11 @@ static void do_guest_trap( >> struct trap_bounce *tb; >> const struct trap_info *ti; >> >> + /* If we take the trap whilst in HVM deprivileged mode >> + * then we should crash the domain. >> + */ >> + hvm_deprivileged_check_trap(__FUNCTION__); > > I wonder whether it would be better to switch to an IDT with all > unacceptable traps stubbed out, rather than have to blacklist them all > separately. Probably not - this check is cheap, and maintaining the > parallel tables would be a pain. > > Or maybe there's some single point upstream of here, in the asm > handlers, that would catch all the cases where this check is needed? > Yep, I think this can be done. > In any case, the check needs to return an error code so the caller > knows to return without running the rest of the handler (and likewise > elsewhere). > understood. > Cheers, > > Tim. >