| 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
| ||
|
Date: Tue, 30 Jan 2018 18:43:35 +0200
From: Mihai Carabas <mihai.carabas@...cle.com>
To: Jim Mattson <jmattson@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
LKML <linux-kernel@...r.kernel.org>,
kvm list <kvm@...r.kernel.org>,
Radim Krčmář <rkrcmar@...hat.com>,
Liran Alon <liran.alon@...cle.com>,
Anthony Liguori <aliguori@...zon.com>,
Tom Lendacky <thomas.lendacky@....com>,
David Woodhouse <dwmw@...zon.co.uk>,
Borislav Petkov <bp@...en8.de>,
the arch/x86 maintainers <x86@...nel.org>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Subject: Re: [9/8] KVM: x86: limit MSR_IA32_SPEC_CTRL access based on CPUID
availability
On 30.01.2018 18:33, Jim Mattson wrote:
> All MSR intercepts are enabled by default, so I don't think this patch
> does anything at all, unless I'm missing some context.
>
Currently on upstream some MSR are intercepted:
https://github.com/torvalds/linux/blob/master/arch/x86/kvm/vmx.c#L6838
In particular to this patch, the MSR_IA32_SPEC_CTRL intercept is
disabled in 3/8: https://patchwork.kernel.org/patch/10151889/
> On Tue, Jan 30, 2018 at 5:21 AM, Mihai Carabas <mihai.carabas@...cle.com> wrote:
>> Hello Paolo,
>>
>> I've back ported this patch on 4.1, after adding the per-vcpu MSR bitmap.
>> Also enabled the SPEC_CTRL_MSR intercept if qemu instructed so [1].
>>
>> Reviewed-by: Mihai Carabas <mihai.carabas@...cle.com>
>>
>> [1]
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -8391,6 +8391,16 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm
>> *kvm, unsigned int id)
>> vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PRED_CMD,
>> MSR_TYPE_R | MSR_TYPE_W);
>> vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_BNDCFGS,
>> MSR_TYPE_R | MSR_TYPE_W);
>>
>> + /*
>> + * If the physical CPU or the vCPU of this VM doesn't
>> + * support SPEC_CTRL feature, catch each access to it.
>> + */
>> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
>> + !guest_cpuid_has_spec_ctrl(&vmx->vcpu))
>> + vmx_enable_intercept_for_msr(
>> + msr_bitmap,
>> + MSR_IA32_SPEC_CTRL,
>> + MSR_TYPE_R | MSR_TYPE_W);
>>
>> /*
>> * If PML is turned on, failure on enabling PML just results in
>> failure
>>
>>
>>
>> On 09.01.2018 14:03, Paolo Bonzini wrote:
>>>
>>> MSR_IA32_SPEC_CTRL is not available unless CPU[7,0].EDX[26] is 1.
>>> Check that against host CPUID or guest CPUID, respectively for
>>> host-initiated and guest-initiated accesses.
>>>
>>> Suggested-by: Jim Mattson <jmattson@...gle.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
>>> ---
>>> This is for after X86_FEATURE_SPEC_CTRL is added to Linux, but
>>> I still wanted to ack Jim's improvement.
>>>
>>> arch/x86/kvm/svm.c | 8 ++++++++
>>> arch/x86/kvm/vmx.c | 8 ++++++++
>>> 2 files changed, 16 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>>> index 97126c2bd663..3a646580d7c5 100644
>>> --- a/arch/x86/kvm/svm.c
>>> +++ b/arch/x86/kvm/svm.c
>>> @@ -3648,6 +3648,10 @@ static int svm_get_msr(struct kvm_vcpu *vcpu,
>>> struct msr_data *msr_info)
>>> msr_info->data = svm->nested.vm_cr_msr;
>>> break;
>>> case MSR_IA32_SPEC_CTRL:
>>> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
>>> + (!msr_info->host_initiated &&
>>> + !guest_cpuid_has(vcpu,
>>> X86_FEATURE_SPEC_CTRL)))
>>> + return 1;
>>> msr_info->data = svm->spec_ctrl;
>>> break;
>>> case MSR_IA32_UCODE_REV:
>>> @@ -3806,6 +3810,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu,
>>> struct msr_data *msr)
>>> vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data
>>> 0x%llx\n", ecx, data);
>>> break;
>>> case MSR_IA32_SPEC_CTRL:
>>> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
>>> + (!msr_info->host_initiated &&
>>> + !guest_cpuid_has(vcpu,
>>> X86_FEATURE_SPEC_CTRL)))
>>> + return 1;
>>> svm->spec_ctrl = data;
>>> break;
>>> case MSR_IA32_APICBASE:
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 49b4a2d61603..42bc7ee293e4 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -3368,6 +3368,10 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu,
>>> struct msr_data *msr_info)
>>> msr_info->data = guest_read_tsc(vcpu);
>>> break;
>>> case MSR_IA32_SPEC_CTRL:
>>> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
>>> + (!msr_info->host_initiated &&
>>> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)))
>>> + return 1;
>>> msr_info->data = to_vmx(vcpu)->spec_ctrl;
>>> break;
>>> case MSR_IA32_SYSENTER_CS:
>>> @@ -3510,6 +3514,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
>>> struct msr_data *msr_info)
>>> kvm_write_tsc(vcpu, msr_info);
>>> break;
>>> case MSR_IA32_SPEC_CTRL:
>>> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) ||
>>> + (!msr_info->host_initiated &&
>>> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)))
>>> + return 1;
>>> to_vmx(vcpu)->spec_ctrl = data;
>>> break;
>>> case MSR_IA32_CR_PAT:
>>>
>>
Powered by blists - more mailing lists