[PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

When a virtio-gpu resource is destroyed, any associated udmabuf must be
properly torn down. Currently, the code may leave dangling references
to dmabuf file descriptors in the scanout primary buffers.

This patch updates virtio_gpu_fini_udmabuf to:
1. Iterate through all active scanouts.
2. Identify dmabufs that match the resource's file descriptor.
3. Close the dmabuf and invalidate the resource's FD reference to
   prevent use-after-free or double-close scenarios.
4. Finally, trigger the underlying udmabuf destruction.

This ensures that the display backend does not attempt to access
memory or FDs that have been released by the guest or the host.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/hw/virtio/virtio-gpu.h  |  3 ++-
 hw/display/virtio-gpu-udmabuf.c | 25 ++++++++++++++++++-------
 hw/display/virtio-gpu.c         |  2 +-
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 58e0f91fda..65312f869d 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
 /* virtio-gpu-udmabuf.c */
 bool virtio_gpu_have_udmabuf(void);
 void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
-void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
+void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
+                             struct virtio_gpu_simple_resource *res);
 int virtio_gpu_update_dmabuf(VirtIOGPU *g,
                              uint32_t scanout_id,
                              struct virtio_gpu_simple_resource *res,
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index d804f321aa..bd5b44f5fb 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
     res->blob = pdata;
 }
 
-void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
-{
-    if (res->remapped) {
-        virtio_gpu_destroy_udmabuf(res);
-    }
-}
-
 static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
 {
     struct virtio_gpu_scanout *scanout;
@@ -169,6 +162,24 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
     g_free(dmabuf);
 }
 
+void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
+{
+    int max_outputs = g->parent_obj.conf.max_outputs;
+    int i;
+
+    for (i = 0; i < max_outputs; i++) {
+        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
+
+        if (dmabuf && (res->dmabuf_fd != -1) &&
+            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd) {
+            qemu_dmabuf_close(dmabuf->buf);
+            res->dmabuf_fd = -1;
+        }
+    }
+
+    virtio_gpu_destroy_udmabuf(res);
+}
+
 static VGPUDMABuf
 *virtio_gpu_create_dmabuf(VirtIOGPU *g,
                           uint32_t scanout_id,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 643e91ca2a..b2af861f0d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
     res->addrs = NULL;
 
     if (res->blob) {
-        virtio_gpu_fini_udmabuf(res);
+        virtio_gpu_fini_udmabuf(g, res);
     }
 }
 
-- 
2.43.0

Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[Marc-André Lureau]

Hi

On Tue, Mar 3, 2026 at 2:06 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> When a virtio-gpu resource is destroyed, any associated udmabuf must be
> properly torn down. Currently, the code may leave dangling references
> to dmabuf file descriptors in the scanout primary buffers.
>
> This patch updates virtio_gpu_fini_udmabuf to:
> 1. Iterate through all active scanouts.
> 2. Identify dmabufs that match the resource's file descriptor.
> 3. Close the dmabuf and invalidate the resource's FD reference to
>    prevent use-after-free or double-close scenarios.
> 4. Finally, trigger the underlying udmabuf destruction.
>
> This ensures that the display backend does not attempt to access
> memory or FDs that have been released by the guest or the host.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>

Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  include/hw/virtio/virtio-gpu.h  |  3 ++-
>  hw/display/virtio-gpu-udmabuf.c | 25 ++++++++++++++++++-------
>  hw/display/virtio-gpu.c         |  2 +-
>  3 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 58e0f91fda..65312f869d 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
>  /* virtio-gpu-udmabuf.c */
>  bool virtio_gpu_have_udmabuf(void);
>  void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
> -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
> +void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
> +                             struct virtio_gpu_simple_resource *res);
>  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>                               uint32_t scanout_id,
>                               struct virtio_gpu_simple_resource *res,
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index d804f321aa..bd5b44f5fb 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>      res->blob = pdata;
>  }
>
> -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
> -{
> -    if (res->remapped) {
> -        virtio_gpu_destroy_udmabuf(res);
> -    }
> -}
> -
>  static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>  {
>      struct virtio_gpu_scanout *scanout;
> @@ -169,6 +162,24 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>      g_free(dmabuf);
>  }
>
> +void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
> +{
> +    int max_outputs = g->parent_obj.conf.max_outputs;
> +    int i;
> +
> +    for (i = 0; i < max_outputs; i++) {
> +        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
> +
> +        if (dmabuf && (res->dmabuf_fd != -1) &&

Maybe add qemu_dmabuf_get_numplanes() > 0 ?

> +            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd) {
> +            qemu_dmabuf_close(dmabuf->buf);
> +            res->dmabuf_fd = -1;

I am not really happy about that we close the underlying fd here
before the next destroy, but I don't have an immediate solution

> +        }
> +    }
> +
> +    virtio_gpu_destroy_udmabuf(res);
> +}
> +
>  static VGPUDMABuf
>  *virtio_gpu_create_dmabuf(VirtIOGPU *g,
>                            uint32_t scanout_id,
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 643e91ca2a..b2af861f0d 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
>      res->addrs = NULL;
>
>      if (res->blob) {
> -        virtio_gpu_fini_udmabuf(res);
> +        virtio_gpu_fini_udmabuf(g, res);
>      }
>  }
>
> --
> 2.43.0
>
>


