Re: [PATCH] kvm: arm: Enforce PTE mappings at stage2 when needed
Hi Suzuki, On 2019/3/12 17:52, Suzuki K Poulose wrote: commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") made the checks to skip huge mappings, stricter. However it introduced a bug where we still use huge mappings, ignoring the flag to use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE. Also, the checks do not cover the PUD huge pages, that was under review during the same period. This patch fixes both the issues. Fixes : 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") Reported-by: Zenghui Yu Cc: Zenghui Yu Cc: Christoffer Dall Cc: Marc Zyngier Signed-off-by: Suzuki K Poulose --- Thanks for Cc-ing me. It works fine now! zenghui virt/kvm/arm/mmu.c | 43 +-- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c index 30251e2..66e0fbb5 100644 --- a/virt/kvm/arm/mmu.c +++ b/virt/kvm/arm/mmu.c @@ -1595,8 +1595,9 @@ static void kvm_send_hwpoison_signal(unsigned long address, send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current); } -static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, - unsigned long hva) +static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot, + unsigned long hva, + unsigned long map_size) { gpa_t gpa_start, gpa_end; hva_t uaddr_start, uaddr_end; @@ -1612,34 +1613,34 @@ static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, /* * Pages belonging to memslots that don't have the same alignment -* within a PMD for userspace and IPA cannot be mapped with stage-2 -* PMD entries, because we'll end up mapping the wrong pages. +* within a PMD/PUD for userspace and IPA cannot be mapped with stage-2 +* PMD/PUD entries, because we'll end up mapping the wrong pages. * * Consider a layout like the following: * *memslot->userspace_addr: *+-+++---+ -*|abcde|fgh Stage-1 PMD|Stage-1 PMD tv|xyz| +*|abcde|fgh Stage-1 block |Stage-1 block tv|xyz| *+-+++---+ * *memslot->base_gfn << PAGE_SIZE: * +---+++-+ -* |abc|def Stage-2 PMD|Stage-2 PMD |tvxyz| +* |abc|def Stage-2 block |Stage-2 block |tvxyz| * +---+++-+ * -* If we create those stage-2 PMDs, we'll end up with this incorrect +* If we create those stage-2 blocks, we'll end up with this incorrect * mapping: * d -> f * e -> g * f -> h */ - if ((gpa_start & ~S2_PMD_MASK) != (uaddr_start & ~S2_PMD_MASK)) + if ((gpa_start & (map_size - 1)) != (uaddr_start & (map_size - 1))) return false; /* * Next, let's make sure we're not trying to map anything not covered -* by the memslot. This means we have to prohibit PMD size mappings -* for the beginning and end of a non-PMD aligned and non-PMD sized +* by the memslot. This means we have to prohibit block size mappings +* for the beginning and end of a non-block aligned and non-block sized * memory slot (illustrated by the head and tail parts of the * userspace view above containing pages 'abcde' and 'xyz', * respectively). @@ -1648,8 +1649,8 @@ static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, * userspace_addr or the base_gfn, as both are equally aligned (per * the check above) and equally sized. */ - return (hva & S2_PMD_MASK) >= uaddr_start && - (hva & S2_PMD_MASK) + S2_PMD_SIZE <= uaddr_end; + return (hva & ~(map_size - 1)) >= uaddr_start && + (hva & ~(map_size - 1)) + map_size <= uaddr_end; } static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, @@ -1678,12 +1679,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, return -EFAULT; } - if (!fault_supports_stage2_pmd_mappings(memslot, hva)) - force_pte = true; - - if (logging_active) - force_pte = true; - /* Let's check if we will get back a huge page backed by hugetlbfs */ down_read(>mm->mmap_sem); vma = find_vma_intersection(current->mm, hva, hva + 1); @@ -1694,6 +1689,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, } vma_pagesize = vma_kernel_pagesize(vma); + if
[PATCH] kvm: arm: Enforce PTE mappings at stage2 when needed
commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") made the checks to skip huge mappings, stricter. However it introduced a bug where we still use huge mappings, ignoring the flag to use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE. Also, the checks do not cover the PUD huge pages, that was under review during the same period. This patch fixes both the issues. Fixes : 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") Reported-by: Zenghui Yu Cc: Zenghui Yu Cc: Christoffer Dall Cc: Marc Zyngier Signed-off-by: Suzuki K Poulose --- virt/kvm/arm/mmu.c | 43 +-- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c index 30251e2..66e0fbb5 100644 --- a/virt/kvm/arm/mmu.c +++ b/virt/kvm/arm/mmu.c @@ -1595,8 +1595,9 @@ static void kvm_send_hwpoison_signal(unsigned long address, send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current); } -static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, - unsigned long hva) +static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot, + unsigned long hva, + unsigned long map_size) { gpa_t gpa_start, gpa_end; hva_t uaddr_start, uaddr_end; @@ -1612,34 +1613,34 @@ static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, /* * Pages belonging to memslots that don't have the same alignment -* within a PMD for userspace and IPA cannot be mapped with stage-2 -* PMD entries, because we'll end up mapping the wrong pages. +* within a PMD/PUD for userspace and IPA cannot be mapped with stage-2 +* PMD/PUD entries, because we'll end up mapping the wrong pages. * * Consider a layout like the following: * *memslot->userspace_addr: *+-+++---+ -*|abcde|fgh Stage-1 PMD|Stage-1 PMD tv|xyz| +*|abcde|fgh Stage-1 block |Stage-1 block tv|xyz| *+-+++---+ * *memslot->base_gfn << PAGE_SIZE: * +---+++-+ -* |abc|def Stage-2 PMD|Stage-2 PMD |tvxyz| +* |abc|def Stage-2 block |Stage-2 block |tvxyz| * +---+++-+ * -* If we create those stage-2 PMDs, we'll end up with this incorrect +* If we create those stage-2 blocks, we'll end up with this incorrect * mapping: * d -> f * e -> g * f -> h */ - if ((gpa_start & ~S2_PMD_MASK) != (uaddr_start & ~S2_PMD_MASK)) + if ((gpa_start & (map_size - 1)) != (uaddr_start & (map_size - 1))) return false; /* * Next, let's make sure we're not trying to map anything not covered -* by the memslot. This means we have to prohibit PMD size mappings -* for the beginning and end of a non-PMD aligned and non-PMD sized +* by the memslot. This means we have to prohibit block size mappings +* for the beginning and end of a non-block aligned and non-block sized * memory slot (illustrated by the head and tail parts of the * userspace view above containing pages 'abcde' and 'xyz', * respectively). @@ -1648,8 +1649,8 @@ static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, * userspace_addr or the base_gfn, as both are equally aligned (per * the check above) and equally sized. */ - return (hva & S2_PMD_MASK) >= uaddr_start && - (hva & S2_PMD_MASK) + S2_PMD_SIZE <= uaddr_end; + return (hva & ~(map_size - 1)) >= uaddr_start && + (hva & ~(map_size - 1)) + map_size <= uaddr_end; } static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, @@ -1678,12 +1679,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, return -EFAULT; } - if (!fault_supports_stage2_pmd_mappings(memslot, hva)) - force_pte = true; - - if (logging_active) - force_pte = true; - /* Let's check if we will get back a huge page backed by hugetlbfs */ down_read(>mm->mmap_sem); vma = find_vma_intersection(current->mm, hva, hva + 1); @@ -1694,6 +1689,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, } vma_pagesize = vma_kernel_pagesize(vma); + if (logging_active || + !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) { +