From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] VMX: don't crash processing 'd' debug key Date: Thu, 7 Nov 2013 15:35:14 +0000 Message-ID: <527BB332.2040704@citrix.com> References: <527B7D3802000078001008A4@nat28.tlf.novell.com> <527BA4F7.60500@citrix.com> <527BB6950200007800100BA3@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VeRc8-0000n0-Tt for xen-devel@lists.xenproject.org; Thu, 07 Nov 2013 15:35:21 +0000 In-Reply-To: <527BB6950200007800100BA3@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Eddie Dong , Jun Nakajima List-Id: xen-devel@lists.xenproject.org On 07/11/13 14:49, Jan Beulich wrote: >>>> On 07.11.13 at 15:34, Andrew Cooper wrote: >> On 07/11/13 10:44, Jan Beulich wrote: >>> --- a/xen/arch/x86/hvm/vmx/vmx.c >>> +++ b/xen/arch/x86/hvm/vmx/vmx.c >>> @@ -675,7 +675,17 @@ void vmx_get_segment_register(struct vcp >>> { >>> unsigned long attr = 0, sel = 0, limit; >>> >>> - vmx_vmcs_enter(v); >>> + /* >>> + * We may get here in the context of dump_execstate(), which may have >>> + * interrupted context switching between setting "current" and >>> + * vmx_do_resume() reaching the end of vmx_load_vmcs(). That would make >>> + * all the VMREADs below fail if we don't bail right away. >>> + */ >>> + if ( unlikely(!vmx_vmcs_enter(v)) ) >>> + { >>> + memset(reg, 0, sizeof(*reg)); >>> + return; >>> + } >> What are the implications of this? All callers unconditionally expect >> this to succeed, and use the results straight as-are. >> >> On the other hand, I am not certain how we could go about dealing with >> the error. > The thing is that for all "normal" code paths we won't get there. > Hence it'll only be that - rather than crashing - no useful > information will be displayed for the debug key. (And obviously, > should vmx_vmcs_enter() nevertheless end up failing on a > "normal" code path, all callers properly looking at the results > should produce #GP faults to the guest [present bit clear, > minimal permissions, and segment limit zero].) > > Jan > Hmm yes. I certainly cant see any normal codepaths which would get here, and the crash path certainly needs fixing. Reviewed-by: Andrew Cooper