From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1953443AbdDYStb (ORCPT ); Tue, 25 Apr 2017 14:49:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42972 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1946272AbdDYStZ (ORCPT ); Tue, 25 Apr 2017 14:49:25 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 331E746D0A6 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=rkrcmar@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 331E746D0A6 Date: Tue, 25 Apr 2017 20:49:05 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Suzuki K Poulose Cc: pbonzini@redhat.com, christoffer.dall@linaro.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, marc.zyngier@arm.com, mark.rutland@arm.com, andreyknvl@google.com Subject: Re: [PATCH 1/2] kvm: Fix mmu_notifier release race Message-ID: <20170425184904.GI5713@potion> References: <1493028624-29837-1-git-send-email-suzuki.poulose@arm.com> <1493028624-29837-2-git-send-email-suzuki.poulose@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1493028624-29837-2-git-send-email-suzuki.poulose@arm.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 25 Apr 2017 18:49:10 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-04-24 11:10+0100, Suzuki K Poulose: > The KVM uses mmu_notifier (wherever available) to keep track > of the changes to the mm of the guest. The guest shadow page > tables are released when the VM exits via mmu_notifier->ops.release(). > There is a rare chance that the mmu_notifier->release could be > called more than once via two different paths, which could end > up in use-after-free of kvm instance (such as [0]). > > e.g: > > thread A thread B > ------- -------------- > > get_signal-> kvm_destroy_vm()-> > do_exit-> mmu_notifier_unregister-> > exit_mm-> kvm_arch_flush_shadow_all()-> > exit_mmap-> spin_lock(&kvm->mmu_lock) > mmu_notifier_release-> .... > kvm_arch_flush_shadow_all()-> ..... > ... spin_lock(&kvm->mmu_lock) ..... > spin_unlock(&kvm->mmu_lock) > kvm_arch_free_kvm() > *** use after free of kvm *** I don't understand this race ... a piece of code in mmu_notifier_unregister() says: /* * Wait for any running method to finish, of course including * ->release if it was run by mmu_notifier_release instead of us. */ synchronize_srcu(&srcu); and code before that removes the notifier from the list, so it cannot be called after we pass this point. mmu_notifier_release() does roughly the same and explains it as: /* * synchronize_srcu here prevents mmu_notifier_release from returning to * exit_mmap (which would proceed with freeing all pages in the mm) * until the ->release method returns, if it was invoked by * mmu_notifier_unregister. * * The mmu_notifier_mm can't go away from under us because one mm_count * is held by exit_mmap. */ synchronize_srcu(&srcu); The call of mmu_notifier->release is protected by srcu in both cases and while it seems possible that mmu_notifier->release would be called twice, I don't see a combination that could result in use-after-free from mmu_notifier_release after mmu_notifier_unregister() has returned. Doesn't [2/2] solve the exact same issue (that the release method cannot be called twice in parallel)? Thanks.