linux-kernel - Re: [PATCH] KVM: VMX: Disable Intel PT before VM-entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Mar 2020 10:21:52 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     "Kang, Luwei" <luwei.kang@...el.com>
Cc:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "wanpengli@...cent.com" <wanpengli@...cent.com>,
        "jmattson@...gle.com" <jmattson@...gle.com>,
        "joro@...tes.org" <joro@...tes.org>
Subject: Re: [PATCH] KVM: VMX: Disable Intel PT before VM-entry

On Mon, Mar 23, 2020 at 02:20:01AM -0700, Kang, Luwei wrote:
> > On Wed, Mar 18, 2020 at 11:48:18AM +0800, Luwei Kang wrote:
> > > If the logical processor is operating with Intel PT enabled (
> > > IA32_RTIT_CTL.TraceEn = 1) at the time of VM entry, the “load
> > > IA32_RTIT_CTL” VM-entry control must be 0(SDM 26.2.1.1).
> > >
> > > The first disabled the host Intel PT(Clear TraceEn) will make all the
> > > buffered packets are flushed out of the processor and it may cause an
> > > Intel PT PMI. The host Intel PT will be re-enabled in the host Intel
> > > PT PMI handler.
> > >
> > > handle_pmi_common()
> > >     -> intel_pt_interrupt()
> > >             -> pt_config_start()
> > 
> > IIUC, this is only possible when PT "plays nice" with VMX, correct?
> > Otherwise pt->vmx_on will be true and pt_config_start() would skip the
> > WRMSR.
> > 
> > And IPT PMI must be delivered via NMI (though maybe they're always
> > delivered via NMI?).
> > 
> > In any case, redoing WRMSR doesn't seem safe, and it certainly isn't
> > performant, e.g. what prevents the second WRMSR from triggering a second
> > IPT PMI?
> > 
> > pt_guest_enter() is called after the switch to the vCPU has already been
> > recorded, can't this be handled in the IPT code, e.g. something like this?
> > 
> > diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c index
> > 1db7a51d9792..e38ddae9f0d1 100644
> > --- a/arch/x86/events/intel/pt.c
> > +++ b/arch/x86/events/intel/pt.c
> > @@ -405,7 +405,7 @@ static void pt_config_start(struct perf_event *event)
> >         ctl |= RTIT_CTL_TRACEEN;
> >         if (READ_ONCE(pt->vmx_on))
> >                 perf_aux_output_flag(&pt->handle, PERF_AUX_FLAG_PARTIAL);
> > -       else
> > +       else (!(current->flags & PF_VCPU))
> >                 wrmsrl(MSR_IA32_RTIT_CTL, ctl);
> 
> Intel PT can work in SYSTEM and HOST_GUEST mode by setting the kvm-intel.ko
> parameter "pt_mode".  In SYSTEM mode, the host and guest PT trace will be
> saved in the host buffer. The KVM do nothing during VM-entry/exit in SYSTEM
> mode and Intel PT PMI may happened on any place. The PT trace may be disabled
> when running in KVM(PT only needs to be disabled before VM-entry in
> HOST_GUEST mode).

Ah, right.  What about enhancing intel_pt_handle_vmx() and 'struct pt' to
replace vmx_on with a field that incorporates the KVM mode?  From an
outsider's perspective, that'd be an improvment irrespective of this bug
fix as 'vmx_on' is misleading, e.g. it can be %false when the CPU is post-
VMXON, and really means "post-VMXON and Intel PT can't trace it".