From patchwork Wed May 3 14:17:52 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Suzuki K Poulose X-Patchwork-Id: 758085 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wJ0d22CkDz9rxw for ; Thu, 4 May 2017 00:18:30 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="XV8NclVa"; dkim-atps=neutral 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=gjGb701PZ2sFPTqrLeThjAvtateuzmpCHL1Cz8DN5lU=; b=XV8NclVa/Qv1+n urNdU5LdorLJ8da+CHlM3IH6MQ6Iv4L97otj+4S3utuxUsx2IqM9gqH5siN7x4bosVWm1g+ifuKTP RuadtbNA18v0scFd5DYCeXbAVpze8n62OrO8QWZsJwdvKudqDWSHL2jS28FFsoEiyxlHJ8HniUPht /bjK/Z6+gFio5fQQ9u9hYfB9F94qLjU3i4IoWrglTWFLhKs17z0pu4x/w8gWvlZc+AlX7inFHnDMd cW6kBHodmp44Gps5t6y8ydlvN+ObkYIs0w5KQAFvUgUKHm1ykzIL93h5z/Z2q7QMtsILA99Z7VczD W7DvC3veIoc/VSe+49og==; 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 1d5v6m-0000oF-Bi; Wed, 03 May 2017 14:18:24 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1d5v6j-0000mg-Jp for linux-arm-kernel@lists.infradead.org; Wed, 03 May 2017 14:18:23 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DA03F1509; Wed, 3 May 2017 07:18:04 -0700 (PDT) Received: from e107814-lin.cambridge.arm.com (e107814-lin.cambridge.arm.com [10.1.206.28]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 0A2643F3E1; Wed, 3 May 2017 07:18:02 -0700 (PDT) From: Suzuki K Poulose To: christoffer.dall@linaro.org Subject: [PATCH 2/2] kvm: arm/arm64: Fix use after free of stage2 page table Date: Wed, 3 May 2017 15:17:52 +0100 Message-Id: <1493821072-29713-3-git-send-email-suzuki.poulose@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1493821072-29713-1-git-send-email-suzuki.poulose@arm.com> References: <1493821072-29713-1-git-send-email-suzuki.poulose@arm.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170503_071821_677545_A07B292A X-CRM114-Status: GOOD ( 12.26 ) X-Spam-Score: -6.9 (------) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-6.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [217.140.101.70 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, kvm@vger.kernel.org, rkrcmar@redhat.com, marc.zyngier@arm.com, andreyknvl@google.com, suzuki.poulose@arm.com, agraf@suse.de, linux-kernel@vger.kernel.org, pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org We yield the kvm->mmu_lock occassionaly while performing an operation (e.g, unmap or permission changes) on a large area of stage2 mappings. However this could possibly cause another thread to clear and free up the stage2 page tables while we were waiting for regaining the lock and thus the original thread could end up in accessing memory that was freed. This patch fixes the problem by making sure that the stage2 pagetable is still valid after we regain the lock. The fact that mmu_notifer->release() could be called twice (via __mmu_notifier_release and mmu_notifier_unregsister) enhances the possibility of hitting this race where there are two threads trying to unmap the entire guest shadow pages. While at it, cleanup the redudant checks around cond_resched_lock in stage2_wp_range(), as cond_resched_lock already does the same checks. Cc: Mark Rutland Cc: Radim Krčmář Cc: andreyknvl@google.com Cc: Christoffer Dall Cc: Marc Zyngier Cc: Paolo Bonzini Signed-off-by: Suzuki K Poulose Reviewed-by: Marc Zyngier --- arch/arm/kvm/mmu.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 909a1a7..5b3e0db 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -301,9 +301,14 @@ static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size) /* * If the range is too large, release the kvm->mmu_lock * to prevent starvation and lockup detector warnings. + * Make sure the page table is still active when we regain + * the lock. */ - if (next != end) + if (next != end) { cond_resched_lock(&kvm->mmu_lock); + if (!READ_ONCE(kvm->arch.pgd)) + break; + } } while (pgd++, addr = next, addr != end); } @@ -1170,11 +1175,13 @@ static void stage2_wp_range(struct kvm *kvm, phys_addr_t addr, phys_addr_t end) * large. Otherwise, we may see kernel panics with * CONFIG_DETECT_HUNG_TASK, CONFIG_LOCKUP_DETECTOR, * CONFIG_LOCKDEP. Additionally, holding the lock too long - * will also starve other vCPUs. + * will also starve other vCPUs. We have to also make sure + * that the page tables are not freed while we released + * the lock. */ - if (need_resched() || spin_needbreak(&kvm->mmu_lock)) - cond_resched_lock(&kvm->mmu_lock); - + cond_resched_lock(&kvm->mmu_lock); + if (!READ_ONCE(kvm->arch.pgd)) + break; next = stage2_pgd_addr_end(addr, end); if (stage2_pgd_present(*pgd)) stage2_wp_puds(pgd, addr, next);