Re: [PATCH] KVM: x86: hyper-v: Fix Hyper-V context null-ptr-deref

[Vitaly Kuznetsov]

Wanpeng Li <kernellwp@xxxxxxxxx> writes:

> From: Wanpeng Li <wanpengli@xxxxxxxxxxx>
>
> Reported by syzkaller:
>
>     KASAN: null-ptr-deref in range [0x0000000000000140-0x0000000000000147]
>     CPU: 1 PID: 8370 Comm: syz-executor859 Not tainted 5.11.0-syzkaller #0
>     RIP: 0010:synic_get arch/x86/kvm/hyperv.c:165 [inline]
>     RIP: 0010:kvm_hv_set_sint_gsi arch/x86/kvm/hyperv.c:475 [inline]
>     RIP: 0010:kvm_hv_irq_routing_update+0x230/0x460 arch/x86/kvm/hyperv.c:498
>     Call Trace:
>      kvm_set_irq_routing+0x69b/0x940 arch/x86/kvm/../../../virt/kvm/irqchip.c:223
>      kvm_vm_ioctl+0x12d0/0x2800 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3959
>      vfs_ioctl fs/ioctl.c:48 [inline]
>      __do_sys_ioctl fs/ioctl.c:753 [inline]
>      __se_sys_ioctl fs/ioctl.c:739 [inline]
>      __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
>      do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
>      entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Hyper-V context is lazily allocated until Hyper-V specific MSRs are accessed 
> or SynIC is enabled. However, the syzkaller testcase sets irq routing table 
> directly w/o enabling SynIC. This results in null-ptr-deref when accessing 
> SynIC Hyper-V context. This patch fixes it.
>      
> syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=163342ccd00000
>
> Reported-by: syzbot+6987f3b2dbd9eda95f12@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 8f014550dfb1 ("KVM: x86: hyper-v: Make Hyper-V emulation enablement conditional")
> Signed-off-by: Wanpeng Li <wanpengli@xxxxxxxxxxx>
> ---
>  arch/x86/kvm/hyperv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 7d2dae9..58fa8c0 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -159,7 +159,7 @@ static struct kvm_vcpu_hv_synic *synic_get(struct kvm *kvm, u32 vpidx)
>  	struct kvm_vcpu_hv_synic *synic;
>  
>  	vcpu = get_vcpu_by_vpidx(kvm, vpidx);
> -	if (!vcpu)
> +	if (!vcpu || !to_hv_vcpu(vcpu))
>  		return NULL;
>  	synic = to_hv_synic(vcpu);
>  	return (synic->active) ? synic : NULL;

Oops, I've missed this path completely. Thanks for the fix!

Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>

-- 
Vitaly