On 30/11/16 19:35, Aneesh Kumar K.V wrote: > Balbir Singh <bsingharora@xxxxxxxxx> writes: > >> Some KVM functions for book3s_hv are called in real mode. >> In real mode the top 4 bits of the address space are ignored, >> hence an address beginning with 0xc0000000+offset is the >> same as 0xd0000000+offset. The issue was observed when >> a kvm memslot resolution lead to random values when >> access from kvmppc_h_enter(). The issue is hit if the >> KVM host is running with a page size of 4K, since >> kvzalloc() looks at size < PAGE_SIZE. On systems with >> 64K the issue is not observed easily, it largely depends >> on the size of the structure being allocated. >> >> The proposed fix moves all KVM allocations for book3s_hv >> to kzalloc() until all structures used in real mode are >> audited. For safety allocations are moved to kmalloc >> space. The impact is a large allocation on systems with >> 4K page size. > > We did such access using *real_vmalloc_addr(void *x). So you are > suggesting here is we don't do that for all code path ? > Yep.. that is true > Do you have a stack dump for which you identified the issue ? > I found it with kvm_memslots, don't have a stack dump, but IIRC, I saw it with search_memslots <-- __gfn_to_memslot() >> >> Signed-off-by: Balbir Singh <bsingharora@xxxxxxxxx> >> --- >> Changelog v2: >> Fix build failures reported by the kbuild test robot >> http://www.spinics.net/lists/kvm/msg141727.html >> >> arch/powerpc/include/asm/kvm_host.h | 19 +++++++++++++++++++ >> include/linux/kvm_host.h | 11 +++++++++++ >> virt/kvm/kvm_main.c | 2 +- >> 3 files changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h >> index f15713a..53f5172 100644 >> --- a/arch/powerpc/include/asm/kvm_host.h >> +++ b/arch/powerpc/include/asm/kvm_host.h >> @@ -734,6 +734,25 @@ struct kvm_vcpu_arch { >> #define __KVM_HAVE_ARCH_WQP >> #define __KVM_HAVE_CREATE_DEVICE >> >> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE >> +#define __KVM_HAVE_ARCH_VZALLOC_OVERRIDE > > do we need that OVERRIDE ? We usually have HAVE_ARCH_KVM_VZALLOC > or just say #ifndef kvm_arch_vzalloc ? > I can move __KVM_HAVE_ARCH_VZALLOC_OVERRIDE to HAVE_ARCH_KVM_VZALLOC_OVERRIDE if it helps with clarity and convention Thanks for the review, Balbir -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
