Re: [PATCH 3/3] KVM: arm64: Sanitise MTE tags when a permission fault installs a mapping

[Lorenzo Stoakes (ARM)]

On Mon, Oct 19, 2026 at 02:21:03PM +0100, Fuad Tabba wrote:
> The stage-2 fault path sanitises MTE tags only on translation faults, as
> a permission fault normally just relaxes an existing leaf. However, when
> the page is backed by a THP, a permission fault at page granule installs
> a block whose other pages may never have been mapped or sanitised. Once
> dirty logging is off, a guest write to a page mapped read-only during
> logging installs the block, and the guest can read the tags those pages'
> previous owners left.
>
> Sanitise whenever the fault path calls kvm_pgtable_stage2_map() rather
> than relaxing permissions. If the VMM replaces the VMA after the guest
> takes a permission fault but before vma_lookup(), that path would map
> and sanitise memory from the new VMA without checking it. Run the
> VM_MTE_ALLOWED check in kvm_s2_fault_compute_prot() on permission
> faults too.
>
> Fixes: ea7fc1bb1cd1b ("KVM: arm64: Introduce MTE VM feature")

Yikes! It goes quite far back :)

> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>

Nits below, but with those addressed LGTM and:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

> ---
>  arch/arm64/kvm/mmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 48a08b608ea72..208fcae80f67f 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -2005,7 +2005,7 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
>  	if (s2fd->nested)
>  		*prot = adjust_nested_exec_perms(kvm, s2fd->nested, *prot);
>
> -	if (!kvm_s2_fault_is_perm(s2fd) && !s2vi->map_non_cacheable && kvm_has_mte(kvm)) {
> +	if (!s2vi->map_non_cacheable && kvm_has_mte(kvm)) {

OK so doing this check on permission faults too.

>  		/* Check the VMM hasn't introduced a new disallowed VMA */
>  		if (!s2vi->mte_allowed)
>  			return -EFAULT;
> @@ -2060,7 +2060,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
>  		}
>  	}
>
> -	if (!perm_fault_granule && !s2vi->map_non_cacheable && kvm_has_mte(kvm))
> +	if (mapping_size != perm_fault_granule && !s2vi->map_non_cacheable && kvm_has_mte(kvm))

So 'translation fault or permission fault if this will install a mapping' rather
than 'translation fault' alone right?

Just after this code you have:

	if (mapping_size == perm_fault_granule) {
		...
	} else {
		...
	}

So you could put

		if (!s2vi->map_non_cacheable && kvm_has_mte(kvm))
			sanitise_mte_tags(kvm, pfn, mapping_size);

In the else branch? Seems neater and clearer.

Also I wonder if the !s2vi->map_non_cacheable && kvm_has_mte(kvm) check could be
abstracted?

E.g.:

static bool is_mte_fault(const struct kvm *kvm,
		         const struct kvm_s2_fault_vma_info *s2vi)
{
	return !s2vi->map_non_cacheable && kvm_has_mte(kvm);
}

Then could be:

	if (is_mte_fault(kvm, s2vi))
		sanitise_mte_tags(kvm, pfn, mapping_size);

>  		sanitise_mte_tags(kvm, pfn, mapping_size);
>
>  	/*
> --
> 2.47.3
>
>

--
Cheers, Lorenzo