-- 
Marc-André Lureau

Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Tue, Mar 3, 2026 at 2:06 AM <dongwon.kim@intel.com> wrote:
>>
>> From: Dongwon Kim <dongwon.kim@intel.com>
>>
>> When a virtio-gpu resource is destroyed, any associated udmabuf must be
>> properly torn down. Currently, the code may leave dangling references
>> to dmabuf file descriptors in the scanout primary buffers.

Queued to virtio-gpu/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

RE: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[Kim, Dongwon]

Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@gmail.com>
> Sent: Tuesday, March 3, 2026 8:28 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource
> destruction
> 
> Hi
> 
> On Tue, Mar 3, 2026 at 2:06 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > When a virtio-gpu resource is destroyed, any associated udmabuf must
> > be properly torn down. Currently, the code may leave dangling
> > references to dmabuf file descriptors in the scanout primary buffers.
> >
> > This patch updates virtio_gpu_fini_udmabuf to:
> > 1. Iterate through all active scanouts.
> > 2. Identify dmabufs that match the resource's file descriptor.
> > 3. Close the dmabuf and invalidate the resource's FD reference to
> >    prevent use-after-free or double-close scenarios.
> > 4. Finally, trigger the underlying udmabuf destruction.
> >
> > This ensures that the display backend does not attempt to access
> > memory or FDs that have been released by the guest or the host.
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> 
> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> > ---
> >  include/hw/virtio/virtio-gpu.h  |  3 ++-
> > hw/display/virtio-gpu-udmabuf.c | 25 ++++++++++++++++++-------
> >  hw/display/virtio-gpu.c         |  2 +-
> >  3 files changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/hw/virtio/virtio-gpu.h
> > b/include/hw/virtio/virtio-gpu.h index 58e0f91fda..65312f869d 100644
> > --- a/include/hw/virtio/virtio-gpu.h
> > +++ b/include/hw/virtio/virtio-gpu.h
> > @@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct
> > virtio_gpu_framebuffer *fb,
> >  /* virtio-gpu-udmabuf.c */
> >  bool virtio_gpu_have_udmabuf(void);
> >  void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
> > -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
> > +void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
> > +                             struct virtio_gpu_simple_resource *res);
> >  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
> >                               uint32_t scanout_id,
> >                               struct virtio_gpu_simple_resource *res,
> > diff --git a/hw/display/virtio-gpu-udmabuf.c
> > b/hw/display/virtio-gpu-udmabuf.c index d804f321aa..bd5b44f5fb 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >      res->blob = pdata;
> >  }
> >
> > -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
> > -{
> > -    if (res->remapped) {
> > -        virtio_gpu_destroy_udmabuf(res);
> > -    }
> > -}
> > -
> >  static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
> > {
> >      struct virtio_gpu_scanout *scanout; @@ -169,6 +162,24 @@ static
> > void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
> >      g_free(dmabuf);
> >  }
> >
> > +void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct
> > +virtio_gpu_simple_resource *res) {
> > +    int max_outputs = g->parent_obj.conf.max_outputs;
> > +    int i;
> > +
> > +    for (i = 0; i < max_outputs; i++) {
> > +        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
> > +
> > +        if (dmabuf && (res->dmabuf_fd != -1) &&
> 
> Maybe add qemu_dmabuf_get_numplanes() > 0 ?

Do you want me to add this condition and resubmit v2 of this patch? I saw
this patch has already been in the queue.

> 
> > +            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd) {
> > +            qemu_dmabuf_close(dmabuf->buf);
> > +            res->dmabuf_fd = -1;
> 
> I am not really happy about that we close the underlying fd here before the
> next destroy, but I don't have an immediate solution

Yeah, I just thought this would be the best for now.

> 
> > +        }
> > +    }
> > +
> > +    virtio_gpu_destroy_udmabuf(res);
> > +}
> > +
> >  static VGPUDMABuf
> >  *virtio_gpu_create_dmabuf(VirtIOGPU *g,
> >                            uint32_t scanout_id, diff --git
> > a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index
> > 643e91ca2a..b2af861f0d 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
> >      res->addrs = NULL;
> >
> >      if (res->blob) {
> > -        virtio_gpu_fini_udmabuf(res);
> > +        virtio_gpu_fini_udmabuf(g, res);
> >      }
> >  }
> >
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Marc-André Lureau

