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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6b811eb0-1204-08b1-61c2-0030fa450ad4@redhat.com>
Date:   Mon, 18 Sep 2017 10:41:21 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Yu Zhang <yu.c.zhang@...ux.intel.com>,
        Jim Mattson <jmattson@...gle.com>
Cc:     kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>, xiaoguangrong@...cent.com,
        Joerg Roedel <joro@...tes.org>
Subject: Re: [PATCH v3 2/5] KVM: MMU: check guest CR3 reserved bits based on
 its physical address width.

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.

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

Paolo