Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752794AbdGFHHz (ORCPT ); Thu, 6 Jul 2017 03:07:55 -0400 Received: from mx2.suse.de ([195.135.220.15]:45917 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750922AbdGFHHy (ORCPT ); Thu, 6 Jul 2017 03:07:54 -0400 Subject: Re: [PATCH v2] KVM: arm/arm64: Handle hva aging while destroying the vm To: Suzuki K Poulose Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Christoffer Dall , Andrea Arcangeli , stable@vger.kernel.org References: <1499235631-141725-1-git-send-email-agraf@suse.de> <20170705085700.GA16881@e107814-lin.cambridge.arm.com> From: Alexander Graf Message-ID: Date: Thu, 6 Jul 2017 09:07:49 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170705085700.GA16881@e107814-lin.cambridge.arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1747 Lines: 54 On 05.07.17 10:57, Suzuki K Poulose wrote: > 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. That probably works too - I'm not sure which version is more consistent as well as more maintainable in the long run. I'll leave the call here to Christoffer. Alex