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 <[email protected]>
Tested-by: Quentin Perret <[email protected]>

Thanks,
Quentin
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to