Thanks,

Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[Alex Bennée]

"Kim, Dongwon" <dongwon.kim@intel.com> writes:

> Hi Marc-André,
>
>> -----Original Message-----
>> From: Marc-André Lureau <marcandre.lureau@gmail.com>
>> Sent: Tuesday, March 3, 2026 8:28 AM
>> To: Kim, Dongwon <dongwon.kim@intel.com>
>> Cc: qemu-devel@nongnu.org
>> Subject: Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource
>> destruction
>> 
>> Hi
>> 
>> On Tue, Mar 3, 2026 at 2:06 AM <dongwon.kim@intel.com> wrote:
>> >
>> > From: Dongwon Kim <dongwon.kim@intel.com>
>> >
>> > When a virtio-gpu resource is destroyed, any associated udmabuf must
>> > be properly torn down. Currently, the code may leave dangling
>> > references to dmabuf file descriptors in the scanout primary buffers.
>> >
>> > This patch updates virtio_gpu_fini_udmabuf to:
>> > 1. Iterate through all active scanouts.
>> > 2. Identify dmabufs that match the resource's file descriptor.
>> > 3. Close the dmabuf and invalidate the resource's FD reference to
>> >    prevent use-after-free or double-close scenarios.
>> > 4. Finally, trigger the underlying udmabuf destruction.
>> >
>> > This ensures that the display backend does not attempt to access
>> > memory or FDs that have been released by the guest or the host.
>> >
>> > Cc: Gerd Hoffmann <kraxel@redhat.com>
>> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
>> 
>> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> 
>> > ---
>> >  include/hw/virtio/virtio-gpu.h  |  3 ++-
>> > hw/display/virtio-gpu-udmabuf.c | 25 ++++++++++++++++++-------
>> >  hw/display/virtio-gpu.c         |  2 +-
>> >  3 files changed, 21 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/include/hw/virtio/virtio-gpu.h
>> > b/include/hw/virtio/virtio-gpu.h index 58e0f91fda..65312f869d 100644
>> > --- a/include/hw/virtio/virtio-gpu.h
>> > +++ b/include/hw/virtio/virtio-gpu.h
>> > @@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct
>> > virtio_gpu_framebuffer *fb,
>> >  /* virtio-gpu-udmabuf.c */
>> >  bool virtio_gpu_have_udmabuf(void);
>> >  void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
>> > -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
>> > +void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
>> > +                             struct virtio_gpu_simple_resource *res);
>> >  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>> >                               uint32_t scanout_id,
>> >                               struct virtio_gpu_simple_resource *res,
>> > diff --git a/hw/display/virtio-gpu-udmabuf.c
>> > b/hw/display/virtio-gpu-udmabuf.c index d804f321aa..bd5b44f5fb 100644
>> > --- a/hw/display/virtio-gpu-udmabuf.c
>> > +++ b/hw/display/virtio-gpu-udmabuf.c
>> > @@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>> >      res->blob = pdata;
>> >  }
>> >
>> > -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
>> > -{
>> > -    if (res->remapped) {
>> > -        virtio_gpu_destroy_udmabuf(res);
>> > -    }
>> > -}
>> > -
>> >  static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>> > {
>> >      struct virtio_gpu_scanout *scanout; @@ -169,6 +162,24 @@ static
>> > void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>> >      g_free(dmabuf);
>> >  }
>> >
>> > +void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct
>> > +virtio_gpu_simple_resource *res) {
>> > +    int max_outputs = g->parent_obj.conf.max_outputs;
>> > +    int i;
>> > +
>> > +    for (i = 0; i < max_outputs; i++) {
>> > +        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
>> > +
>> > +        if (dmabuf && (res->dmabuf_fd != -1) &&
>> 
>> Maybe add qemu_dmabuf_get_numplanes() > 0 ?
>
> Do you want me to add this condition and resubmit v2 of this patch? I saw
> this patch has already been in the queue.

If you send v2 I can swap it out.
>
>> 
>> > +            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd) {
>> > +            qemu_dmabuf_close(dmabuf->buf);
>> > +            res->dmabuf_fd = -1;
>> 
>> I am not really happy about that we close the underlying fd here before the
>> next destroy, but I don't have an immediate solution
>
> Yeah, I just thought this would be the best for now.
>
>> 
>> > +        }
>> > +    }
>> > +
>> > +    virtio_gpu_destroy_udmabuf(res);
>> > +}
>> > +
>> >  static VGPUDMABuf
>> >  *virtio_gpu_create_dmabuf(VirtIOGPU *g,
>> >                            uint32_t scanout_id, diff --git
>> > a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index
>> > 643e91ca2a..b2af861f0d 100644
>> > --- a/hw/display/virtio-gpu.c
>> > +++ b/hw/display/virtio-gpu.c
>> > @@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
>> >      res->addrs = NULL;
>> >
>> >      if (res->blob) {
>> > -        virtio_gpu_fini_udmabuf(res);
>> > +        virtio_gpu_fini_udmabuf(g, res);
>> >      }
>> >  }
>> >
>> > --
>> > 2.43.0
>> >
>> >
>> 
>> 
>> --
>> Marc-André Lureau
>
> Thanks,

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> "Kim, Dongwon" <dongwon.kim@intel.com> writes:
>
>> Hi Marc-André,
>>
>>> -----Original Message-----
>>> From: Marc-André Lureau <marcandre.lureau@gmail.com>
>>> Sent: Tuesday, March 3, 2026 8:28 AM
>>> To: Kim, Dongwon <dongwon.kim@intel.com>
>>> Cc: qemu-devel@nongnu.org
>>> Subject: Re: [PATCH] virtio-gpu: Fix scanout dmabuf cleanup during resource
>>> destruction
>>> 
>>> Hi
>>> 
>>> On Tue, Mar 3, 2026 at 2:06 AM <dongwon.kim@intel.com> wrote:
>>> >
>>> > From: Dongwon Kim <dongwon.kim@intel.com>
>>> >
>>> > When a virtio-gpu resource is destroyed, any associated udmabuf must
>>> > be properly torn down. Currently, the code may leave dangling
>>> > references to dmabuf file descriptors in the scanout primary buffers.
>>> >
>>> > This patch updates virtio_gpu_fini_udmabuf to:
>>> > 1. Iterate through all active scanouts.
>>> > 2. Identify dmabufs that match the resource's file descriptor.
>>> > 3. Close the dmabuf and invalidate the resource's FD reference to
>>> >    prevent use-after-free or double-close scenarios.
>>> > 4. Finally, trigger the underlying udmabuf destruction.
>>> >
>>> > This ensures that the display backend does not attempt to access
>>> > memory or FDs that have been released by the guest or the host.
>>> >
>>> > Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
>>> 
>>> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> 
>>> > ---
>>> >  include/hw/virtio/virtio-gpu.h  |  3 ++-
>>> > hw/display/virtio-gpu-udmabuf.c | 25 ++++++++++++++++++-------
>>> >  hw/display/virtio-gpu.c         |  2 +-
>>> >  3 files changed, 21 insertions(+), 9 deletions(-)
>>> >
>>> > diff --git a/include/hw/virtio/virtio-gpu.h
>>> > b/include/hw/virtio/virtio-gpu.h index 58e0f91fda..65312f869d 100644
>>> > --- a/include/hw/virtio/virtio-gpu.h
>>> > +++ b/include/hw/virtio/virtio-gpu.h
>>> > @@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct
>>> > virtio_gpu_framebuffer *fb,
>>> >  /* virtio-gpu-udmabuf.c */
>>> >  bool virtio_gpu_have_udmabuf(void);
>>> >  void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
>>> > -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
>>> > +void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
>>> > +                             struct virtio_gpu_simple_resource *res);
>>> >  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>>> >                               uint32_t scanout_id,
>>> >                               struct virtio_gpu_simple_resource *res,
>>> > diff --git a/hw/display/virtio-gpu-udmabuf.c
>>> > b/hw/display/virtio-gpu-udmabuf.c index d804f321aa..bd5b44f5fb 100644
>>> > --- a/hw/display/virtio-gpu-udmabuf.c
>>> > +++ b/hw/display/virtio-gpu-udmabuf.c
>>> > @@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct
>>> virtio_gpu_simple_resource *res)
>>> >      res->blob = pdata;
>>> >  }
>>> >
>>> > -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
>>> > -{
>>> > -    if (res->remapped) {
>>> > -        virtio_gpu_destroy_udmabuf(res);
>>> > -    }
>>> > -}
>>> > -
>>> >  static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>>> > {
>>> >      struct virtio_gpu_scanout *scanout; @@ -169,6 +162,24 @@ static
>>> > void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>>> >      g_free(dmabuf);
>>> >  }
>>> >
>>> > +void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct
>>> > +virtio_gpu_simple_resource *res) {
>>> > +    int max_outputs = g->parent_obj.conf.max_outputs;
>>> > +    int i;
>>> > +
>>> > +    for (i = 0; i < max_outputs; i++) {
>>> > +        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
>>> > +
>>> > +        if (dmabuf && (res->dmabuf_fd != -1) &&
>>> 
>>> Maybe add qemu_dmabuf_get_numplanes() > 0 ?
>>
>> Do you want me to add this condition and resubmit v2 of this patch? I saw
>> this patch has already been in the queue.
>
> If you send v2 I can swap it out.

I also noted you need to fix the stub:

  void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
  {
      /* nothing (stub) */
  }


>>
>>> 
>>> > +            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd) {
>>> > +            qemu_dmabuf_close(dmabuf->buf);
>>> > +            res->dmabuf_fd = -1;
>>> 
>>> I am not really happy about that we close the underlying fd here before the
>>> next destroy, but I don't have an immediate solution
>>
>> Yeah, I just thought this would be the best for now.
>>
>>> 
>>> > +        }
>>> > +    }
>>> > +
>>> > +    virtio_gpu_destroy_udmabuf(res);
>>> > +}
>>> > +
>>> >  static VGPUDMABuf
>>> >  *virtio_gpu_create_dmabuf(VirtIOGPU *g,
>>> >                            uint32_t scanout_id, diff --git
>>> > a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index
>>> > 643e91ca2a..b2af861f0d 100644
>>> > --- a/hw/display/virtio-gpu.c
>>> > +++ b/hw/display/virtio-gpu.c
>>> > @@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
>>> >      res->addrs = NULL;
>>> >
>>> >      if (res->blob) {
>>> > -        virtio_gpu_fini_udmabuf(res);
>>> > +        virtio_gpu_fini_udmabuf(g, res);
>>> >      }
>>> >  }
>>> >
>>> > --
>>> > 2.43.0
>>> >
>>> >
>>> 
>>> 
>>> --
>>> Marc-André Lureau
>>
>> Thanks,

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v2] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[dongwon.kim]

