From patchwork Thu May 18 09:47:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 9733185 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2F66D601BC for ; Thu, 18 May 2017 10:16:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 198DF287A3 for ; Thu, 18 May 2017 10:16:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0E116287BE; Thu, 18 May 2017 10:16:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6D388287A3 for ; Thu, 18 May 2017 10:16:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Xd1NECLMKfx4Nkudl/6bZz010QNxTM/TbbprPXGR+l0=; b=TioH3tc2xmrIO2 bLBKy5o9vFi+tgctPHWttmQcscIRcURy6PQgJYK2bwhL3VUQbJU8m8TeNrWgUCTqZHmCyhRrQuJoc hSYkV9jUzeuuQwR6HnyFmuqvPJ3n8XNeIRPPKeUGUaP4vBUnOGHJB9y8sednr/wVdWFYyOaOO4SZE 9+EmMI5e650OPoWgSbm/5USROXZZDUv5nZyw6d8DrJuhCV+6Qywq+VE47oCkBVFhshZG70ljwdjPQ Zf9tCF1uT4l6HuXa8YBsQ/VSU+yQ4pxwTUFxAObeDss5RhBiwvdcjTYluExlwNwCL8i25IrJIAV8k d1Th//GjwUrR+LKECFwQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dBIUF-00076O-An; Thu, 18 May 2017 10:16:51 +0000 Received: from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dBI2P-0005LJ-P9 for linux-arm-kernel@lists.infradead.org; Thu, 18 May 2017 09:48:15 +0000 Received: by mail-wm0-x22f.google.com with SMTP id 70so40079282wmq.1 for ; Thu, 18 May 2017 02:47:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=LTuhpDmPRbmEUcqj+lYAddJkbX/zFKHVvmYKyEnD8bI=; b=QAH5T/D+3gWoEptUVLOztAZMN2dwjXSzBJonQJ4TbZMfwa7nLj+3WCx2Bpc9GIkFSB zMLiw44Vrw4fJKdIwY2iYWjJS2TTn75UsD2ZrDc7pzmDgeOs1aLalyOf2l84GWbryVRN wKNjtSQqo+XWVrwwQbj6jnkTpRC6dFqJvtn6s= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=LTuhpDmPRbmEUcqj+lYAddJkbX/zFKHVvmYKyEnD8bI=; b=VF42LndVuCzsgiDY9YBtgp4bSJhYSe035bQih3WLoJeUDYQ0+2hZY0cVtnmI20nNGl QvVnQ0FQWNsYhy6XwFuQ8BTPHhtmKR9+Suns0GmVx+uz+X9bdPxPeeq596FNlT8ElTFP juXu8Maj4jbQVgav9+t3D0N/iUrSHIDGmtjrjInCjiFEN/IgV0ovYCJ6lPw/uXIW28KP yveUXjdikmJZBwoCi/m8qvXV64im+S2H+90sLRZFwhwhERZOIUujXzIDB1Byv8HDAXsp KSsQZEBqhfNZQtatGujzej7P9u+eOUwE71RdLFwySqXBWfq1ZrqlLZWBiwCbOui+yTtQ LYDQ== X-Gm-Message-State: AODbwcC/rfa5rAZPMF8Ijm5Ks00hfDP0vrVezlF7qIK7rigBGEW+kp3I 2DijpugyP+raebm9sCbzqQ== X-Received: by 10.80.131.67 with SMTP id 61mr2629552edh.21.1495100863981; Thu, 18 May 2017 02:47:43 -0700 (PDT) Received: from localhost.localdomain (xd93ddc2d.cust.hiper.dk. [217.61.220.45]) by smtp.gmail.com with ESMTPSA id w15sm2377437edw.27.2017.05.18.02.47.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 18 May 2017 02:47:43 -0700 (PDT) From: Christoffer Dall To: Paolo Bonzini , =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Subject: [PULL 07/13] kvm: arm/arm64: Fix race in resetting stage2 PGD Date: Thu, 18 May 2017 11:47:16 +0200 Message-Id: <20170518094722.9926-8-cdall@linaro.org> X-Mailer: git-send-email 2.9.0 In-Reply-To: <20170518094722.9926-1-cdall@linaro.org> References: <20170518094722.9926-1-cdall@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170518_024806_269682_473042DD X-CRM114-Status: GOOD ( 15.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Christoffer Dall , kvm@vger.kernel.org, Suzuki K Poulose , Marc Zyngier , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP From: Suzuki K Poulose In kvm_free_stage2_pgd() we check the stage2 PGD before holding the lock and proceed to take the lock if it is valid. And we unmap the page tables, followed by releasing the lock. We reset the PGD only after dropping this lock, which could cause a race condition where another thread waiting on or even holding the lock, could potentially see that the PGD is still valid and proceed to perform a stage2 operation and later encounter a NULL PGD. [223090.242280] Unable to handle kernel NULL pointer dereference at virtual address 00000040 [223090.262330] PC is at unmap_stage2_range+0x8c/0x428 [223090.262332] LR is at kvm_unmap_hva_handler+0x2c/0x3c [223090.262531] Call trace: [223090.262533] [] unmap_stage2_range+0x8c/0x428 [223090.262535] [] kvm_unmap_hva_handler+0x2c/0x3c [223090.262537] [] handle_hva_to_gpa+0xb0/0x104 [223090.262539] [] kvm_unmap_hva+0x5c/0xbc [223090.262543] [] kvm_mmu_notifier_invalidate_page+0x50/0x8c [223090.262547] [] __mmu_notifier_invalidate_page+0x5c/0x84 [223090.262551] [] try_to_unmap_one+0x1d0/0x4a0 [223090.262553] [] rmap_walk+0x1cc/0x2e0 [223090.262555] [] try_to_unmap+0x74/0xa4 [223090.262557] [] migrate_pages+0x31c/0x5ac [223090.262561] [] compact_zone+0x3fc/0x7ac [223090.262563] [] compact_zone_order+0x94/0xb0 [223090.262564] [] try_to_compact_pages+0x108/0x290 [223090.262569] [] __alloc_pages_direct_compact+0x70/0x1ac [223090.262571] [] __alloc_pages_nodemask+0x434/0x9f4 [223090.262572] [] alloc_pages_vma+0x230/0x254 [223090.262574] [] do_huge_pmd_anonymous_page+0x114/0x538 [223090.262576] [] handle_mm_fault+0xd40/0x17a4 [223090.262577] [] __get_user_pages+0x12c/0x36c [223090.262578] [] get_user_pages_unlocked+0xa4/0x1b8 [223090.262579] [] __gfn_to_pfn_memslot+0x280/0x31c [223090.262580] [] gfn_to_pfn_prot+0x4c/0x5c [223090.262582] [] kvm_handle_guest_abort+0x240/0x774 [223090.262584] [] handle_exit+0x11c/0x1ac [223090.262586] [] kvm_arch_vcpu_ioctl_run+0x31c/0x648 [223090.262587] [] kvm_vcpu_ioctl+0x378/0x768 [223090.262590] [] do_vfs_ioctl+0x324/0x5a4 [223090.262591] [] SyS_ioctl+0x90/0xa4 [223090.262595] [] el0_svc_naked+0x38/0x3c This patch moves the stage2 PGD manipulation under the lock. Reported-by: Alexander Graf Cc: Mark Rutland Cc: Marc Zyngier Cc: Paolo Bonzini Cc: Radim Krčmář Reviewed-by: Christoffer Dall Reviewed-by: Marc Zyngier Signed-off-by: Suzuki K Poulose Signed-off-by: Christoffer Dall --- virt/kvm/arm/mmu.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c index 313ee64..909a1a7 100644 --- a/virt/kvm/arm/mmu.c +++ b/virt/kvm/arm/mmu.c @@ -829,22 +829,22 @@ void stage2_unmap_vm(struct kvm *kvm) * Walks the level-1 page table pointed to by kvm->arch.pgd and frees all * underlying level-2 and level-3 tables before freeing the actual level-1 table * and setting the struct pointer to NULL. - * - * Note we don't need locking here as this is only called when the VM is - * destroyed, which can only be done once. */ void kvm_free_stage2_pgd(struct kvm *kvm) { - if (kvm->arch.pgd == NULL) - return; + void *pgd = NULL; spin_lock(&kvm->mmu_lock); - unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); + if (kvm->arch.pgd) { + unmap_stage2_range(kvm, 0, KVM_PHYS_SIZE); + pgd = kvm->arch.pgd; + kvm->arch.pgd = NULL; + } spin_unlock(&kvm->mmu_lock); /* Free the HW pgd, one page at a time */ - free_pages_exact(kvm->arch.pgd, S2_PGD_SIZE); - kvm->arch.pgd = NULL; + if (pgd) + free_pages_exact(pgd, S2_PGD_SIZE); } static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,