From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 646E7C6FD19 for ; Mon, 13 Mar 2023 17:41:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=PUNwlFnvkhGCaMWyzw3gstbL1VilbKXUOjlI9sRtG/U=; b=DLooPp0OWRUN9/ WSCFiiJlPgr7zLlQhWRRTXbIqjuU/qbxEtoZC81VzwC+bImxzvcl3p2AQV+JMZCy+7criITWcDv23 5xFEwB1XCf+ajWPjwIUObfeJUbxl8jZsB7OpplGecXP9vWh06bhBcEPM7skQKIR6RgiFzAxsLB5r9 zfyJ8VcrrJXWFqeHoWqXqHXpgiv/Pf5JITLC6XhcFQR+kZz7veH66aVo7ZaRlTJSyfHhX5XMd/7sU kxeWBDaDPHECj57iiLYuPkRfJk8vIGP66oSxaoa+CRCQx6Jmc0UK1c9m6QuQhwuGHVc2CjWQmgoR+ NreqCA4JLwiqrJMFbrag==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pbm9w-007AiJ-16; Mon, 13 Mar 2023 17:40:32 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pbm9s-007Ahd-Nm for linux-arm-kernel@lists.infradead.org; Mon, 13 Mar 2023 17:40:30 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 5DD5FB811A1; Mon, 13 Mar 2023 17:40:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 228E6C433D2; Mon, 13 Mar 2023 17:40:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1678729225; bh=HGclvf2eMYSO2tUhOiMvSKCDztLmiMtZOfMVb6g6DVk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HmZWuJL81X+QwZ5k7rXig9TGU2O6rtggZA1Wyv18AEy74H/w+I50F3BjrViWg3tBC MSiT1FiuUy6TYON8kwfIa11YdTqevEHWpABQuMQlm4GduLoP4ijgCskTYnhjfxGKnw j1Exb8c+QLFy6fGlqXm4ubrz1Xc0hIjcLQMhrxDOCwECaWuaghRrYjZl/PvZEDJR3l fesJmClEcaisFb1zwZBGSHXRdUJkNKnF9bBQmxO9c8bIOB16i15R3qF8tQiUKRn8Ol dx/qAj2c1bfBMY3SbVtUzstOFKWH4mBH2VhhmaQniqYAjUQyfS06fKVeMXkzpwGAC7 MGg1XslgtDBQw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pbm9m-00HKV1-MV; Mon, 13 Mar 2023 17:40:22 +0000 Date: Mon, 13 Mar 2023 17:40:22 +0000 Message-ID: <86lek0y2gp.wl-maz@kernel.org> From: Marc Zyngier To: Sean Christopherson Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, James Morse , Suzuki K Poulose , Oliver Upton , Zenghui Yu , Ard Biesheuvel , Will Deacon , Quentin Perret , stable@vger.kernel.org, David Matlack Subject: Re: [PATCH 1/2] KVM: arm64: Disable interrupts while walking userspace PTs In-Reply-To: References: <20230313091425.1962708-1-maz@kernel.org> <20230313091425.1962708-2-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: seanjc@google.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, james.morse@arm.com, suzuki.poulose@arm.com, oliver.upton@linux.dev, yuzenghui@huawei.com, ardb@kernel.org, will@kernel.org, qperret@google.com, stable@vger.kernel.org, dmatlack@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230313_104029_090533_AC10E26F X-CRM114-Status: GOOD ( 40.68 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 13 Mar 2023 15:53:37 +0000, Sean Christopherson wrote: > > +David > > On Mon, Mar 13, 2023, Marc Zyngier wrote: > > We walk the userspace PTs to discover what mapping size was > > used there. However, this can race against the userspace tables > > being freed, and we end-up in the weeds. > > > > Thankfully, the mm code is being generous and will IPI us when > > doing so. So let's implement our part of the bargain and disable > > interrupts around the walk. This ensures that nothing terrible > > happens during that time. > > > > We still need to handle the removal of the page tables before > > the walk. For that, allow get_user_mapping_size() to return an > > error, and make sure this error can be propagated all the way > > to the the exit handler. > > > > Signed-off-by: Marc Zyngier > > Cc: stable@vger.kernel.org > > --- > > arch/arm64/kvm/mmu.c | 35 ++++++++++++++++++++++++++++------- > > 1 file changed, 28 insertions(+), 7 deletions(-) > > > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > > index 7113587222ff..d7b8b25942df 100644 > > --- a/arch/arm64/kvm/mmu.c > > +++ b/arch/arm64/kvm/mmu.c > > @@ -666,14 +666,23 @@ static int get_user_mapping_size(struct kvm *kvm, u64 addr) > > CONFIG_PGTABLE_LEVELS), > > .mm_ops = &kvm_user_mm_ops, > > }; > > + unsigned long flags; > > kvm_pte_t pte = 0; /* Keep GCC quiet... */ > > u32 level = ~0; > > int ret; > > > > + /* > > + * Disable IRQs so that we hazard against a concurrent > > + * teardown of the userspace page tables (which relies on > > + * IPI-ing threads). > > + */ > > + local_irq_save(flags); > > ret = kvm_pgtable_get_leaf(&pgt, addr, &pte, &level); > > - VM_BUG_ON(ret); > > - VM_BUG_ON(level >= KVM_PGTABLE_MAX_LEVELS); > > - VM_BUG_ON(!(pte & PTE_VALID)); > > + local_irq_restore(flags); > > + > > + /* Oops, the userspace PTs are gone... */ > > + if (ret || level >= KVM_PGTABLE_MAX_LEVELS || !(pte & PTE_VALID)) > > + return -EFAULT; > > I don't think this should return -EFAULT all the way out to userspace. Unless > arm64 differs from x86 in terms of how the userspace page tables are managed, not > having a valid translation _right now_ doesn't mean that one can't be created in > the future, e.g. by way of a subsequent hva_to_pfn(). I probably took an overly restrictive approach of only looking at the issue at hand, where exit_mmap() had already torn down the userspace PTs. But I guess there are other ways for this scenario to happen, none of which deserve -EFAULT indeed. > FWIW, the approach x86 takes is to install a 4KiB (smallest granuale) translation, > which is safe since there _was_ a valid translation when mmu_lock was acquired and > mmu_invalidate_retry() was checked. It's the primary MMU's responsibility to ensure > all secondary MMUs are purged before freeing memory, i.e. worst case should be that > KVMs stage-2 translation will be immediately zapped via mmu_notifier. I'd rather avoid extra work. At this stage, we might as well return -EAGAIN and replay the fault. We already do that in a number of racy cases, so it fits in the infrastructure. > KVM ARM also has a bug that might be related: the mmu_seq snapshot needs to be > taken _before_ mmap_read_unlock(), otherwise vma_shift may be stale by the time > it's consumed. I believe David is going to submit a patch (I found and "reported" > the bug when doing an internal review of "common MMU" stuff). Huh, that's interesting. David, please post this at your earliest convenience. I'd rather squash these all in one go. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel