xserver: Branch 'master' - 4 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Jun 25 16:05:14 UTC 2020


 hw/xwayland/xwayland-present.c |   58 +++++++++++++++++++----------------------
 hw/xwayland/xwayland-present.h |    2 -
 present/present_screen.c       |    8 ++---
 3 files changed, 32 insertions(+), 36 deletions(-)

New commits:
commit b67052742980fd3ec669100048da71e09aa61358
Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Fri Jun 19 18:10:18 2020 +0200

    xwayland: Free all remaining events in xwl_present_cleanup
    
    These events aren't reachable after xwl_present_cleanup, so they're
    leaked if we don't free them first.
    
    This requires storing the pixmap pointer in struct xwl_present_window.
    Luckily, the buffer pointer isn't used for anything, so just replace
    that.
    
    v2:
    * Bump pixmap reference count in xwl_present_flip and drop it in
      xwl_present_free_event, fixes use-after-free in the latter due to the
      pixmap already being destroyed.
    
    Reviewed-by: Dave Airlie <airlied at redhat.com>

diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index 30835b030..181e923b9 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -120,6 +120,16 @@ xwl_present_reset_timer(struct xwl_present_window *xwl_present_window)
 static void
 xwl_present_free_event(struct xwl_present_event *event)
 {
+    if (!event)
+        return;
+
+    if (event->pixmap) {
+        if (!event->buffer_released)
+            xwl_pixmap_del_buffer_release_cb(event->pixmap);
+
+        dixDestroyPixmap(event->pixmap, event->pixmap->drawable.id);
+    }
+
     xorg_list_del(&event->list);
     free(event);
 }
@@ -144,21 +154,10 @@ xwl_present_cleanup(WindowPtr window)
     xorg_list_for_each_entry_safe(event, tmp, &xwl_present_window->event_list, list)
         xwl_present_free_event(event);
 
-    /* Clear remaining buffer releases and inform Present about free ressources */
-    event = xwl_present_window->sync_flip;
-    xwl_present_window->sync_flip = NULL;
-    if (event) {
-        if (event->buffer_released) {
-            xwl_present_free_event(event);
-        } else {
-            event->pending = FALSE;
-            event->abort = TRUE;
-        }
-    }
-    xorg_list_for_each_entry_safe(event, tmp, &xwl_present_window->release_queue, list) {
-        xorg_list_del(&event->list);
-        event->abort = TRUE;
-    }
+    xwl_present_free_event(xwl_present_window->sync_flip);
+
+    xorg_list_for_each_entry_safe(event, tmp, &xwl_present_window->release_queue, list)
+        xwl_present_free_event(event);
 
     /* Clear timer */
     xwl_present_free_timer(xwl_present_window);
@@ -356,6 +355,7 @@ xwl_present_queue_vblank(WindowPtr present_window,
         return BadAlloc;
 
     event->event_id = event_id;
+    event->pixmap = NULL;
     event->xwl_present_window = xwl_present_window;
     event->target_msc = msc;
 
@@ -459,11 +459,12 @@ xwl_present_flip(WindowPtr present_window,
     if (!event)
         return FALSE;
 
+    pixmap->refcnt++;
     buffer = xwl_glamor_pixmap_get_wl_buffer(pixmap, NULL);
 
     event->event_id = event_id;
     event->xwl_present_window = xwl_present_window;
-    event->buffer = buffer;
+    event->pixmap = pixmap;
     event->target_msc = target_msc;
     event->pending = TRUE;
     event->abort = FALSE;
diff --git a/hw/xwayland/xwayland-present.h b/hw/xwayland/xwayland-present.h
index d29430205..a3de1a523 100644
--- a/hw/xwayland/xwayland-present.h
+++ b/hw/xwayland/xwayland-present.h
@@ -59,7 +59,7 @@ struct xwl_present_event {
     Bool buffer_released;
 
     struct xwl_present_window *xwl_present_window;
-    struct wl_buffer *buffer;
+    PixmapPtr pixmap;
 
     struct xorg_list list;
 };
commit 1beffba699e2cc3f23039d2177c025bc127966de
Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Fri Jun 19 18:14:35 2020 +0200

    xwayland: Always use xwl_present_free_event for freeing Present events
    
    Minor cleanup, and will make the next change simpler. No functional
    change intended.
    
    Reviewed-by: Dave Airlie <airlied at redhat.com>

diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index d8ba7b1b3..30835b030 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -117,6 +117,13 @@ xwl_present_reset_timer(struct xwl_present_window *xwl_present_window)
     }
 }
 
+static void
+xwl_present_free_event(struct xwl_present_event *event)
+{
+    xorg_list_del(&event->list);
+    free(event);
+}
+
 void
 xwl_present_cleanup(WindowPtr window)
 {
@@ -134,17 +141,15 @@ xwl_present_cleanup(WindowPtr window)
     }
 
     /* Clear remaining events */
