KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
From: Quentin Perret <qperret@google.com>
To: Will Deacon <will@kernel.org>
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: Wed, 22 Jul 2020 16:54:28 +0100
Message-ID: <20200722155428.GA275809@google.com> (raw)
In-Reply-To: <20200722131511.14639-1-will@kernel.org>

Hey Will,

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.

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.

>  }
> @@ -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. So, with the above
applied:

Reviewed-by: Quentin Perret <qperret@google.com>
Tested-by: Quentin Perret <qperret@google.com>

Thanks,
Quentin
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

  reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-22 13:15 Will Deacon
2020-07-22 15:54 ` Quentin Perret [this message]
2020-07-23 10:08   ` Will Deacon

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=20200722155428.GA275809@google.com \
    --to=qperret@google.com \
    --cc=kernel-team@android.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=will@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

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu
	public-inbox-index kvmarm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git