| 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: <e45669fa-372f-a29d-d9c9-b4747e56b97c@amd.com>
Date: Tue, 26 Oct 2021 13:03:00 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Vitaly Kuznetsov <vkuznets@...hat.com>,
Paolo Bonzini <pbonzini@...hat.com>
Cc: Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>,
Sean Christopherson <seanjc@...gle.com>,
Wanpeng Li <wanpengli@...cent.com>,
Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Brijesh Singh <brijesh.singh@....com>,
Ashish Kalra <Ashish.Kalra@....com>,
linux-kernel@...r.kernel.org, x86@...nel.org,
kvm list <kvm@...r.kernel.org>
Subject: Re: [PATCH v2] KVM: x86: Assume a 64-bit hypercall for guests with
protected state
On 10/1/21 12:06 PM, Tom Lendacky wrote:
> On 5/25/21 1:25 AM, Vitaly Kuznetsov wrote:
>> Tom Lendacky <thomas.lendacky@....com> writes:
>>
>>> When processing a hypercall for a guest with protected state, currently
>>> SEV-ES guests, the guest CS segment register can't be checked to
>>> determine if the guest is in 64-bit mode. For an SEV-ES guest, it is
>>> expected that communication between the guest and the hypervisor is
>>> performed to shared memory using the GHCB. In order to use the GHCB, the
>>> guest must have been in long mode, otherwise writes by the guest to the
>>> GHCB would be encrypted and not be able to be comprehended by the
>>> hypervisor.
>>>
>>> Create a new helper function, is_64_bit_hypercall(), that assumes the
>>> guest is in 64-bit mode when the guest has protected state, and returns
>>> true, otherwise invoking is_64_bit_mode() to determine the mode. Update
>>> the hypercall related routines to use is_64_bit_hypercall() instead of
>>> is_64_bit_mode().
>>>
>>> Add a WARN_ON_ONCE() to is_64_bit_mode() to catch occurences of calls to
>>> this helper function for a guest running with protected state.
>>>
>>> Fixes: f1c6366e3043 ("KVM: SVM: Add required changes to support
>>> intercepts under SEV-ES")
>>> Reported-by: Sean Christopherson <seanjc@...gle.com>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>>> ---
>>>
>>> Changes since v1:
>>> - Create a new helper routine, is_64_bit_hypercall(), and use it in place
>>> of is_64_bit_mode() in hypercall related areas.
>>> - Add a WARN_ON_ONCE() to is_64_bit_mode() to issue a warning if invoked
>>> for a guest with protected state.
>>> ---
>>> arch/x86/kvm/hyperv.c | 4 ++--
>>> arch/x86/kvm/x86.c | 2 +-
>>> arch/x86/kvm/x86.h | 12 ++++++++++++
>>> arch/x86/kvm/xen.c | 2 +-
>>> 4 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>>> index f98370a39936..1cdf2b213f41 100644
>>> --- a/arch/x86/kvm/hyperv.c
>>> +++ b/arch/x86/kvm/hyperv.c
>>> @@ -1818,7 +1818,7 @@ static void kvm_hv_hypercall_set_result(struct
>>> kvm_vcpu *vcpu, u64 result)
>>> {
>>> bool longmode;
>>> - longmode = is_64_bit_mode(vcpu);
>>> + longmode = is_64_bit_hypercall(vcpu);
>>> if (longmode)
>>> kvm_rax_write(vcpu, result);
>>> else {
>>> @@ -1895,7 +1895,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>>> }
>>> #ifdef CONFIG_X86_64
>>> - if (is_64_bit_mode(vcpu)) {
>>> + if (is_64_bit_hypercall(vcpu)) {
>>> param = kvm_rcx_read(vcpu);
>>> ingpa = kvm_rdx_read(vcpu);
>>> outgpa = kvm_r8_read(vcpu);
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 9b6bca616929..dc72f0a1609a 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -8403,7 +8403,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
>>> trace_kvm_hypercall(nr, a0, a1, a2, a3);
>>> - op_64_bit = is_64_bit_mode(vcpu);
>>> + op_64_bit = is_64_bit_hypercall(vcpu);
>>> if (!op_64_bit) {
>>> nr &= 0xFFFFFFFF;
>>> a0 &= 0xFFFFFFFF;
>>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>>> index 521f74e5bbf2..3102caf689d2 100644
>>> --- a/arch/x86/kvm/x86.h
>>> +++ b/arch/x86/kvm/x86.h
>>> @@ -151,12 +151,24 @@ static inline bool is_64_bit_mode(struct kvm_vcpu
>>> *vcpu)
>>> {
>>> int cs_db, cs_l;
>>> + WARN_ON_ONCE(vcpu->arch.guest_state_protected);
>>> +
>>> if (!is_long_mode(vcpu))
>>> return false;
>>> static_call(kvm_x86_get_cs_db_l_bits)(vcpu, &cs_db, &cs_l);
>>> return cs_l;
>>> }
>>> +static inline bool is_64_bit_hypercall(struct kvm_vcpu *vcpu)
>>> +{
>>> + /*
>>> + * If running with protected guest state, the CS register is not
>>> + * accessible. The hypercall register values will have had to been
>>> + * provided in 64-bit mode, so assume the guest is in 64-bit.
>>> + */
>>> + return vcpu->arch.guest_state_protected || is_64_bit_mode(vcpu);
>>> +}
>>> +
>>> static inline bool is_la57_mode(struct kvm_vcpu *vcpu)
>>> {
>>> #ifdef CONFIG_X86_64
>>> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
>>> index ae17250e1efe..c58f6369e668 100644
>>> --- a/arch/x86/kvm/xen.c
>>> +++ b/arch/x86/kvm/xen.c
>>> @@ -680,7 +680,7 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
>>> kvm_hv_hypercall_enabled(vcpu))
>>> return kvm_hv_hypercall(vcpu);
>>> - longmode = is_64_bit_mode(vcpu);
>>> + longmode = is_64_bit_hypercall(vcpu);
>>> if (!longmode) {
>>> params[0] = (u32)kvm_rbx_read(vcpu);
>>> params[1] = (u32)kvm_rcx_read(vcpu);
>>
>> Reviewed-by: Vitaly Kuznetsov <vkuznets@...hat.com>
>>
>> Thanks!
>
> Paolo,
>
> This got lost in my stack of work... any comments?
>
> Thanks,
> Tom
Ping
Thanks,
Tom
>
>>
Powered by blists - more mailing lists