From 754d6b6dd0a9ec42d75247596a8885bf54352ee7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <mdaenzer@redhat.com>
Date: Fri, 10 Mar 2023 12:39:30 +0100
Subject: [PATCH] xwayland: Prevent nested xwl_present_for_each_frame_callback
 calls

It could happen with the following call path:

frame_callback
 xwl_present_frame_callback
  xwl_present_msc_bump
   xwl_present_execute
    xwl_present_flip
     xwl_window_create_frame_callback

The nested loop called xwl_present_reset_timer, which may end up calling
xorg_list_del for the entry after the one frame_callback started the
chain for. This resulted in the outer loop never terminating, because
its next element wasn't hooked up to the list anymore.

We avoid this by calling xwl_present_reset_timer as needed in
frame_callback, and bailing from xwl_window_create_frame_callback if it
was called from the former.

We also catch nested calls and FatalError if they ever happen again due
to another bug.

v2:
* Leave xwl_present_reset_timer call in xwl_present_frame_callback,
  needed if xwl_present_msc_bump didn't hook up the window to the frame
  callback list again.

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1442
---
 hw/xwayland/xwayland-present.c | 15 +++++++++++++++
 hw/xwayland/xwayland-present.h |  1 +
 hw/xwayland/xwayland-window.c  | 16 ++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index e8a173774..189e7cfd6 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -89,16 +89,31 @@ xwl_present_event_from_id(uint64_t event_id)
     return (struct xwl_present_event*)(uintptr_t)event_id;
 }
 
+static Bool entered_for_each_frame_callback;
+
+Bool
+xwl_present_entered_for_each_frame_callback(void)
+{
+    return entered_for_each_frame_callback;
+}
+
 void
 xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
                                     void iter_func(struct xwl_present_window *))
 {
     struct xwl_present_window *xwl_present_window, *tmp;
 
+    if (entered_for_each_frame_callback)
+        FatalError("Nested xwl_present_for_each_frame_callback call");
+
+    entered_for_each_frame_callback = TRUE;
+
     xorg_list_for_each_entry_safe(xwl_present_window, tmp,
                                   &xwl_window->frame_callback_list,
                                   frame_callback_list)
         iter_func(xwl_present_window);
+
+    entered_for_each_frame_callback = FALSE;
 }
 
 static void
diff --git a/hw/xwayland/xwayland-present.h b/hw/xwayland/xwayland-present.h
index 620e6c5ca..806272089 100644
--- a/hw/xwayland/xwayland-present.h
+++ b/hw/xwayland/xwayland-present.h
@@ -61,6 +61,7 @@ struct xwl_present_event {
     PixmapPtr pixmap;
 };
 
+Bool xwl_present_entered_for_each_frame_callback(void);
 void xwl_present_for_each_frame_callback(struct xwl_window *xwl_window,
                                          void iter_func(struct xwl_present_window *));
 void xwl_present_reset_timer(struct xwl_present_window *xwl_present_window);
diff --git a/hw/xwayland/xwayland-window.c b/hw/xwayland/xwayland-window.c
index b80a7bf67..6b7f38605 100644
--- a/hw/xwayland/xwayland-window.c
+++ b/hw/xwayland/xwayland-window.c
@@ -1226,8 +1226,16 @@ frame_callback(void *data,
     xwl_window->frame_callback = NULL;
 
 #ifdef GLAMOR_HAS_GBM
-    if (xwl_window->xwl_screen->present)
+    if (xwl_window->xwl_screen->present) {
         xwl_present_for_each_frame_callback(xwl_window, xwl_present_frame_callback);
+
+        /* If xwl_window_create_frame_callback was called from
+         * xwl_present_frame_callback, need to make sure all fallback timers
+         * are adjusted correspondingly.
+         */
+        if (xwl_window->frame_callback)
+            xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
+    }
 #endif
 }
 
@@ -1243,7 +1251,11 @@ xwl_window_create_frame_callback(struct xwl_window *xwl_window)
                              xwl_window);
 
 #ifdef GLAMOR_HAS_GBM
-    if (xwl_window->xwl_screen->present)
+    /* If we get called from frame_callback, it will take care of calling
+     * xwl_present_reset_timer.
+     */
+    if (xwl_window->xwl_screen->present &&
+        !xwl_present_entered_for_each_frame_callback())
         xwl_present_for_each_frame_callback(xwl_window, xwl_present_reset_timer);
 #endif
 }