Re: [PATCH v3 2/5] KVM: MMU: check guest CR3 reserved bits based on its physical address width.

[Yu Zhang]

On 9/18/2017 4:41 PM, Paolo Bonzini wrote:
> On 18/09/2017 10:15, Yu Zhang wrote:
> > > static bool emulator_get_cpuid(struct x86_emulate_ctxt *ctxt,
> > >                           u32 *eax, u32 *ebx, u32 *ecx, u32 *edx, bool
> > > check_limit)
> > > {
> > >           return kvm_cpuid(emul_to_vcpu(ctxt), eax, ebx, ecx, edx,
> > > check_limit);
> > > }
> > >
> > > And:
> > >
> > > bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
> > >          u32 *ecx, u32 *edx, bool check_limit)
> > > {
> > > u32 function = *eax, index = *ecx;
> > > struct kvm_cpuid_entry2 *best;
> > > bool entry_found = true;
> > > ...
> > >
> > > Doesn't this immediately try to dereference a NULL pointer?  How much
> > > testing have you done of this code?
> > Thanks Jim.
> > I have tested this code in a simulator to successfully boot a VM in
> > shadow mode. Seems this code is not covered(but I am now still
> > perplexed why this is not covered). Any possibility that the
> > check_cr_write() is not triggered when emulating the cr operations?
> CR moves usually don't go through the emulator (the main exception is
> emulation of invalid guest state when the processor doesn't support
> unrestricted_guest=1, but even that is unlikely to happen with
> EFER.LMA=1).  This explains why you didn't see the failure.

Oh, right. It normally goes to handle_cr(). Thanks, Paolo.

Yu

> > Anyway, this should be a bug and thanks for pointing this out, and
> > I'll send out the fix later.
> Thanks,
>
> Paolo