2020-07-03 02:20:46

by Sean Christopherson

Subject: [PATCH] KVM: x86: Inject #GP if guest attempts to toggle CR4.LA57 in 64-bit mode

Inject a #GP on MOV CR4 if CR4.LA57 is toggled in 64-bit mode, which is
illegal per Intel's SDM:

CR4.LA57
57-bit linear addresses (bit 12 of CR4) ... blah blah blah ...
This bit cannot be modified in IA-32e mode.

Note, the pseudocode for MOV CR doesn't call out the fault condition,
which is likely why the check was missed during initial development.
This is arguably an SDM bug and will hopefully be fixed in future
release of the SDM.

Fixes: fd8cb433734ee ("KVM: MMU: Expose the LA57 feature to VM.")
Cc: [email protected]
Reported-by: Sebastien Boeuf <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/x86.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 00c88c2f34e4..2bb48896dbdc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -975,6 +975,8 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
if (is_long_mode(vcpu)) {
if (!(cr4 & X86_CR4_PAE))
return 1;
+ if ((cr4 ^ old_cr4) & X86_CR4_LA57)
+ return 1;
} else if (is_paging(vcpu) && (cr4 & X86_CR4_PAE)
&& ((cr4 ^ old_cr4) & pdptr_bits)
&& !load_pdptrs(vcpu, vcpu->arch.walk_mmu,
--
2.26.0

2020-07-03 16:16:46

by Paolo Bonzini

Subject: Re: [PATCH] KVM: x86: Inject #GP if guest attempts to toggle CR4.LA57 in 64-bit mode

On 03/07/20 04:17, Sean Christopherson wrote:
> Inject a #GP on MOV CR4 if CR4.LA57 is toggled in 64-bit mode, which is
> illegal per Intel's SDM:
>
> CR4.LA57
> 57-bit linear addresses (bit 12 of CR4) ... blah blah blah ...
> This bit cannot be modified in IA-32e mode.
>
> Note, the pseudocode for MOV CR doesn't call out the fault condition,
> which is likely why the check was missed during initial development.
> This is arguably an SDM bug and will hopefully be fixed in future
> release of the SDM.
>
> Fixes: fd8cb433734ee ("KVM: MMU: Expose the LA57 feature to VM.")
> Cc: [email protected]
> Reported-by: Sebastien Boeuf <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/x86.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 00c88c2f34e4..2bb48896dbdc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -975,6 +975,8 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> if (is_long_mode(vcpu)) {
> if (!(cr4 & X86_CR4_PAE))
> return 1;
> + if ((cr4 ^ old_cr4) & X86_CR4_LA57)
> + return 1;
> } else if (is_paging(vcpu) && (cr4 & X86_CR4_PAE)
> && ((cr4 ^ old_cr4) & pdptr_bits)
> && !load_pdptrs(vcpu, vcpu->arch.walk_mmu,
>

Queued, thanks.

Paolo