From: Dongwon Kim <dongwon.kim@intel.com>

When a virtio-gpu resource is destroyed, any associated udmabuf must be
properly torn down. Currently, the code may leave dangling references
to dmabuf file descriptors in the scanout primary buffers.

This patch updates virtio_gpu_fini_udmabuf to:
1. Iterate through all active scanouts.
2. Identify dmabufs that match the resource's file descriptor.
3. Close the dmabuf and invalidate the resource's FD reference to
   prevent use-after-free or double-close scenarios.
4. Finally, trigger the underlying udmabuf destruction.

This ensures that the display backend does not attempt to access
memory or FDs that have been released by the guest or the host.

v2: - Corrected virtio_gpu_fini_udmabuf in stub
      (Alex Bennée)

    - Make sure that qemu dmabuf has at least one plane before
      Comparing fds
      (Marc-André Lureau)

Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 include/hw/virtio/virtio-gpu.h        |  3 ++-
 hw/display/virtio-gpu-udmabuf-stubs.c |  2 +-
 hw/display/virtio-gpu-udmabuf.c       | 27 ++++++++++++++++++++-------
 hw/display/virtio-gpu.c               |  2 +-
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 58e0f91fda..65312f869d 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
 /* virtio-gpu-udmabuf.c */
 bool virtio_gpu_have_udmabuf(void);
 void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
