linux-kernel - Re: [PATCH] KVM: X86: Fix async pf caused null-ptr-deref

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 29 Jun 2020 15:46:43 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Wanpeng Li <kernellwp@...il.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Vivek Goyal <vgoyal@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH] KVM: X86: Fix async pf caused null-ptr-deref

Wanpeng Li <kernellwp@...il.com> writes:

> From: Wanpeng Li <wanpengli@...cent.com>
>
> Syzbot reported that:
>
>   CPU: 1 PID: 6780 Comm: syz-executor153 Not tainted 5.7.0-syzkaller #0
>   Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>   RIP: 0010:__apic_accept_irq+0x46/0xb80
>   Call Trace:
>    kvm_arch_async_page_present+0x7de/0x9e0
>    kvm_check_async_pf_completion+0x18d/0x400
>    kvm_arch_vcpu_ioctl_run+0x18bf/0x69f0
>    kvm_vcpu_ioctl+0x46a/0xe20
>    ksys_ioctl+0x11a/0x180
>    __x64_sys_ioctl+0x6f/0xb0
>    do_syscall_64+0xf6/0x7d0
>    entry_SYSCALL_64_after_hwframe+0x49/0xb3
>  
> The testcase enables APF mechanism in MSR_KVM_ASYNC_PF_EN with ASYNC_PF_INT 
> enabled w/o setting MSR_KVM_ASYNC_PF_INT before, what's worse, interrupt 
> based APF 'page ready' event delivery depends on in kernel lapic, however, 
> we didn't bail out when lapic is not in kernel during guest setting 
> MSR_KVM_ASYNC_PF_EN which causes the null-ptr-deref in host later. 
> This patch fixes it.
>
> Reported-by: syzbot+1bf777dfdde86d64b89b@...kaller.appspotmail.com
> Fixes: 2635b5c4a0 (KVM: x86: interrupt based APF 'page ready' event delivery)

Thanks!

> Signed-off-by: Wanpeng Li <wanpengli@...cent.com>
> ---
>  arch/x86/kvm/x86.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 00c88c2..1c0b4f5 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2693,6 +2693,9 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
>  	if (data & 0x30)
>  		return 1;
>  
> +	if (!lapic_in_kernel(vcpu))
> +		return 1;
> +

I'm not sure how much we care about !lapic_in_kernel() case but this
change should be accompanied with userspace changes to not expose
KVM_FEATURE_ASYNC_PF_INT or how would the guest know that writing a
legitimate value will result in #GP?

Alternatively, we may just return '0' here: guest will be able to check
what's in the MSR to see if the feature was enabled. Normally, guests
shouldn't care about this but maybe there are cases when they do?

>  	vcpu->arch.apf.msr_en_val = data;
>  
>  	if (!kvm_pv_async_pf_enabled(vcpu)) {

-- 
Vitaly