[PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region

Laurent Vivier posted 1 patch 3 years, 12 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220211170259.1388734-1-lvivier@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/vhost-vdpa.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region
Posted by Laurent Vivier 3 years, 12 months ago
If call virtio_queue_set_host_notifier_mr fails, should free
host-notifier memory-region.

This problem can trigger a coredump with some vDPA drivers (mlx5,
but not with the vdpasim), if we unplug the virtio-net card from
the guest after a stop/start.

The same fix has been done for vhost-user:
  1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")

Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
Cc: jasowang@redhat.com
Resolves: https://bugzilla.redhat.com/2027208
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/virtio/vhost-vdpa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 04ea43704f5d..11f696468dc1 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)
     g_free(name);
 
     if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
+        object_unparent(OBJECT(&n->mr));
         munmap(addr, page_size);
         goto err;
     }
-- 
2.34.1


Re: [PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region
Posted by Stefano Garzarella 3 years, 11 months ago
On Fri, Feb 11, 2022 at 06:02:59PM +0100, Laurent Vivier wrote:
>If call virtio_queue_set_host_notifier_mr fails, should free
>host-notifier memory-region.
>
>This problem can trigger a coredump with some vDPA drivers (mlx5,
>but not with the vdpasim), if we unplug the virtio-net card from
>the guest after a stop/start.
>
>The same fix has been done for vhost-user:
>  1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")
>
>Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
>Cc: jasowang@redhat.com
>Resolves: https://bugzilla.redhat.com/2027208
>Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>---
> hw/virtio/vhost-vdpa.c | 1 +
> 1 file changed, 1 insertion(+)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
>diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>index 04ea43704f5d..11f696468dc1 100644
>--- a/hw/virtio/vhost-vdpa.c
>+++ b/hw/virtio/vhost-vdpa.c
>@@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)
>     g_free(name);
>
>     if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
>+        object_unparent(OBJECT(&n->mr));
>         munmap(addr, page_size);
>         goto err;
>     }
>-- 
>2.34.1
>
>


Re: [PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region
Posted by Laurent Vivier 3 years, 11 months ago
Michael,

do you plan to merge this soon?

This is a bugfix for a QEMU coredump.

Thanks,
Laurent

On 11/02/2022 18:02, Laurent Vivier wrote:
> If call virtio_queue_set_host_notifier_mr fails, should free
> host-notifier memory-region.
> 
> This problem can trigger a coredump with some vDPA drivers (mlx5,
> but not with the vdpasim), if we unplug the virtio-net card from
> the guest after a stop/start.
> 
> The same fix has been done for vhost-user:
>    1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")
> 
> Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
> Cc: jasowang@redhat.com
> Resolves: https://bugzilla.redhat.com/2027208
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   hw/virtio/vhost-vdpa.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 04ea43704f5d..11f696468dc1 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)
>       g_free(name);
>   
>       if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
> +        object_unparent(OBJECT(&n->mr));
>           munmap(addr, page_size);
>           goto err;
>       }


Re: [PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region
Posted by Laurent Vivier 3 years, 11 months ago
Ping?

Thanks,
Laurent

On 22/02/2022 09:53, Laurent Vivier wrote:
> Michael,
> 
> do you plan to merge this soon?
> 
> This is a bugfix for a QEMU coredump.
> 
> Thanks,
> Laurent
> 
> On 11/02/2022 18:02, Laurent Vivier wrote:
>> If call virtio_queue_set_host_notifier_mr fails, should free
>> host-notifier memory-region.
>>
>> This problem can trigger a coredump with some vDPA drivers (mlx5,
>> but not with the vdpasim), if we unplug the virtio-net card from
>> the guest after a stop/start.
>>
>> The same fix has been done for vhost-user:
>>    1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")
>>
>> Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
>> Cc: jasowang@redhat.com
>> Resolves: https://bugzilla.redhat.com/2027208
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>   hw/virtio/vhost-vdpa.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index 04ea43704f5d..11f696468dc1 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int 
>> queue_index)
>>       g_free(name);
>>       if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
>> +        object_unparent(OBJECT(&n->mr));
>>           munmap(addr, page_size);
>>           goto err;
>>       }
> 


Re: [PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region
Posted by Michael S. Tsirkin 3 years, 11 months ago
yes - in the next pull, thanks!

On Mon, Feb 28, 2022 at 07:27:34PM +0100, Laurent Vivier wrote:
> Ping?
> 
> Thanks,
> Laurent
> 
> On 22/02/2022 09:53, Laurent Vivier wrote:
> > Michael,
> > 
> > do you plan to merge this soon?
> > 
> > This is a bugfix for a QEMU coredump.
> > 
> > Thanks,
> > Laurent
> > 
> > On 11/02/2022 18:02, Laurent Vivier wrote:
> > > If call virtio_queue_set_host_notifier_mr fails, should free
> > > host-notifier memory-region.
> > > 
> > > This problem can trigger a coredump with some vDPA drivers (mlx5,
> > > but not with the vdpasim), if we unplug the virtio-net card from
> > > the guest after a stop/start.
> > > 
> > > The same fix has been done for vhost-user:
> > >    1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")
> > > 
> > > Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
> > > Cc: jasowang@redhat.com
> > > Resolves: https://bugzilla.redhat.com/2027208
> > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > > ---
> > >   hw/virtio/vhost-vdpa.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > > index 04ea43704f5d..11f696468dc1 100644
> > > --- a/hw/virtio/vhost-vdpa.c
> > > +++ b/hw/virtio/vhost-vdpa.c
> > > @@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct
> > > vhost_dev *dev, int queue_index)
> > >       g_free(name);
> > >       if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
> > > +        object_unparent(OBJECT(&n->mr));
> > >           munmap(addr, page_size);
> > >           goto err;
> > >       }
> > 
Re: [PATCH] hw/virtio: vdpa: Fix leak of host-notifier memory-region
Posted by Jason Wang 3 years, 11 months ago
On Sat, Feb 12, 2022 at 1:03 AM Laurent Vivier <lvivier@redhat.com> wrote:
>
> If call virtio_queue_set_host_notifier_mr fails, should free
> host-notifier memory-region.
>
> This problem can trigger a coredump with some vDPA drivers (mlx5,
> but not with the vdpasim), if we unplug the virtio-net card from
> the guest after a stop/start.
>
> The same fix has been done for vhost-user:
>   1f89d3b91e3e ("hw/virtio: Fix leak of host-notifier memory-region")
>
> Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if possible")
> Cc: jasowang@redhat.com
> Resolves: https://bugzilla.redhat.com/2027208
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Cc: qemu-stable@nongnu.org
Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  hw/virtio/vhost-vdpa.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 04ea43704f5d..11f696468dc1 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -431,6 +431,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int queue_index)
>      g_free(name);
>
>      if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
> +        object_unparent(OBJECT(&n->mr));
>          munmap(addr, page_size);
>          goto err;
>      }
> --
> 2.34.1
>