-void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
+void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
+                             struct virtio_gpu_simple_resource *res);
 int virtio_gpu_update_dmabuf(VirtIOGPU *g,
                              uint32_t scanout_id,
                              struct virtio_gpu_simple_resource *res,
diff --git a/hw/display/virtio-gpu-udmabuf-stubs.c b/hw/display/virtio-gpu-udmabuf-stubs.c
index f692e13510..85d03935a3 100644
--- a/hw/display/virtio-gpu-udmabuf-stubs.c
+++ b/hw/display/virtio-gpu-udmabuf-stubs.c
@@ -12,7 +12,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
     /* nothing (stub) */
 }
 
-void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
+void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
 {
     /* nothing (stub) */
 }
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index d804f321aa..74b6a7766a 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
     res->blob = pdata;
 }
 
-void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
-{
-    if (res->remapped) {
-        virtio_gpu_destroy_udmabuf(res);
-    }
-}
-
 static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
 {
     struct virtio_gpu_scanout *scanout;
@@ -169,6 +162,26 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
     g_free(dmabuf);
 }
 
+void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
+{
+    int max_outputs = g->parent_obj.conf.max_outputs;
+    int i;
+
+    for (i = 0; i < max_outputs; i++) {
+        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
+
+        if (dmabuf &&
+            qemu_dmabuf_get_num_planes(dmabuf->buf) > 0 &&
+            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd &&
+            res->dmabuf_fd != -1) {
+            qemu_dmabuf_close(dmabuf->buf);
+            res->dmabuf_fd = -1;
+        }
+    }
+
+    virtio_gpu_destroy_udmabuf(res);
+}
+
 static VGPUDMABuf
 *virtio_gpu_create_dmabuf(VirtIOGPU *g,
                           uint32_t scanout_id,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 643e91ca2a..b2af861f0d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
     res->addrs = NULL;
 
     if (res->blob) {
-        virtio_gpu_fini_udmabuf(res);
+        virtio_gpu_fini_udmabuf(g, res);
     }
 }
 
