| Message ID | 20210422022128.3464144-1-seanjc@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: x86: Fixes for (benign?) truncation bugs | expand |
On 22/04/21 04:21, Sean Christopherson wrote: > Patches 01 and 02 fix theoretical bugs related to loading CRs through > the emulator. The rest of the patches are a bunch of small fixes for > cases where KVM reads/writes a 64-bit register outside of 64-bit mode. > > I stumbled on this when puzzling over commit 0107973a80ad ("KVM: x86: > Introduce cr3_lm_rsvd_bits in kvm_vcpu_arch"), which stated that SEV > guests failed to boot on PCID-enabled hosts. Why only PCID hosts? > > After much staring, I realized that the initial CR3 load in > rsm_enter_protected_mode() would skip the MAXPHYADDR check due to the > vCPU not being in long mode. But due to the ordering problems with > PCID, when PCID is enabled in the guest, the second load of CR3 would > be done with long mode enabled and thus hit the SEV C-bit bug. > > Changing kvm_set_cr3() made me look at the callers, and seeing that > SVM didn't properly truncate the value made me look at everything else, > and here we are. > > Note, I strongly suspect the emulator still has bugs. But, unless the > guest is deliberately trying to hit these types of bugs, even the ones > fixed here, they're likely benign. I figured I was more likely to break > something than I was to fix something by diving into the emulator, so I > left it alone. For now. :-) > > v2: Rebase to kvm/queue, commit 89a22e37c8c2 ("KVM: avoid "deadlock" > between install_new_memslots and MMU notifier") > > v1: https://lkml.kernel.org/r/20210213010518.1682691-1-seanjc@google.com > > Sean Christopherson (9): > KVM: x86: Remove emulator's broken checks on CR0/CR3/CR4 loads > KVM: x86: Check CR3 GPA for validity regardless of vCPU mode > KVM: SVM: Truncate GPR value for DR and CR accesses in !64-bit mode > KVM: VMX: Truncate GPR value for DR and CR reads in !64-bit mode > KVM: nVMX: Truncate bits 63:32 of VMCS field on nested check in > !64-bit > KVM: nVMX: Truncate base/index GPR value on address calc in !64-bit > KVM: x86/xen: Drop RAX[63:32] when processing hypercall > KVM: SVM: Use default rAX size for INVLPGA emulation > KVM: x86: Rename GPR accessors to make mode-aware variants the > defaults > > arch/x86/kvm/emulate.c | 68 +---------------------------------- > arch/x86/kvm/kvm_cache_regs.h | 19 ++++++---- > arch/x86/kvm/svm/svm.c | 12 +++++-- > arch/x86/kvm/vmx/nested.c | 14 ++++---- > arch/x86/kvm/vmx/vmx.c | 6 ++-- > arch/x86/kvm/x86.c | 19 ++++++---- > arch/x86/kvm/x86.h | 8 ++--- > 7 files changed, 48 insertions(+), 98 deletions(-) > Queued, thanks. Paolo
Patches 01 and 02 fix theoretical bugs related to loading CRs through the emulator. The rest of the patches are a bunch of small fixes for cases where KVM reads/writes a 64-bit register outside of 64-bit mode. I stumbled on this when puzzling over commit 0107973a80ad ("KVM: x86: Introduce cr3_lm_rsvd_bits in kvm_vcpu_arch"), which stated that SEV guests failed to boot on PCID-enabled hosts. Why only PCID hosts? After much staring, I realized that the initial CR3 load in rsm_enter_protected_mode() would skip the MAXPHYADDR check due to the vCPU not being in long mode. But due to the ordering problems with PCID, when PCID is enabled in the guest, the second load of CR3 would be done with long mode enabled and thus hit the SEV C-bit bug. Changing kvm_set_cr3() made me look at the callers, and seeing that SVM didn't properly truncate the value made me look at everything else, and here we are. Note, I strongly suspect the emulator still has bugs. But, unless the guest is deliberately trying to hit these types of bugs, even the ones fixed here, they're likely benign. I figured I was more likely to break something than I was to fix something by diving into the emulator, so I left it alone. For now. :-) v2: Rebase to kvm/queue, commit 89a22e37c8c2 ("KVM: avoid "deadlock" between install_new_memslots and MMU notifier") v1: https://lkml.kernel.org/r/20210213010518.1682691-1-seanjc@google.com Sean Christopherson (9): KVM: x86: Remove emulator's broken checks on CR0/CR3/CR4 loads KVM: x86: Check CR3 GPA for validity regardless of vCPU mode KVM: SVM: Truncate GPR value for DR and CR accesses in !64-bit mode KVM: VMX: Truncate GPR value for DR and CR reads in !64-bit mode KVM: nVMX: Truncate bits 63:32 of VMCS field on nested check in !64-bit KVM: nVMX: Truncate base/index GPR value on address calc in !64-bit KVM: x86/xen: Drop RAX[63:32] when processing hypercall KVM: SVM: Use default rAX size for INVLPGA emulation KVM: x86: Rename GPR accessors to make mode-aware variants the defaults arch/x86/kvm/emulate.c | 68 +---------------------------------- arch/x86/kvm/kvm_cache_regs.h | 19 ++++++---- arch/x86/kvm/svm/svm.c | 12 +++++-- arch/x86/kvm/vmx/nested.c | 14 ++++---- arch/x86/kvm/vmx/vmx.c | 6 ++-- arch/x86/kvm/x86.c | 19 ++++++---- arch/x86/kvm/x86.h | 8 ++--- 7 files changed, 48 insertions(+), 98 deletions(-)