| 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
| ||
|
Message-ID: <701a94eb-feac-4578-850c-5b1f015877af@linux.intel.com>
Date: Sun, 27 Apr 2025 17:10:44 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
x86@...nel.org
Cc: linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Seth Forshee <sforshee@...nel.org>
Subject: Re: [PATCH] perf/x86/intel: KVM: Mask PEBS_ENABLE loaded for guest
with vCPU's value.
On 4/26/2025 8:13 AM, Sean Christopherson wrote:
> When generating the MSR_IA32_PEBS_ENABLE value that will be loaded on
> VM-Entry to a KVM guest, mask the value with the vCPU's desired PEBS_ENABLE
> value. Consulting only the host kernel's host vs. guest masks results in
> running the guest with PEBS enabled even when the guest doesn't want to use
> PEBS. Because KVM uses perf events to proxy the guest virtual PMU, simply
> looking at exclude_host can't differentiate between events created by host
> userspace, and events created by KVM on behalf of the guest.
>
> Running the guest with PEBS unexpectedly enabled typically manifests as
> crashes due to a near-infinite stream of #PFs. E.g. if the guest hasn't
> written MSR_IA32_DS_AREA, the CPU will hit page faults on address '0' when
> trying to record PEBS events.
>
> The issue is most easily reproduced by running `perf kvm top` from before
> commit 7b100989b4f6 ("perf evlist: Remove __evlist__add_default") (after
> which, `perf kvm top` effectively stopped using PEBS). The userspace side
> of perf creates a guest-only PEBS event, which intel_guest_get_msrs()
> misconstrues a guest-*owned* PEBS event.
>
> Arguably, this is a userspace bug, as enabling PEBS on guest-only events
> simply cannot work, and userspace can kill VMs in many other ways (there
> is no danger to the host). However, even if this is considered to be bad
> userspace behavior, there's zero downside to perf/KVM restricting PEBS to
> guest-owned events.
>
> Note, commit 854250329c02 ("KVM: x86/pmu: Disable guest PEBS temporarily
> in two rare situations") fixed the case where host userspace is profiling
> KVM *and* userspace, but missed the case where userspace is profiling only
> KVM.
>
> Fixes: c59a1f106f5c ("KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS")
> Reported-by: Seth Forshee <sforshee@...nel.org>
> Closes: https://lore.kernel.org/all/Z_VUswFkWiTYI0eD@do-x1carbon
> Cc: stable@...r.kernel.org
> Cc: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> arch/x86/events/intel/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index cd6329207311..75a376478b21 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -4625,7 +4625,7 @@ static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr, void *data)
> arr[pebs_enable] = (struct perf_guest_switch_msr){
> .msr = MSR_IA32_PEBS_ENABLE,
> .host = cpuc->pebs_enabled & ~cpuc->intel_ctrl_guest_mask,
> - .guest = pebs_mask & ~cpuc->intel_ctrl_host_mask,
> + .guest = pebs_mask & ~cpuc->intel_ctrl_host_mask & kvm_pmu->pebs_enable,
Although I can't reproduce the guest crash issue on local SPR server, the
logic is correct.
Reviewed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
Furthermore, the below global_ctrl guest value calculation seems
incorrect as well.
u64 pebs_mask = cpuc->pebs_enabled & x86_pmu.pebs_capable;
arr[global_ctrl] = (struct perf_guest_switch_msr){
.msr = MSR_CORE_PERF_GLOBAL_CTRL,
.host = intel_ctrl & ~cpuc->intel_ctrl_guest_mask,
.guest = intel_ctrl & ~cpuc->intel_ctrl_host_mask & ~pebs_mask,
};
For guest PEBS events, its HW counter idx would be set into
cpuc->pebs_enabled as well, so the calculated guest value would clear the
corresponding bit of the guest PEBS events. Thus the guest PEBS events
should not be enabled in non-root mode, but coincidentally since the
calculated host and guest value of global_ctrl MSR is same,
atomic_switch_perf_msrs() doesn't really write the guest value into
GLOBAL_CTRL MSR before vm-entry and GLOBAL_CTRL MSR still keeps the default
value (all HW counters are enabled). That's why we see guest PEBS still works.
for (i = 0; i < nr_msrs; i++)
if (msrs[i].host == msrs[i].guest)
clear_atomic_switch_msr(vmx, msrs[i].msr);
else
add_atomic_switch_msr(vmx, msrs[i].msr, msrs[i].guest,
msrs[i].host, false);
Currently we have this Sean's fix, only the guest PEBS event bits of
IA32_PEBS_ENABLE MSR are enabled in non-root mode, suppose we can simply
change global_ctrl guest value calculation to this.
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 09d2d66c9f21..5bc56bb616ec 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4342,9 +4342,12 @@ static struct perf_guest_switch_msr
*intel_guest_get_msrs(int *nr, void *data)
arr[global_ctrl] = (struct perf_guest_switch_msr){
.msr = MSR_CORE_PERF_GLOBAL_CTRL,
.host = intel_ctrl & ~cpuc->intel_ctrl_guest_mask,
- .guest = intel_ctrl & ~cpuc->intel_ctrl_host_mask & ~pebs_mask,
+ .guest = intel_ctrl & ~cpuc->intel_ctrl_host_mask,
};
> };
>
> if (arr[pebs_enable].host) {
>
> base-commit: 2492e5aba2be064d0604ae23ae0770ecc0168192
Powered by blists - more mailing lists