| Message ID | 20170710185328.32166-1-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10.07.2017 20:53, Radim Krčmář wrote: > kvm_vm_release() did not have slots_lock when calling > kvm_io_bus_unregister_dev() and this went unnoticed until 4a12f9517728 > ("KVM: mark kvm->busses as rcu protected") added dynamic checks. > Luckily, there should be no race at that point: > > ============================= > WARNING: suspicious RCU usage > 4.12.0.kvm+ #0 Not tainted > ----------------------------- > ./include/linux/kvm_host.h:479 suspicious rcu_dereference_check() usage! > > lockdep_rcu_suspicious+0xc5/0x100 > kvm_io_bus_unregister_dev+0x173/0x190 [kvm] > kvm_free_pit+0x28/0x80 [kvm] > kvm_arch_sync_events+0x2d/0x30 [kvm] > kvm_put_kvm+0xa7/0x2a0 [kvm] > kvm_vm_release+0x21/0x30 [kvm] > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > --- > arch/x86/kvm/i8254.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > index a78b445ce411..af192895b1fc 100644 > --- a/arch/x86/kvm/i8254.c > +++ b/arch/x86/kvm/i8254.c > @@ -724,8 +724,10 @@ void kvm_free_pit(struct kvm *kvm) > struct kvm_pit *pit = kvm->arch.vpit; > > if (pit) { > + mutex_lock(&kvm->slots_lock); > kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev); > kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->speaker_dev); > + mutex_unlock(&kvm->slots_lock); > kvm_pit_set_reinject(pit, false); > hrtimer_cancel(&pit->pit_state.timer); > kthread_destroy_worker(pit->worker); > I tried to verify that we don't have any hierarchical locking inversion in the way here (kvm->lock, kvm->slots_lock, kvm->irq_lock). Hopefully I did this carefully enough :) Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c index a78b445ce411..af192895b1fc 100644 --- a/arch/x86/kvm/i8254.c +++ b/arch/x86/kvm/i8254.c @@ -724,8 +724,10 @@ void kvm_free_pit(struct kvm *kvm) struct kvm_pit *pit = kvm->arch.vpit; if (pit) { + mutex_lock(&kvm->slots_lock); kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev); kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->speaker_dev); + mutex_unlock(&kvm->slots_lock); kvm_pit_set_reinject(pit, false); hrtimer_cancel(&pit->pit_state.timer); kthread_destroy_worker(pit->worker);
kvm_vm_release() did not have slots_lock when calling kvm_io_bus_unregister_dev() and this went unnoticed until 4a12f9517728 ("KVM: mark kvm->busses as rcu protected") added dynamic checks. Luckily, there should be no race at that point: ============================= WARNING: suspicious RCU usage 4.12.0.kvm+ #0 Not tainted ----------------------------- ./include/linux/kvm_host.h:479 suspicious rcu_dereference_check() usage! lockdep_rcu_suspicious+0xc5/0x100 kvm_io_bus_unregister_dev+0x173/0x190 [kvm] kvm_free_pit+0x28/0x80 [kvm] kvm_arch_sync_events+0x2d/0x30 [kvm] kvm_put_kvm+0xa7/0x2a0 [kvm] kvm_vm_release+0x21/0x30 [kvm] Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- arch/x86/kvm/i8254.c | 2 ++ 1 file changed, 2 insertions(+)