| Message ID | 20210422022128.3464144-2-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Fixes for (benign?) truncation bugs | expand |
On 22/04/21 04:21, Sean Christopherson wrote: > Remove the emulator's checks for illegal CR0, CR3, and CR4 values, as > the checks are redundant, outdated, and in the case of SEV's C-bit, > broken. The emulator manually calculates MAXPHYADDR from CPUID and > neglects to mask off the C-bit. For all other checks, kvm_set_cr*() are > a superset of the emulator checks, e.g. see CR4.LA57. > > Fixes: a780a3ea6282 ("KVM: X86: Fix reserved bits check for MOV to CR3") > Cc: Babu Moger <babu.moger@amd.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/emulate.c | 68 +----------------------------------------- > 1 file changed, 1 insertion(+), 67 deletions(-) This can be (opportunistically ;)) squashed on top: diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index f4273b8e31fa..abd9a4db11a8 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4220,15 +4220,7 @@ static bool valid_cr(int nr) } } -static int check_cr_read(struct x86_emulate_ctxt *ctxt) -{ - if (!valid_cr(ctxt->modrm_reg)) - return emulate_ud(ctxt); - - return X86EMUL_CONTINUE; -} - -static int check_cr_write(struct x86_emulate_ctxt *ctxt) +static int check_cr_access(struct x86_emulate_ctxt *ctxt) { if (!valid_cr(ctxt->modrm_reg)) return emulate_ud(ctxt); @@ -4775,10 +4767,10 @@ static const struct opcode twobyte_table[256] = { D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */ D(ImplicitOps | ModRM | SrcMem | NoAccess), /* NOP + 7 * reserved NOP */ /* 0x20 - 0x2F */ - DIP(ModRM | DstMem | Priv | Op3264 | NoMod, cr_read, check_cr_read), + DIP(ModRM | DstMem | Priv | Op3264 | NoMod, cr_read, check_cr_access), DIP(ModRM | DstMem | Priv | Op3264 | NoMod, dr_read, check_dr_read), IIP(ModRM | SrcMem | Priv | Op3264 | NoMod, em_cr_write, cr_write, - check_cr_write), + check_cr_access), IIP(ModRM | SrcMem | Priv | Op3264 | NoMod, em_dr_write, dr_write, check_dr_write), N, N, N, N, > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index f7970ba6219f..f4273b8e31fa 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -4230,75 +4230,9 @@ static int check_cr_read(struct x86_emulate_ctxt *ctxt) > > static int check_cr_write(struct x86_emulate_ctxt *ctxt) > { > - u64 new_val = ctxt->src.val64; > - int cr = ctxt->modrm_reg; > - u64 efer = 0; > - > - static u64 cr_reserved_bits[] = { > - 0xffffffff00000000ULL, > - 0, 0, 0, /* CR3 checked later */ > - CR4_RESERVED_BITS, > - 0, 0, 0, > - CR8_RESERVED_BITS, > - }; > - > - if (!valid_cr(cr)) > + if (!valid_cr(ctxt->modrm_reg)) > return emulate_ud(ctxt); > > - if (new_val & cr_reserved_bits[cr]) > - return emulate_gp(ctxt, 0); > - > - switch (cr) { > - case 0: { > - u64 cr4; > - if (((new_val & X86_CR0_PG) && !(new_val & X86_CR0_PE)) || > - ((new_val & X86_CR0_NW) && !(new_val & X86_CR0_CD))) > - return emulate_gp(ctxt, 0); > - > - cr4 = ctxt->ops->get_cr(ctxt, 4); > - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); > - > - if ((new_val & X86_CR0_PG) && (efer & EFER_LME) && > - !(cr4 & X86_CR4_PAE)) > - return emulate_gp(ctxt, 0); > - > - break; > - } > - case 3: { > - u64 rsvd = 0; > - > - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); > - if (efer & EFER_LMA) { > - u64 maxphyaddr; > - u32 eax, ebx, ecx, edx; > - > - eax = 0x80000008; > - ecx = 0; > - if (ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, > - &edx, true)) > - maxphyaddr = eax & 0xff; > - else > - maxphyaddr = 36; > - rsvd = rsvd_bits(maxphyaddr, 63); > - if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_PCIDE) > - rsvd &= ~X86_CR3_PCID_NOFLUSH; > - } > - > - if (new_val & rsvd) > - return emulate_gp(ctxt, 0); > - > - break; > - } > - case 4: { > - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); > - > - if ((efer & EFER_LMA) && !(new_val & X86_CR4_PAE)) > - return emulate_gp(ctxt, 0); > - > - break; > - } > - } > - > return X86EMUL_CONTINUE; > } > >
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index f7970ba6219f..f4273b8e31fa 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4230,75 +4230,9 @@ static int check_cr_read(struct x86_emulate_ctxt *ctxt) static int check_cr_write(struct x86_emulate_ctxt *ctxt) { - u64 new_val = ctxt->src.val64; - int cr = ctxt->modrm_reg; - u64 efer = 0; - - static u64 cr_reserved_bits[] = { - 0xffffffff00000000ULL, - 0, 0, 0, /* CR3 checked later */ - CR4_RESERVED_BITS, - 0, 0, 0, - CR8_RESERVED_BITS, - }; - - if (!valid_cr(cr)) + if (!valid_cr(ctxt->modrm_reg)) return emulate_ud(ctxt); - if (new_val & cr_reserved_bits[cr]) - return emulate_gp(ctxt, 0); - - switch (cr) { - case 0: { - u64 cr4; - if (((new_val & X86_CR0_PG) && !(new_val & X86_CR0_PE)) || - ((new_val & X86_CR0_NW) && !(new_val & X86_CR0_CD))) - return emulate_gp(ctxt, 0); - - cr4 = ctxt->ops->get_cr(ctxt, 4); - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); - - if ((new_val & X86_CR0_PG) && (efer & EFER_LME) && - !(cr4 & X86_CR4_PAE)) - return emulate_gp(ctxt, 0); - - break; - } - case 3: { - u64 rsvd = 0; - - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); - if (efer & EFER_LMA) { - u64 maxphyaddr; - u32 eax, ebx, ecx, edx; - - eax = 0x80000008; - ecx = 0; - if (ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, - &edx, true)) - maxphyaddr = eax & 0xff; - else - maxphyaddr = 36; - rsvd = rsvd_bits(maxphyaddr, 63); - if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_PCIDE) - rsvd &= ~X86_CR3_PCID_NOFLUSH; - } - - if (new_val & rsvd) - return emulate_gp(ctxt, 0); - - break; - } - case 4: { - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); - - if ((efer & EFER_LMA) && !(new_val & X86_CR4_PAE)) - return emulate_gp(ctxt, 0); - - break; - } - } - return X86EMUL_CONTINUE; }
Remove the emulator's checks for illegal CR0, CR3, and CR4 values, as the checks are redundant, outdated, and in the case of SEV's C-bit, broken. The emulator manually calculates MAXPHYADDR from CPUID and neglects to mask off the C-bit. For all other checks, kvm_set_cr*() are a superset of the emulator checks, e.g. see CR4.LA57. Fixes: a780a3ea6282 ("KVM: X86: Fix reserved bits check for MOV to CR3") Cc: Babu Moger <babu.moger@amd.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/emulate.c | 68 +----------------------------------------- 1 file changed, 1 insertion(+), 67 deletions(-)