Re: [PATCH v4] KVM: LAPIC: Fix reentrancy issues with preempt notifiers

[Takashi Iwai]

On Tue, 25 Jul 2017 09:43:15 +0200,
Wanpeng Li wrote:
> 
> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
> 
> Preempt can occur in the preemption timer expiration handler:
> 
>           CPU0                    CPU1
> 
>   preemption timer vmexit
>   handle_preemption_timer(vCPU0)
>     kvm_lapic_expired_hv_timer
>       hv_timer_is_use == true
>   sched_out
>                            sched_in
>                            kvm_arch_vcpu_load
>                              kvm_lapic_restart_hv_timer
>                                restart_apic_timer
>                                  start_hv_timer
>                                    already-expired timer or sw timer triggerd in the window
>                                  start_sw_timer
>                                    cancel_hv_timer
>                            /* back in kvm_lapic_expired_hv_timer */
>                            cancel_hv_timer
>                              WARN_ON(!apic->lapic_timer.hv_timer_in_use);  ==> Oops
> 
> This can be reproduced if CONFIG_PREEMPT is enabled.
> 
> ------------[ cut here ]------------
>  WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1563 kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm]
>  CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G           OE   4.13.0-rc2+ #16
>  RIP: 0010:kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm]
> Call Trace:
>   handle_preemption_timer+0xe/0x20 [kvm_intel]
>   vmx_handle_exit+0xb8/0xd70 [kvm_intel]
>   kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm]
>   ? kvm_arch_vcpu_load+0x47/0x230 [kvm]
>   ? kvm_arch_vcpu_load+0x62/0x230 [kvm]
>   kvm_vcpu_ioctl+0x340/0x700 [kvm]
>   ? kvm_vcpu_ioctl+0x340/0x700 [kvm]
>   ? __fget+0xfc/0x210
>   do_vfs_ioctl+0xa4/0x6a0
>   ? __fget+0x11d/0x210
>   SyS_ioctl+0x79/0x90
>   do_syscall_64+0x81/0x220
>   entry_SYSCALL64_slow_path+0x25/0x25
>  ------------[ cut here ]------------
>  WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1498 cancel_hv_timer.isra.40+0x4f/0x60 [kvm]
>  CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G        W  OE   4.13.0-rc2+ #16
>  RIP: 0010:cancel_hv_timer.isra.40+0x4f/0x60 [kvm]
> Call Trace:
>   kvm_lapic_expired_hv_timer+0x3e/0xb0 [kvm]
>   handle_preemption_timer+0xe/0x20 [kvm_intel]
>   vmx_handle_exit+0xb8/0xd70 [kvm_intel]
>   kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm]
>   ? kvm_arch_vcpu_load+0x47/0x230 [kvm]
>   ? kvm_arch_vcpu_load+0x62/0x230 [kvm]
>   kvm_vcpu_ioctl+0x340/0x700 [kvm]
>   ? kvm_vcpu_ioctl+0x340/0x700 [kvm]
>   ? __fget+0xfc/0x210
>   do_vfs_ioctl+0xa4/0x6a0
>   ? __fget+0x11d/0x210
>   SyS_ioctl+0x79/0x90
>   do_syscall_64+0x81/0x220
>   entry_SYSCALL64_slow_path+0x25/0x25
> 
> This patch fixes it by making the caller of start_hv_timer and start_sw_timer 
> be in preemption-disabled regions, which trivially avoid any reentrancy 
> issue with preempt notifier.
> 
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>

I've hit this warning today, and your patch seems fixing it.
  Tested-by: Takashi Iwai <tiwai@xxxxxxx>

Thanks!


Takashi