linux-kernel - Re: [PATCH v2] KVM: x86/intr: Explicitly check NMI from guest to eliminate false positives

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:   Wed, 13 Dec 2023 09:01:18 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Like Xu <like.xu.linux@...il.com>
Cc:     Dongli Zhang <dongli.zhang@...cle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] KVM: x86/intr: Explicitly check NMI from guest to
 eliminate false positives

On Wed, Dec 13, 2023, Like Xu wrote:
> 
> 
> On 13/12/2023 3:28 pm, Dongli Zhang wrote:
> > Hi Like,
> > 
> > On 12/5/23 19:20, Like Xu wrote:
> > > From: Like Xu <likexu@...cent.com>
> > > 
> > > Explicitly checking the source of external interrupt is indeed NMI and not
> > > other types in the kvm_arch_pmi_in_guest(), which prevents perf-kvm false
> > > positive samples generated in perf/core NMI mode after vm-exit but before
> > > kvm_before_interrupt() from being incorrectly labelled as guest samples:
> > 
> > About the before kvm_before_interrupt() ...
> > 
> > > 
> > > # test: perf-record + cpu-cycles:HP (which collects host-only precise samples)
> > > # Symbol                                   Overhead       sys       usr  guest sys  guest usr
> > > # .......................................  ........  ........  ........  .........  .........
> > > #
> > > # Before:
> > >    [g] entry_SYSCALL_64                       24.63%     0.00%     0.00%     24.63%      0.00%
> > >    [g] syscall_return_via_sysret              23.23%     0.00%     0.00%     23.23%      0.00%
> > >    [g] files_lookup_fd_raw                     6.35%     0.00%     0.00%      6.35%      0.00%
> > > # After:
> > >    [k] perf_adjust_freq_unthr_context         57.23%    57.23%     0.00%      0.00%      0.00%
> > >    [k] __vmx_vcpu_run                          4.09%     4.09%     0.00%      0.00%      0.00%
> > >    [k] vmx_update_host_rsp                     3.17%     3.17%     0.00%      0.00%      0.00%
> > > 
> > > In the above case, perf records the samples labelled '[g]', the RIPs behind
> > > the weird samples are actually being queried by perf_instruction_pointer()
> > > after determining whether it's in GUEST state or not, and here's the issue:
> > > 
> > > If vm-exit is caused by a non-NMI interrupt (such as hrtimer_interrupt) and
> > > at least one PMU counter is enabled on host, the kvm_arch_pmi_in_guest()
> > > will remain true (KVM_HANDLING_IRQ is set) until kvm_before_interrupt().
> > 
> > ... and here.
> > 
> > Would you mind helping why kvm_arch_pmi_in_guest() remains true before
> > *kvm_before_interrupt()*.
> > 
> > According to the source code, the vcpu->arch.handling_intr_from_guest
> > is set to non-zero only at kvm_before_interrupt(), and cleared at
> > kvm_after_interrupt().
> > 
> > Or would you mean kvm_after_interrupt()?
> 
> Oops, it should refer to kvm_after_interrupt() as the code fixed. Thank you.

No need for another version if that's the only hiccup, I can fixup when applying.