| 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: <Y7WpBSx5fBJFhFR4@google.com>
Date: Wed, 4 Jan 2023 16:27:49 +0000
From: Sean Christopherson <seanjc@...gle.com>
To: Like Xu <like.xu.linux@...il.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] KVM: x86: Ignore host accesses to higher version PMU
features MSRs
On Mon, Dec 26, 2022, Like Xu wrote:
> From: Like Xu <likexu@...cent.com>
>
> Higher version PMU features are obviously not supported on hosts with
> lower version Arch pmu, such as trying to access FIXED_CTR registers
> and PERF_GLOBAL registers from pmu.version >1 on a host with version 1.
>
> Ignore host userspace reads and writes of '0' to those PMU MSRs that
> KVM reports in the MSR-to-save list, but the MSRs are ultimately
> unsupported. All MSRs in said list must be writable by userspace, e.g.
> if userspace sends the list back at KVM without filtering out the MSRs
> it doesn't need.
>
> Signed-off-by: Like Xu <likexu@...cent.com>
> ---
> arch/x86/kvm/x86.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f570367463c8..fcb9c317df59 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3881,6 +3881,11 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> case MSR_IA32_DS_AREA:
> case MSR_PEBS_DATA_CFG:
> case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
> + case MSR_ARCH_PERFMON_FIXED_CTR0 ... MSR_ARCH_PERFMON_FIXED_CTR_MAX:
> + case MSR_CORE_PERF_FIXED_CTR_CTRL:
> + case MSR_CORE_PERF_GLOBAL_STATUS:
> + case MSR_CORE_PERF_GLOBAL_CTRL:
> + case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> if (kvm_pmu_is_valid_msr(vcpu, msr))
> return kvm_pmu_set_msr(vcpu, msr_info);
> /*
> @@ -3984,6 +3989,11 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> case MSR_IA32_DS_AREA:
> case MSR_PEBS_DATA_CFG:
> case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
> + case MSR_ARCH_PERFMON_FIXED_CTR0 ... MSR_ARCH_PERFMON_FIXED_CTR_MAX:
> + case MSR_CORE_PERF_FIXED_CTR_CTRL:
> + case MSR_CORE_PERF_GLOBAL_STATUS:
> + case MSR_CORE_PERF_GLOBAL_CTRL:
> + case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
Having to manually handle each MSR is again a maintenance burden. Rather than
manually add the affect PMU MSRs, I think we should just allow benign accesses
to all "MSRs to save". The lookup will be a linear search, but the array isn't
_that_ big and this should be a rare occurrence.
That might also make it easier to handle non-PMU MSRs that want similar treatment.
I'll send a series with the patches I've proposed, along with the patch to clean
up the unimplemented MSR printks[*], which was never formally posted.
[*] https://lore.kernel.org/all/Y1wCqAzJwvz4s8OR@google.com
---
arch/x86/kvm/x86.c | 51 ++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 87bb7024e18f..4ad7e3065c69 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3561,6 +3561,18 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
}
+static bool kvm_is_msr_to_save(u32 msr_index)
+{
+ unsigned int i;
+
+ for (i = 0; i < num_msrs_to_save; i++) {
+ if (msrs_to_save[i] == msr_index)
+ return true;
+ }
+
+ return false;
+}
+
int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
{
bool pr = false;
@@ -3884,20 +3896,18 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
vcpu->arch.guest_fpu.xfd_err = data;
break;
#endif
- case MSR_IA32_PEBS_ENABLE:
- case MSR_IA32_DS_AREA:
- case MSR_PEBS_DATA_CFG:
- case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
+ default:
if (kvm_pmu_is_valid_msr(vcpu, msr))
return kvm_pmu_set_msr(vcpu, msr_info);
+
/*
* Userspace is allowed to write '0' to MSRs that KVM reports
* as to-be-saved, even if an MSRs isn't fully supported.
*/
- return !msr_info->host_initiated || data;
- default:
- if (kvm_pmu_is_valid_msr(vcpu, msr))
- return kvm_pmu_set_msr(vcpu, msr_info);
+ if (msr_info->host_initiated && !data &&
+ kvm_is_msr_to_save(msr))
+ break;
+
return KVM_MSR_RET_INVALID;
}
return 0;
@@ -3987,20 +3997,6 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_DRAM_ENERGY_STATUS: /* DRAM controller */
msr_info->data = 0;
break;
- case MSR_IA32_PEBS_ENABLE:
- case MSR_IA32_DS_AREA:
- case MSR_PEBS_DATA_CFG:
- case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
- if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
- return kvm_pmu_get_msr(vcpu, msr_info);
- /*
- * Userspace is allowed to read MSRs that KVM reports as
- * to-be-saved, even if an MSR isn't fully supported.
- */
- if (!msr_info->host_initiated)
- return 1;
- msr_info->data = 0;
- break;
case MSR_K7_EVNTSEL0 ... MSR_K7_EVNTSEL3:
case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3:
case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1:
@@ -4256,6 +4252,17 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
default:
if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
return kvm_pmu_get_msr(vcpu, msr_info);
+
+ /*
+ * Userspace is allowed to read MSRs that KVM reports as
+ * to-be-saved, even if an MSR isn't fully supported.
+ */
+ if (msr_info->host_initiated &&
+ kvm_is_msr_to_save(msr_info->index)) {
+ msr_info->data = 0;
+ break;
+ }
+
return KVM_MSR_RET_INVALID;
}
return 0;
base-commit: eee17ec1d2c43ab3fcba604c4b88c6eb2d728fcd
--
Powered by blists - more mailing lists