-    xorg_list_for_each_entry_safe(event, tmp, &xwl_present_window->event_list, list) {
-        xorg_list_del(&event->list);
-        free(event);
-    }
+    xorg_list_for_each_entry_safe(event, tmp, &xwl_present_window->event_list, list)
+        xwl_present_free_event(event);
 
     /* Clear remaining buffer releases and inform Present about free ressources */
     event = xwl_present_window->sync_flip;
     xwl_present_window->sync_flip = NULL;
     if (event) {
         if (event->buffer_released) {
-            free(event);
+            xwl_present_free_event(event);
         } else {
             event->pending = FALSE;
             event->abort = TRUE;
@@ -166,13 +171,6 @@ xwl_present_cleanup(WindowPtr window)
     free(xwl_present_window);
 }
 
-static void
-xwl_present_free_event(struct xwl_present_event *event)
-{
-    xorg_list_del(&event->list);
-    free(event);
-}
-
 static void
 xwl_present_buffer_release(PixmapPtr pixmap, void *data)
 {
@@ -219,7 +217,7 @@ xwl_present_msc_bump(struct xwl_present_window *xwl_present_window)
             /* If the buffer was already released, clean up now */
             present_wnmd_event_notify(xwl_present_window->window, event->event_id,
                                       xwl_present_window->ust, msc);
-            free(event);
+            xwl_present_free_event(event);
         } else {
             xorg_list_add(&event->list, &xwl_present_window->release_queue);
         }
@@ -395,8 +393,7 @@ xwl_present_abort_vblank(WindowPtr present_window,
 
     xorg_list_for_each_entry_safe(event, tmp, &xwl_present_window->event_list, list) {
         if (event->event_id == event_id) {
-            xorg_list_del(&event->list);
-            free(event);
+            xwl_present_free_event(event);
             return;
         }
     }
commit 1bdedc8dbb9d035b85444c2558a137470ff52113
Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Fri Jun 19 17:24:42 2020 +0200

    present/wnmd: Free flip_queue entries in present_wnmd_clear_window_flip
    
    When present_wnmd_clear_window_flip is done, present_destroy_window
    frees struct present_window_priv, and the events in the flip queue
    become unreachable. So if we don't free them first, they're leaked.
    
    Also drop the call to present_wnmd_set_abort_flip, which just sets a
    flag in struct present_window_priv and thus can't have any observable
    effect after present_destroy_window.
    
    Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1042
    Reviewed-by: Dave Airlie <airlied at redhat.com>

diff --git a/present/present_screen.c b/present/present_screen.c
index c435f55f4..bfd30b8ba 100644
--- a/present/present_screen.c
+++ b/present/present_screen.c
@@ -115,9 +115,9 @@ present_wnmd_clear_window_flip(WindowPtr window)
     present_window_priv_ptr     window_priv = present_window_priv(window);
     present_vblank_ptr          vblank, tmp;
 
-    if (window_priv->flip_pending) {
-        present_wnmd_set_abort_flip(window);
-        window_priv->flip_pending->window = NULL;
+    xorg_list_for_each_entry_safe(vblank, tmp, &window_priv->flip_queue, event_queue) {
+        present_pixmap_idle(vblank->pixmap, vblank->window, vblank->serial, vblank->idle_fence);
+        present_vblank_destroy(vblank);
     }
 
     xorg_list_for_each_entry_safe(vblank, tmp, &window_priv->idle_queue, event_queue) {
commit bc9dd1c71c3722284ffaa7183f4119151b25a44f
Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Fri Jun 19 17:02:40 2020 +0200

    present/wnmd: Keep pixmap pointer in present_wnmd_clear_window_flip
    
    The comment was incorrect: Any reference held by the window (see
    present_wnmd_execute) is in addition to the one in struct present_vblank
    (see present_vblank_create). So if we don't drop the latter, the pixmap
    will be leaked.
    
    Reviewed-by: Dave Airlie <airlied at redhat.com>

diff --git a/present/present_screen.c b/present/present_screen.c
index c7e37c5fd..c435f55f4 100644
--- a/present/present_screen.c
+++ b/present/present_screen.c
@@ -122,8 +122,6 @@ present_wnmd_clear_window_flip(WindowPtr window)
 
     xorg_list_for_each_entry_safe(vblank, tmp, &window_priv->idle_queue, event_queue) {
         present_pixmap_idle(vblank->pixmap, vblank->window, vblank->serial, vblank->idle_fence);
-        /* The pixmap will be destroyed by freeing the window resources. */
-        vblank->pixmap = NULL;
         present_vblank_destroy(vblank);
     }
 


More information about the xorg-commit mailing list