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 X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F39EC433E1 for ; Thu, 23 Jul 2020 10:10:19 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CAEE82065F for ; Thu, 23 Jul 2020 10:10:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="xXwEnxzL"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="znBEr0fj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CAEE82065F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=7wm0wrznH27WbcCmcB8mNLqqCeMZCX2LUsHUUiHZzgE=; b=xXwEnxzLa/SdDvLe8flRoWx55 rkeaS5B26iTYDG3tFTEPutBq12pbK7UA2JSH9YPFImGd3a+Q+IU19RYpRvT1+r0IiMcNeZTt/fAv6 /ylBbjyv0KERQR5/SjGsERt4lUqbK5qRH5J08DjitNYFZMT2+2+y07w4fYp0UowwhT5GXP8R0pCxn 9X5CYDr1BTz6ll+tGylULYki3iumO4PAd4kNvZPbQJK/Z1eSEDW/K9PwlMg3d6huSCyBHOVoZJoN+ TiP3M3n67Fj3TSOK0lKSYMbfuWGPgtxCKIx9JCACE/EO230GCKnCEvT/Wb6yjPxF6DKQvC9QMaEnm iu79ioGWg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyY9j-00041I-VC; Thu, 23 Jul 2020 10:08:52 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyY9h-00040L-K0 for linux-arm-kernel@lists.infradead.org; Thu, 23 Jul 2020 10:08:50 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 4CBB22086A; Thu, 23 Jul 2020 10:08:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595498928; bh=AML2GZZ2Ph91q1oR/Eg49IMw55Q5QbxKvzvu1iz4bIg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=znBEr0fjlV5rdxomE2c2cQfMuQCavRErZqYac5EeyDHjYb8f0l2fdRZsHNZmfC2wq yy/ZPIbgy+ilYflMX3t24hpAMnehRlU3O/PJIcrtIvDsQtxVLtJH/IKUVkubqYQZ+J lp0pMsjHDo6n8wGKQefFPgy22LP0Efb/r3nN81AE= Date: Thu, 23 Jul 2020 11:08:44 +0100 From: Will Deacon To: Quentin Perret Subject: Re: [PATCH] KVM: arm64: Don't inherit exec permission across page-table levels Message-ID: <20200723100843.GA15711@willie-the-truck> References: <20200722131511.14639-1-will@kernel.org> <20200722155428.GA275809@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200722155428.GA275809@google.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200723_060849_769906_DF10CB14 X-CRM114-Status: GOOD ( 18.19 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: suzuki.poulose@arm.com, Marc Zyngier , stable@vger.kernel.org, james.morse@arm.com, kernel-team@android.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org 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 Hi Quentin, On Wed, Jul 22, 2020 at 04:54:28PM +0100, Quentin Perret wrote: > On Wednesday 22 Jul 2020 at 14:15:10 (+0100), Will Deacon wrote: > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > > index 8c0035cab6b6..69dc36d1d486 100644 > > --- a/arch/arm64/kvm/mmu.c > > +++ b/arch/arm64/kvm/mmu.c > > @@ -1326,7 +1326,7 @@ static bool stage2_get_leaf_entry(struct kvm *kvm, phys_addr_t addr, > > return true; > > } > > > > -static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr) > > +static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr, unsigned long sz) > > { > > pud_t *pudp; > > pmd_t *pmdp; > > @@ -1338,9 +1338,9 @@ static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr) > > return false; > > > > if (pudp) > > - return kvm_s2pud_exec(pudp); > > + return sz == PUD_SIZE && kvm_s2pud_exec(pudp); > > else if (pmdp) > > - return kvm_s2pmd_exec(pmdp); > > + return sz == PMD_SIZE && kvm_s2pmd_exec(pmdp); > > else > > return kvm_s2pte_exec(ptep); > > This wants a 'sz == PAGE_SIZE' check, otherwise you'll happily inherit > the exec flag when a PTE has exec rights while you create a block > mapping on top. Nice catch! Somehow I thought we always had PAGE_SIZE in the 'else' case, but that's obviously not true now that you've pointed it out. > Also, I think it should be safe to make the PMD and PUD case more > permissive, as 'sz <= PMD_SIZE' for instance, as the icache > invalidation shouldn't be an issue there? That probably doesn't matter > all that much though. I'll make that change anyway. > > @@ -1958,7 +1958,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > > * execute permissions, and we preserve whatever we have. > > */ > > needs_exec = exec_fault || > > - (fault_status == FSC_PERM && stage2_is_exec(kvm, fault_ipa)); > > + (fault_status == FSC_PERM && > > + stage2_is_exec(kvm, fault_ipa, vma_pagesize)); > > > > if (vma_pagesize == PUD_SIZE) { > > pud_t new_pud = kvm_pfn_pud(pfn, mem_type); > > -- > > 2.28.0.rc0.105.gf9edc3c819-goog > > > > FWIW, I reproduced the issue with a dummy guest accessing memory just > the wrong way, and toggling dirty logging at the right moment. And this > patch + my suggestion above seems to cure things. Testing?! It'll never catch on... > So, with the above applied: > > Reviewed-by: Quentin Perret > Tested-by: Quentin Perret Cheers. v2 coming up. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel