From 330ef31deb2e5461cff907488b710f5bd9cd2327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 8 Oct 2024 16:50:13 +0400 Subject: ui/win32: fix potential use-after-free with dbus shared memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Akihiko Odaki Message-ID: <20241008125028.1177932-5-marcandre.lureau@redhat.com> --- include/ui/qemu-pixman.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/ui') diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h index ef13a82..e3dd72b 100644 --- a/include/ui/qemu-pixman.h +++ b/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_win32_image_destroy(pixman_image_t *image, void *data); + G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref) #endif /* QEMU_PIXMAN_H */ -- cgit v1.1 From 1bfb726112ea4fda07c988f08df32d1eebb9abec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 8 Oct 2024 16:50:18 +0400 Subject: ui/pixman: generalize shared_image_destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Learn to free memfd-allocated shared memory. Signed-off-by: Marc-André Lureau Reviewed-by: Akihiko Odaki Message-ID: <20241008125028.1177932-10-marcandre.lureau@redhat.com> --- include/ui/qemu-pixman.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/ui') diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h index e3dd72b..a97f56d 100644 --- a/include/ui/qemu-pixman.h +++ b/include/ui/qemu-pixman.h @@ -97,7 +97,7 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph, void qemu_pixman_image_unref(pixman_image_t *image); -void qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data); +void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data); G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref) -- cgit v1.1 From ec818df0005d1598d249f0cfea557226b6ad89a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 8 Oct 2024 16:50:21 +0400 Subject: ui/surface: allocate shared memory on !win32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use qemu_memfd_alloc() to allocate the display surface memory, which will fallback on tmpfile/mmap() on systems without memfd, and allow to share the display with other processes. This is similar to how display memory is allocated on win32 since commit 09b4c198 ("console/win32: allocate shareable display surface"). Signed-off-by: Marc-André Lureau Reviewed-by: Akihiko Odaki Message-ID: <20241008125028.1177932-13-marcandre.lureau@redhat.com> --- include/ui/surface.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'include/ui') diff --git a/include/ui/surface.h b/include/ui/surface.h index 345b191..37d03be 100644 --- a/include/ui/surface.h +++ b/include/ui/surface.h @@ -26,6 +26,9 @@ typedef struct DisplaySurface { #ifdef WIN32 HANDLE handle; uint32_t handle_offset; +#else + int shmfd; + uint32_t shmfd_offset; #endif } DisplaySurface; @@ -40,6 +43,9 @@ DisplaySurface *qemu_create_placeholder_surface(int w, int h, #ifdef WIN32 void qemu_displaysurface_win32_set_handle(DisplaySurface *surface, HANDLE h, uint32_t offset); +#else +void qemu_displaysurface_set_shmfd(DisplaySurface *surface, + int shmfd, uint32_t offset); #endif DisplaySurface *qemu_create_displaysurface(int width, int height); -- cgit v1.1 From 1ff788db9781615be745671ebdb2eb82c137c5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 8 Oct 2024 16:50:27 +0400 Subject: ui: refactor using a common qemu_pixman_shareable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a common shareable type for win32 & unix, and helper functions. This simplify the code as it avoids a lot of #ifdef'ery. Note: if it helps review, commits could be reordered to introduce the common type before introducing shareable memory for unix. Suggested-by: Akihiko Odaki Signed-off-by: Marc-André Lureau Reviewed-by: Akihiko Odaki Message-ID: <20241008125028.1177932-19-marcandre.lureau@redhat.com> --- include/ui/qemu-pixman.h | 24 +++++++++++++++++++++++- include/ui/surface.h | 20 ++++++-------------- 2 files changed, 29 insertions(+), 15 deletions(-) (limited to 'include/ui') diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h index a97f56d..193bc04 100644 --- a/include/ui/qemu-pixman.h +++ b/include/ui/qemu-pixman.h @@ -12,6 +12,8 @@ #include "pixman-minimal.h" #endif +#include "qapi/error.h" + /* * pixman image formats are defined to be native endian, * that means host byte order on qemu. So we go define @@ -97,7 +99,27 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph, void qemu_pixman_image_unref(pixman_image_t *image); -void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data); +#ifdef WIN32 +typedef HANDLE qemu_pixman_shareable; +#define SHAREABLE_NONE (NULL) +#define SHAREABLE_TO_PTR(handle) (handle) +#define PTR_TO_SHAREABLE(ptr) (ptr) +#else +typedef int qemu_pixman_shareable; +#define SHAREABLE_NONE (-1) +#define SHAREABLE_TO_PTR(handle) GINT_TO_POINTER(handle) +#define PTR_TO_SHAREABLE(ptr) GPOINTER_TO_INT(ptr) +#endif + +bool qemu_pixman_image_new_shareable( + pixman_image_t **image, + qemu_pixman_shareable *handle, + const char *name, + pixman_format_code_t format, + int width, + int height, + int rowstride_bytes, + Error **errp); G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref) diff --git a/include/ui/surface.h b/include/ui/surface.h index 37d03be..f16f7be 100644 --- a/include/ui/surface.h +++ b/include/ui/surface.h @@ -23,13 +23,8 @@ typedef struct DisplaySurface { GLenum gltype; GLuint texture; #endif -#ifdef WIN32 - HANDLE handle; - uint32_t handle_offset; -#else - int shmfd; - uint32_t shmfd_offset; -#endif + qemu_pixman_shareable share_handle; + uint32_t share_handle_offset; } DisplaySurface; PixelFormat qemu_default_pixelformat(int bpp); @@ -40,13 +35,10 @@ DisplaySurface *qemu_create_displaysurface_from(int width, int height, DisplaySurface *qemu_create_displaysurface_pixman(pixman_image_t *image); DisplaySurface *qemu_create_placeholder_surface(int w, int h, const char *msg); -#ifdef WIN32 -void qemu_displaysurface_win32_set_handle(DisplaySurface *surface, - HANDLE h, uint32_t offset); -#else -void qemu_displaysurface_set_shmfd(DisplaySurface *surface, - int shmfd, uint32_t offset); -#endif + +void qemu_displaysurface_set_share_handle(DisplaySurface *surface, + qemu_pixman_shareable handle, + uint32_t offset); DisplaySurface *qemu_create_displaysurface(int width, int height); void qemu_free_displaysurface(DisplaySurface *surface); -- cgit v1.1