Messages in this thread | | | Subject | Re: [PATCH] KVM: check userspace_addr for all memslots | From | Maxim Levitsky <> | Date | Thu, 11 Jun 2020 17:44:02 +0300 |
| |
On Mon, 2020-06-01 at 04:21 -0400, Paolo Bonzini wrote: > The userspace_addr alignment and range checks are not performed for private > memory slots that are prepared by KVM itself. This is unnecessary and makes > it questionable to use __*_user functions to access memory later on. We also > rely on the userspace address being aligned since we have an entire family > of functions to map gfn to pfn. > > Fortunately skipping the check is completely unnecessary. Only x86 uses > private memslots and their userspace_addr is obtained from vm_mmap, > therefore it must be below PAGE_OFFSET. In fact, any attempt to pass > an address above PAGE_OFFSET would have failed because such an address > would return true for kvm_is_error_hva. > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
I bisected this patch to break a VM on my AMD system (3970X)
The reason it happens, is because I have avic enabled (which uses a private KVM memslot), but it is permanently disabled for that VM, since I enabled nesting for that VM (+svm) and that triggers the code in __x86_set_memory_region to set userspace_addr of the disabled memslot to non canonical address (0xdeadull << 48) which is later rejected in __kvm_set_memory_region after that patch, and that makes it silently not disable the memslot, which hangs the guest.
The call is from avic_update_access_page, which is called from svm_pre_update_apicv_exec_ctrl which discards the return value.
I think that the fix for this would be to either make access_ok always return true for size==0, or __kvm_set_memory_region should treat size==0 specially and skip that check for it.
Best regards, Maxim Levitsky
| |