From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751818AbdAYJ5c (ORCPT ); Wed, 25 Jan 2017 04:57:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50166 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751501AbdAYJ5a (ORCPT ); Wed, 25 Jan 2017 04:57:30 -0500 Subject: Re: [PATCH] kvm: fix page struct leak in handle_vmon To: Paolo Bonzini References: <1485255381-18069-1-git-send-email-pbonzini@redhat.com> <0cdfcfea-f1f6-fbcb-887a-dae659e2b844@redhat.com> <692606753.12365942.1485337926527.JavaMail.zimbra@redhat.com> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, dvyukov@google.com From: David Hildenbrand Organization: Red Hat GmbH Message-ID: Date: Wed, 25 Jan 2017 10:57:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <692606753.12365942.1485337926527.JavaMail.zimbra@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 25 Jan 2017 09:57:31 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 25.01.2017 um 10:52 schrieb Paolo Bonzini: > > > ----- Original Message ----- >> From: "David Hildenbrand" >> To: "Paolo Bonzini" , linux-kernel@vger.kernel.org, kvm@vger.kernel.org >> Cc: dvyukov@google.com >> Sent: Wednesday, January 25, 2017 10:31:13 AM >> Subject: Re: [PATCH] kvm: fix page struct leak in handle_vmon >> >> Am 24.01.2017 um 11:56 schrieb Paolo Bonzini: >>> handle_vmon gets a reference on VMXON region page, >>> but does not release it. Release the reference. >>> >>> Found by syzkaller; based on a patch by Dmitry. >>> >>> Reported-by: Dmitry Vyukov >>> Signed-off-by: Paolo Bonzini >>> --- >>> arch/x86/kvm/vmx.c | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> index 42cc3d6f4d20..0f7345035210 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -7085,13 +7085,18 @@ static int nested_vmx_check_vmptr(struct kvm_vcpu >>> *vcpu, int exit_reason, >>> } >>> >>> page = nested_get_page(vcpu, vmptr); >>> - if (page == NULL || >>> - *(u32 *)kmap(page) != VMCS12_REVISION) { >>> + if (page == NULL) { >>> nested_vmx_failInvalid(vcpu); >>> + return kvm_skip_emulated_instruction(vcpu); >>> + } >>> + if (*(u32 *)kmap(page) != VMCS12_REVISION) { >> >> shouldn't we also check if kmap even returned a valid pointer before >> dereferencing it? > > I don't think kmap can fail (page_address can)? Then I wonder why there are some checks: e.g. nested_vmx_merge_msr_bitmap() msr_bitmap_l1 = (unsigned long *)kmap(page); if (!msr_bitmap_l1) { // no unmap ... return false; or vmx_complete_nested_posted_interrupt() vapic_page = kmap(vmx->nested.virtual_apic_page); if (!vapic_page) { // no unmap ... return -ENOMEM; But there is also no check in handle_vmptrld() for example. > > Paolo -- David