On 07/09/2017 22:27, Alex Williamson wrote:
> vhost registers a MemoryListener where it adds and removes references
> to MemoryRegions as the MemoryRegionSections pass through. The
> region_add callback is invoked for each existing section when the
> MemoryListener is registered, but unregistering the MemoryListener
> performs no reciprocal region_del callback. It's therefore the
> owner of the MemoryListener's responsibility to cleanup any persistent
> changes, such as these memory references, after unregistering.
>
> The consequence of this bug is that if we have both a vhost device
> and a vfio device, the vhost device will reference any mmap'd MMIO of
> the vfio device via this MemoryListener. If the vhost device is then
> removed, those references remain outstanding. If we then attempt to
> remove the vfio device, it never gets finalized and the only way to
> release the kernel file descriptors is to terminate the QEMU process.
>
> Fixes: dfde4e6e1a86 ("memory: add ref/unref calls")
> Cc: Michael S. Tsirkin <m...@redhat.com>
> Cc: Paolo Bonzini <pbonz...@redhat.com>
> Cc: qemu-sta...@nongnu.org # v1.6.0+
> Signed-off-by: Alex Williamson <alex.william...@redhat.com>
This fix is correct. It's surprising that it was only found now!
Thanks,
Paolo
> ---
> hw/virtio/vhost.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6eddb099b02f..b737ca915b06 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1356,6 +1356,10 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
> if (hdev->mem) {
> /* those are only safe after successful init */
> memory_listener_unregister(&hdev->memory_listener);
> + for (i = 0; i < hdev->n_mem_sections; ++i) {
> + MemoryRegionSection *section = &hdev->mem_sections[i];
> + memory_region_unref(section->mr);
> + }
> QLIST_REMOVE(hdev, entry);
> }
> if (hdev->migration_blocker) {
>
