From: Will Deacon <will@kernel.org>
To: Quentin Perret <qperret@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
stable@vger.kernel.org, kernel-team@android.com,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] KVM: arm64: Don't inherit exec permission across page-table levels
Date: Thu, 23 Jul 2020 11:08:44 +0100 [thread overview]
Message-ID: <20200723100843.GA15711@willie-the-truck> (raw)
In-Reply-To: <20200722155428.GA275809@google.com>
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 <qperret@google.com>
> Tested-by: Quentin Perret <qperret@google.com>
Cheers. v2 coming up.
Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
prev parent reply other threads:[~2020-07-23 10:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-22 13:15 [PATCH] KVM: arm64: Don't inherit exec permission across page-table levels Will Deacon
2020-07-22 15:54 ` Quentin Perret
2020-07-23 10:08 ` Will Deacon [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200723100843.GA15711@willie-the-truck \
--to=will@kernel.org \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=qperret@google.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).