Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758331AbcCCVdM (ORCPT ); Thu, 3 Mar 2016 16:33:12 -0500 Received: from mail-oi0-f44.google.com ([209.85.218.44]:33495 "EHLO mail-oi0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757437AbcCCVdJ convert rfc822-to-8bit (ORCPT ); Thu, 3 Mar 2016 16:33:09 -0500 MIME-Version: 1.0 In-Reply-To: <1457031201-31723-1-git-send-email-rkrcmar@redhat.com> References: <1457031201-31723-1-git-send-email-rkrcmar@redhat.com> From: David Matlack Date: Thu, 3 Mar 2016 13:32:48 -0800 Message-ID: Subject: Re: [PATCH] KVM: x86: disable PEBS before a guest entry To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: "linux-kernel@vger.kernel.org" , kvm list , Paolo Bonzini , =?UTF-8?B?SmnFmcOtIE9sxaFh?= , stable@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3303 Lines: 75 On Thu, Mar 3, 2016 at 10:53 AM, Radim Krčmář wrote: > Linux guests on Haswell (and also SandyBridge and Broadwell, at least) > would crash if you decided to run a host command that uses PEBS, like > perf record -e 'cpu/mem-stores/pp' -a > > This happens because KVM is using VMX MSR switching to disable PEBS, but > SDM [2015-12] 18.4.4.4 Re-configuring PEBS Facilities explains why it > isn't safe: > When software needs to reconfigure PEBS facilities, it should allow a > quiescent period between stopping the prior event counting and setting > up a new PEBS event. The quiescent period is to allow any latent > residual PEBS records to complete its capture at their previously > specified buffer address (provided by IA32_DS_AREA). > > There might not be a quiescent period after the MSR switch, so a CPU > ends up using host's MSR_IA32_DS_AREA to access an area in guest's > memory. (Or MSR switching is just buggy on some models.) > > The guest can learn something about the host this way: > If the guest doesn't map address pointed by MSR_IA32_DS_AREA, it results > in #PF where we leak host's MSR_IA32_DS_AREA through CR2. > > After that, a malicious guest can map and configure memory where > MSR_IA32_DS_AREA is pointing and can therefore get an output from > host's tracing. > > This is not a critical leak as the host must initiate with PEBS tracing > and I have not been able to get a record from more than one instruction > before vmentry in vmx_vcpu_run() (that place has most registers already > overwritten with guest's). > > We could disable PEBS just few instructions before vmentry, but > disabling it earlier shouldn't affect host tracing too much. > We also don't need to switch MSR_IA32_PEBS_ENABLE on VMENTRY, but that > optimization isn't worth its code, IMO. > > (If you are implementing PEBS for guests, be sure to handle the case > where both host and guest enable PEBS, because this patch doesn't.) > > Fixes: 26a4f3c08de4 ("perf/x86: disable PEBS on a guest entry.") > Cc: > Reported-by: Jiří Olša > Signed-off-by: Radim Krčmář > --- > arch/x86/kvm/vmx.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 46154dac71e6..946582f4f105 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -1767,6 +1767,13 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr) > return; > } > break; > + case MSR_IA32_PEBS_ENABLE: > + /* PEBS needs a quiescent period after being disabled (to write > + * a record). Disabling PEBS through VMX MSR swapping doesn't > + * provide that period, so a CPU could write host's record into > + * guest's memory. > + */ > + wrmsrl(MSR_IA32_PEBS_ENABLE, 0); Should this go in add_atomic_switch_msr instead of clear_atomic_switch_msr? > } > > for (i = 0; i < m->nr; ++i) > -- > 2.7.2 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html