Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752497AbdGEI5Z (ORCPT ); Wed, 5 Jul 2017 04:57:25 -0400 Received: from foss.arm.com ([217.140.101.70]:53408 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752452AbdGEI5X (ORCPT ); Wed, 5 Jul 2017 04:57:23 -0400 Date: Wed, 5 Jul 2017 09:57:00 +0100 From: Suzuki K Poulose To: Alexander Graf Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Christoffer Dall , Andrea Arcangeli , stable@vger.kernel.org Subject: Re: [PATCH v2] KVM: arm/arm64: Handle hva aging while destroying the vm Message-ID: <20170705085700.GA16881@e107814-lin.cambridge.arm.com> References: <1499235631-141725-1-git-send-email-agraf@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1499235631-141725-1-git-send-email-agraf@suse.de> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2881 Lines: 91 Hi Alex, On Wed, Jul 05, 2017 at 08:20:31AM +0200, Alexander Graf wrote: > The kvm_age_hva callback may be called all the way concurrently while > kvm_mmu_notifier_release() is running. > > The release function sets kvm->arch.pgd = NULL which the aging function > however implicitly relies on in stage2_get_pud(). That means they can > race and the aging function may dereference a NULL pgd pointer. > > This patch adds a check for that case, so that we leave the aging > function silently. > > Cc: stable@vger.kernel.org > Fixes: 293f29363 ("kvm-arm: Unmap shadow pagetables properly") > Signed-off-by: Alexander Graf > > --- > > v1 -> v2: > > - Fix commit message > - Add Fixes and stable tags > --- > virt/kvm/arm/mmu.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c > index f2d5b6c..227931f 100644 > --- a/virt/kvm/arm/mmu.c > +++ b/virt/kvm/arm/mmu.c > @@ -861,6 +861,10 @@ static pud_t *stage2_get_pud(struct kvm *kvm, struct kvm_mmu_memory_cache *cache > pgd_t *pgd; > pud_t *pud; > > + /* Do we clash with kvm_free_stage2_pgd()? */ > + if (!kvm->arch.pgd) > + return NULL; > + I think this check should be moved up in the chain. We call kvm_age_hva(), with the kvm->mmu_lock held and we don't release it till we reach here. So, ideally, if we find the PGD is null when we reach kvm_age_hva(), we could simply return there, like we do for other call backs from the KVM mmu_notifier. ----8>---- kvm-arm: Handle hva aging while destroying vm The mmu_notifier_release() callback of KVM triggers cleaning up the stage2 page table on kvm-arm. However there could be other notifier callbacks in parallel with the mmu_notifier_release(), which could cause the call backs ending up in an empty stage2 page table. Make sure we check it for all the notifier callbacks. Reported-by: Alex Graf Cc: Christoffer Dall Cc: Marc Zyngier Cc: stable@vger.kernel.org Fixes: commit 293f29363 ("kvm-arm: Unmap shadow pagetables properly") Signed-off-by: Suzuki K Poulose --- virt/kvm/arm/mmu.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c index e2e5eff..db1c7b2 100644 --- a/virt/kvm/arm/mmu.c +++ b/virt/kvm/arm/mmu.c @@ -1665,12 +1665,16 @@ static int kvm_test_age_hva_handler(struct kvm *kvm, gpa_t gpa, u64 size, void * int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end) { + if (!kvm->arch.pgd) + return 0; trace_kvm_age_hva(start, end); return handle_hva_to_gpa(kvm, start, end, kvm_age_hva_handler, NULL); } int kvm_test_age_hva(struct kvm *kvm, unsigned long hva) { + if (!kvm->arch.pgd) + return 0; trace_kvm_test_age_hva(hva); return handle_hva_to_gpa(kvm, hva, hva, kvm_test_age_hva_handler, NULL); } -- 2.7.5