Hi James,
On 2019-12-12 15:34, James Morse wrote:
Hi Marc,
On 12/12/2019 11:33, Marc Zyngier wrote:
On 2019-12-11 16:56, Marc Zyngier wrote:
When we check for a poisoned page, we use the VMA to tell userspace
about the looming disaster. But we pass a pointer to this VMA
after having released the mmap_sem, which isn't a good idea.
Sounds like a bug! The vma-size might not match the poisoned pfn.
Instead, re-check that we have still have a VMA, and that this
VMA still points to a poisoned page. If the VMA isn't there,
userspace is playing with our nerves, so lety's give it a -EFAULT
(it deserves it). If the PFN isn't poisoned anymore, let's restart
from the top and handle the fault again.
virt/kvm/arm/mmu.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
... yeah ...
[...]
How about (untested):
-------------------------%<-------------------------
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 38b4c910b6c3..80212d4935bd 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1591,16 +1591,8 @@ static void
invalidate_icache_guest_page(kvm_pfn_t pfn, unsigned
long size)
__invalidate_icache_guest_page(pfn, size);
}
-static void kvm_send_hwpoison_signal(unsigned long address,
- struct vm_area_struct *vma)
+static void kvm_send_hwpoison_signal(unsigned long address, short
lsb)
{
- short lsb;
-
- if (is_vm_hugetlb_page(vma))
- lsb = huge_page_shift(hstate_vma(vma));
- else
- lsb = PAGE_SHIFT;
-
send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb,
current);
}
@@ -1673,6 +1665,7 @@ static int user_mem_abort(struct kvm_vcpu
*vcpu, phys_addr_t fault_ipa,
struct kvm *kvm = vcpu->kvm;
struct kvm_mmu_memory_cache *memcache =
&vcpu->arch.mmu_page_cache;
struct vm_area_struct *vma;
+ short stage1_vma_size;
kvm_pfn_t pfn;
pgprot_t mem_type = PAGE_S2;
bool logging_active = memslot_is_logging(memslot);
@@ -1703,6 +1696,12 @@ static int user_mem_abort(struct kvm_vcpu
*vcpu, phys_addr_t fault_ipa,
vma_pagesize = PAGE_SIZE;
}
+ /* For signals due to hwpoison, we need to use the stage1
size */
+ if (is_vm_hugetlb_page(vma))
+ stage1_vma_size = huge_page_shift(hstate_vma(vma));
+ else
+ stage1_vma_size = PAGE_SHIFT;
+
/*
* The stage2 has a minimum of 2 level table (For arm64 see
* kvm_arm_setup_stage2()). Hence, we are guaranteed that we
can
@@ -1735,7 +1734,7 @@ static int user_mem_abort(struct kvm_vcpu
*vcpu, phys_addr_t fault_ipa,
pfn = gfn_to_pfn_prot(kvm, gfn, write_fault, &writable);
if (pfn == KVM_PFN_ERR_HWPOISON) {
- kvm_send_hwpoison_signal(hva, vma);
+ kvm_send_hwpoison_signal(hva, stage1_vma_size);
return 0;
}
if (is_error_noslot_pfn(pfn))
-------------------------%<-------------------------
Its possible this could even be the original output of
vma_kernel_pagesize()... (Punit supplied the original
huge_page_shift(hstate_vma()) runes...)
I'd be happy with something along these lines. Any chance you could
a proper patch?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
