2
0
Эх сурвалжийг харах

ui/win32: fix potential use-after-free with dbus shared memory

DisplaySurface may be free before the pixman image is freed, since the
image is refcounted and used by different objects, including pending
dbus messages.

Furthermore, setting the destroy function in
create_displaysurface_from() isn't appropriate, as it may not be used,
and may be overriden as in ramfb.

Set the destroy function when the shared handle is set, use the HANDLE
directly for destroy data, using a single common helper
qemu_pixman_win32_image_destroy().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-ID: <20241008125028.1177932-5-marcandre.lureau@redhat.com>
(cherry picked from commit 330ef31deb2e5461cff907488b710f5bd9cd2327)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Marc-André Lureau 10 сар өмнө
parent
commit
e1324ec946

+ 2 - 12
hw/display/virtio-gpu.c

@@ -238,16 +238,6 @@ static uint32_t calc_image_hostmem(pixman_format_code_t pformat,
     return height * stride;
     return height * stride;
 }
 }
 
 
-#ifdef WIN32
-static void
-win32_pixman_image_destroy(pixman_image_t *image, void *data)
-{
-    HANDLE handle = data;
-
-    qemu_win32_map_free(pixman_image_get_data(image), handle, &error_warn);
-}
-#endif
-
 static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
 static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
                                           struct virtio_gpu_ctrl_command *cmd)
                                           struct virtio_gpu_ctrl_command *cmd)
 {
 {
@@ -308,7 +298,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
             bits, c2d.height ? res->hostmem / c2d.height : 0);
             bits, c2d.height ? res->hostmem / c2d.height : 0);
 #ifdef WIN32
 #ifdef WIN32
         if (res->image) {
         if (res->image) {
-            pixman_image_set_destroy_function(res->image, win32_pixman_image_destroy, res->handle);
+            pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
         }
         }
 #endif
 #endif
     }
     }
@@ -1327,7 +1317,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
             return -EINVAL;
             return -EINVAL;
         }
         }
 #ifdef WIN32
 #ifdef WIN32
-        pixman_image_set_destroy_function(res->image, win32_pixman_image_destroy, res->handle);
+        pixman_image_set_destroy_function(res->image, qemu_pixman_win32_image_destroy, res->handle);
 #endif
 #endif
 
 
         res->addrs = g_new(uint64_t, res->iov_cnt);
         res->addrs = g_new(uint64_t, res->iov_cnt);

+ 2 - 0
include/ui/qemu-pixman.h

@@ -97,6 +97,8 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
 
 
 void qemu_pixman_image_unref(pixman_image_t *image);
 void qemu_pixman_image_unref(pixman_image_t *image);
 
 
+void qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
 
 
 #endif /* QEMU_PIXMAN_H */
 #endif /* QEMU_PIXMAN_H */

+ 2 - 22
ui/console.c

@@ -461,24 +461,6 @@ void qemu_displaysurface_win32_set_handle(DisplaySurface *surface,
     surface->handle = h;
     surface->handle = h;
     surface->handle_offset = offset;
     surface->handle_offset = offset;
 }
 }
-
-static void
-win32_pixman_image_destroy(pixman_image_t *image, void *data)
-{
-    DisplaySurface *surface = data;
-
-    if (!surface->handle) {
-        return;
-    }
-
-    assert(surface->handle_offset == 0);
-
-    qemu_win32_map_free(
-        pixman_image_get_data(surface->image),
-        surface->handle,
-        &error_warn
-    );
-}
 #endif
 #endif
 
 
 DisplaySurface *qemu_create_displaysurface(int width, int height)
 DisplaySurface *qemu_create_displaysurface(int width, int height)
@@ -504,6 +486,8 @@ DisplaySurface *qemu_create_displaysurface(int width, int height)
 
 
 #ifdef WIN32
 #ifdef WIN32
     qemu_displaysurface_win32_set_handle(surface, handle, 0);
     qemu_displaysurface_win32_set_handle(surface, handle, 0);
+    pixman_image_set_destroy_function(surface->image,
+                                      qemu_pixman_win32_image_destroy, handle);
 #endif
 #endif
     return surface;
     return surface;
 }
 }
@@ -519,10 +503,6 @@ DisplaySurface *qemu_create_displaysurface_from(int width, int height,
                                               width, height,
                                               width, height,
                                               (void *)data, linesize);
                                               (void *)data, linesize);
     assert(surface->image != NULL);
     assert(surface->image != NULL);
-#ifdef WIN32
-    pixman_image_set_destroy_function(surface->image,
-                                      win32_pixman_image_destroy, surface);
-#endif
 
 
     return surface;
     return surface;
 }
 }

+ 15 - 0
ui/qemu-pixman.c

@@ -4,6 +4,7 @@
  */
  */
 
 
 #include "qemu/osdep.h"
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "ui/console.h"
 #include "ui/console.h"
 #include "standard-headers/drm/drm_fourcc.h"
 #include "standard-headers/drm/drm_fourcc.h"
 #include "trace.h"
 #include "trace.h"
@@ -268,3 +269,17 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
     pixman_image_unref(ibg);
     pixman_image_unref(ibg);
 }
 }
 #endif /* CONFIG_PIXMAN */
 #endif /* CONFIG_PIXMAN */
+
+#ifdef WIN32
+void
+qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data)
+{
+    HANDLE handle = data;
+
+    qemu_win32_map_free(
+        pixman_image_get_data(image),
+        handle,
+        &error_warn
+    );
+}
+#endif