Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread David Hildenbrand
> > Either that, or an early exit for bus == NULL in kvm_io_bus_destroy(). > (I think the second option is more straightforward.) I prefer to have the checks where kvm->buses[x] is actually accessed. So the chance of missing yet another check is easier to verify. Will send out v2 in a couple

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread David Hildenbrand
> > Either that, or an early exit for bus == NULL in kvm_io_bus_destroy(). > (I think the second option is more straightforward.) I prefer to have the checks where kvm->buses[x] is actually accessed. So the chance of missing yet another check is easier to verify. Will send out v2 in a couple

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread Cornelia Huck
On Thu, 23 Mar 2017 17:20:48 +0100 David Hildenbrand wrote: > > > As this may set kvm->buses[bus_idx] to NULL, don't you also need to > > guard for bus == NULL in kvm_io_bus_destroy()? (I looked at the code on > > kvm/queue.) > > very right, so something like this? > > diff

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread Cornelia Huck
On Thu, 23 Mar 2017 17:20:48 +0100 David Hildenbrand wrote: > > > As this may set kvm->buses[bus_idx] to NULL, don't you also need to > > guard for bus == NULL in kvm_io_bus_destroy()? (I looked at the code on > > kvm/queue.) > > very right, so something like this? > > diff --git

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread David Hildenbrand
> As this may set kvm->buses[bus_idx] to NULL, don't you also need to > guard for bus == NULL in kvm_io_bus_destroy()? (I looked at the code on > kvm/queue.) very right, so something like this? diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index e1be4b4..ef1aa7f 100644 ---

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread David Hildenbrand
> As this may set kvm->buses[bus_idx] to NULL, don't you also need to > guard for bus == NULL in kvm_io_bus_destroy()? (I looked at the code on > kvm/queue.) very right, so something like this? diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index e1be4b4..ef1aa7f 100644 ---

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread Cornelia Huck
On Thu, 23 Mar 2017 15:34:41 +0100 David Hildenbrand wrote: > No caller currently checks the return value of > kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on > freeing their device. A stale reference will remain in the io_bus, > getting at least used

Re: [PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread Cornelia Huck
On Thu, 23 Mar 2017 15:34:41 +0100 David Hildenbrand wrote: > No caller currently checks the return value of > kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on > freeing their device. A stale reference will remain in the io_bus, > getting at least used again, when the

[PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread David Hildenbrand
No caller currently checks the return value of kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on freeing their device. A stale reference will remain in the io_bus, getting at least used again, when the iobus gets teared down on kvm_destroy_vm() - leading to use after free

[PATCH v1] KVM: kvm_io_bus_unregister_dev() should never fail

2017-03-23 Thread David Hildenbrand
No caller currently checks the return value of kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on freeing their device. A stale reference will remain in the io_bus, getting at least used again, when the iobus gets teared down on kvm_destroy_vm() - leading to use after free