Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755691AbdIRP4n (ORCPT ); Mon, 18 Sep 2017 11:56:43 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:46522 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755471AbdIRP4l (ORCPT ); Mon, 18 Sep 2017 11:56:41 -0400 X-Google-Smtp-Source: AOwi7QDtTBRjbtU1CDBtiYKqlkgzgZmtD6tAT8GTksP36cMQ2bF520TC6Qckwsco1Q/jX4oiPQTY+WJqStgwo3KKfgo= MIME-Version: 1.0 In-Reply-To: <62dd03f0-4049-1d9c-d07b-61cac4c49c93@redhat.com> References: <1505731501-6821-1-git-send-email-yu.c.zhang@linux.intel.com> <62dd03f0-4049-1d9c-d07b-61cac4c49c93@redhat.com> From: Jim Mattson Date: Mon, 18 Sep 2017 08:56:39 -0700 Message-ID: Subject: Re: [PATCH] KVM: x86: Fix the NULL pointer parameter in check_cr_write() To: David Hildenbrand Cc: Yu Zhang , kvm list , LKML , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1971 Lines: 51 kvm_cpuid ultimately wants to write all four of the GPRs passed in by reference. I don't see any advantage to allowing some of these pointers to be NULL. Reviewed-by: Jim Mattson On Mon, Sep 18, 2017 at 5:19 AM, David Hildenbrand wrote: > On 18.09.2017 12:45, Yu Zhang wrote: >> Routine check_cr_write() will trigger emulator_get_cpuid()-> >> kvm_cpuid() to get maxphyaddr, and NULL is passed as values >> for ebx/ecx/edx. This is problematic because kvm_cpuid() will >> dereference these pointers. >> >> Fixes: d1cd3ce90044 ("KVM: MMU: check guest CR3 reserved bits based on its physical address width.") >> Reported-by: Jim Mattson >> Signed-off-by: Yu Zhang >> --- >> arch/x86/kvm/emulate.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index 16bf665..15f527b 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -4102,10 +4102,12 @@ static int check_cr_write(struct x86_emulate_ctxt *ctxt) >> ctxt->ops->get_msr(ctxt, MSR_EFER, &efer); >> if (efer & EFER_LMA) { >> u64 maxphyaddr; >> - u32 eax = 0x80000008; >> + u32 eax, ebx, ecx, edx; >> >> - if (ctxt->ops->get_cpuid(ctxt, &eax, NULL, NULL, >> - NULL, false)) >> + eax = 0x80000008; >> + ecx = 0; >> + if (ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, >> + &edx, false)) >> maxphyaddr = eax & 0xff; >> else >> maxphyaddr = 36; >> > > Not sure if fixing kvm_cpuid() would be better. > > Reviewed-by: David Hildenbrand > > -- > > Thanks, > > David