-- 
2.43.0

Re: [PATCH v2] virtio-gpu: Fix scanout dmabuf cleanup during resource destruction

[Alex Bennée]

dongwon.kim@intel.com writes:

> From: Dongwon Kim <dongwon.kim@intel.com>
>
> When a virtio-gpu resource is destroyed, any associated udmabuf must be
> properly torn down. Currently, the code may leave dangling references
> to dmabuf file descriptors in the scanout primary buffers.
>
> This patch updates virtio_gpu_fini_udmabuf to:
> 1. Iterate through all active scanouts.
> 2. Identify dmabufs that match the resource's file descriptor.
> 3. Close the dmabuf and invalidate the resource's FD reference to
>    prevent use-after-free or double-close scenarios.
> 4. Finally, trigger the underlying udmabuf destruction.
>
> This ensures that the display backend does not attempt to access
> memory or FDs that have been released by the guest or the host.

Queued to virtio-gpu/next, thanks.

>
> v2: - Corrected virtio_gpu_fini_udmabuf in stub
>       (Alex Bennée)
>
>     - Make sure that qemu dmabuf has at least one plane before
>       Comparing fds
>       (Marc-André Lureau)

FYI usually we put version information under the --- so it doesn't
pollute the commit message when things are applied.

>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/hw/virtio/virtio-gpu.h        |  3 ++-
>  hw/display/virtio-gpu-udmabuf-stubs.c |  2 +-
>  hw/display/virtio-gpu-udmabuf.c       | 27 ++++++++++++++++++++-------
>  hw/display/virtio-gpu.c               |  2 +-
>  4 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 58e0f91fda..65312f869d 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -357,7 +357,8 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
>  /* virtio-gpu-udmabuf.c */
>  bool virtio_gpu_have_udmabuf(void);
>  void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
> -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
> +void virtio_gpu_fini_udmabuf(VirtIOGPU *g,
> +                             struct virtio_gpu_simple_resource *res);
>  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
>                               uint32_t scanout_id,
>                               struct virtio_gpu_simple_resource *res,
> diff --git a/hw/display/virtio-gpu-udmabuf-stubs.c b/hw/display/virtio-gpu-udmabuf-stubs.c
> index f692e13510..85d03935a3 100644
> --- a/hw/display/virtio-gpu-udmabuf-stubs.c
> +++ b/hw/display/virtio-gpu-udmabuf-stubs.c
> @@ -12,7 +12,7 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>      /* nothing (stub) */
>  }
>  
> -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
> +void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
>  {
>      /* nothing (stub) */
>  }
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index d804f321aa..74b6a7766a 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -151,13 +151,6 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
>      res->blob = pdata;
>  }
>  
> -void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
> -{
> -    if (res->remapped) {
> -        virtio_gpu_destroy_udmabuf(res);
> -    }
> -}
> -
>  static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>  {
>      struct virtio_gpu_scanout *scanout;
> @@ -169,6 +162,26 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf)
>      g_free(dmabuf);
>  }
>  
> +void virtio_gpu_fini_udmabuf(VirtIOGPU *g, struct virtio_gpu_simple_resource *res)
> +{
> +    int max_outputs = g->parent_obj.conf.max_outputs;
> +    int i;
> +
> +    for (i = 0; i < max_outputs; i++) {
> +        VGPUDMABuf *dmabuf = g->dmabuf.primary[i];
> +
> +        if (dmabuf &&
> +            qemu_dmabuf_get_num_planes(dmabuf->buf) > 0 &&
> +            qemu_dmabuf_get_fds(dmabuf->buf, NULL)[0] == res->dmabuf_fd &&
> +            res->dmabuf_fd != -1) {
> +            qemu_dmabuf_close(dmabuf->buf);
> +            res->dmabuf_fd = -1;
> +        }
> +    }
> +
> +    virtio_gpu_destroy_udmabuf(res);
> +}
> +
>  static VGPUDMABuf
>  *virtio_gpu_create_dmabuf(VirtIOGPU *g,
>                            uint32_t scanout_id,
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 643e91ca2a..b2af861f0d 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -902,7 +902,7 @@ void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
>      res->addrs = NULL;
>  
>      if (res->blob) {
> -        virtio_gpu_fini_udmabuf(res);
> +        virtio_gpu_fini_udmabuf(g, res);
>      }
>  }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro