Avoid potential re-entrancy of xserver code in winPositionWindowMultiwindow

Consider the following stack trace:

0  ConfigureWindow (pWin=pWin@entry=0x6004d40a0, mask=mask@entry=3, vlist=vlist@entry=0xffffb990, client=0x6004d31d0) at /usr/src/debug/xorg-server-1.18.1-1/dix/window.c:2199
1  0x0000000100415e13 in winAdjustXWindow (pWin=0x6004d40a0, hwnd=hwnd@entry=0x80490) at /usr/src/debug/xorg-server-1.18.1-1/hw/xwin/winmultiwindowwindow.c:1102
2  0x0000000100419eeb in winTopLevelWindowProc (hwnd=0x80490, message=<optimized out>, wParam=0, lParam=2197848832) at /usr/src/debug/xorg-server-1.18.1-1/hw/xwin/winmultiwindowwndproc.c:885
3  0x00007ffc8ae21169 in USER32!DispatchMessageW () from /cygdrive/c/WINDOWS/system32/USER32.dll
4  0x00007ffc8ae20ee2 in USER32!DispatchMessageW () from /cygdrive/c/WINDOWS/system32/USER32.dll
5  0x00007ffc8ae3098e in USER32!GetMenuItemInfoW () from /cygdrive/c/WINDOWS/system32/USER32.dll
6  0x00007ffc8b438b44 in ntdll!KiUserCallbackDispatcher () from /cygdrive/c/WINDOWS/SYSTEM32/ntdll.dll
7  0x00007ffc8ae41f94 in USER32!InvalidateRect () from /cygdrive/c/WINDOWS/system32/USER32.dll
8  0x00007ffc8ae207c5 in USER32!SendMessageW () from /cygdrive/c/WINDOWS/system32/USER32.dll
9  0x00007ffc8ae1e835 in USER32!GetWindowTextW () from /cygdrive/c/WINDOWS/system32/USER32.dll
10 0x00007ffc8ae24fbd in USER32!CallMsgFilterW () from /cygdrive/c/WINDOWS/system32/USER32.dll
11 0x00007ffc86347ced in UxTheme!CloseThemeData () from /cygdrive/c/WINDOWS/system32/uxtheme.dll
12 0x00007ffc8633566e in UxTheme!GetWindowTheme () from /cygdrive/c/WINDOWS/system32/uxtheme.dll
13 0x00007ffc8ae25062 in USER32!CallMsgFilterW () from /cygdrive/c/WINDOWS/system32/USER32.dll
14 0x0000000100419b16 in winTopLevelWindowProc (hwnd=0x80490, message=<optimized out>, wParam=0, lParam=4294952128) at /usr/src/debug/xorg-server-1.18.1-1/hw/xwin/winmultiwindowwndproc.c:1155
15 0x00007ffc8ae21169 in USER32!DispatchMessageW () from /cygdrive/c/WINDOWS/system32/USER32.dll
16 0x00007ffc8ae20ee2 in USER32!DispatchMessageW () from /cygdrive/c/WINDOWS/system32/USER32.dll
17 0x00007ffc8ae34d02 in UnionRect () from /cygdrive/c/WINDOWS/system32/USER32.dll
18 0x00007ffc8b438b44 in ntdll!KiUserCallbackDispatcher () from /cygdrive/c/WINDOWS/SYSTEM32/ntdll.dll
19 0x00007ffc8ae42a54 in USER32!MoveWindow () from /cygdrive/c/WINDOWS/system32/USER32.dll
20 0x00000001004154b0 in winPositionWindowMultiWindow (pWin=<optimized out>, x=<optimized out>, y=<optimized out>) at /usr/src/debug/xorg-server-1.18.1-1/hw/xwin/winmultiwindowwindow.c:333
21 0x00000001004e5649 in compPositionWindow (pWin=<optimized out>, x=<optimized out>, y=<optimized out>) at /usr/src/debug/xorg-server-1.18.1-1/composite/compwindow.c:247
22 0x000000010048b6e3 in miDbePositionWindow (pWin=0x6004d40a0, x=<optimized out>, y=<optimized out>) at /usr/src/debug/xorg-server-1.18.1-1/dbe/midbe.c:493
23 0x000000010056743a in miResizeWindow (pWin=0x6004d40a0, x=2110, y=243, w=<optimized out>, h=474, pSib=0x6004d3090) at /usr/src/debug/xorg-server-1.18.1-1/mi/miwindow.c:447
24 0x00000001004150d1 in winResizeWindowMultiWindow (pWin=<optimized out>, x=<optimized out>, y=<optimized out>, w=<optimized out>, h=474, pSib=0x6004d3090) at /usr/src/debug/xorg-server-1.18.1-1/hw/xwin/winmultiwindowwindow.c:1053
25 0x00000001004e53d1 in compResizeWindow (pWin=0x6004d40a0, x=<optimized out>, y=<optimized out>, w=<optimized out>, h=474, pSib=0x6004d3090) at /usr/src/debug/xorg-server-1.18.1-1/composite/compwindow.c:410
26 0x00000001005470f2 in ConfigureWindow (pWin=0x6004d40a0, mask=<optimized out>, vlist=vlist@entry=0x6004dda1c, client=client@entry=0x6004d31d0) at /usr/src/debug/xorg-server-1.18.1-1/dix/window.c:2422
27 0x0000000100515255 in ProcConfigureWindow (client=0x6004d31d0) at /usr/src/debug/xorg-server-1.18.1-1/dix/dispatch.c:868
28 0x000000010051a44f in Dispatch () at /usr/src/debug/xorg-server-1.18.1-1/dix/dispatch.c:430
29 0x000000010051e4f6 in dix_main (argc=3, argv=0xffffcc10, envp=<optimized out>) at /usr/src/debug/xorg-server-1.18.1-1/dix/main.c:300
30 0x00000001800484ad in _cygwin_exit_return () at /usr/src/debug/cygwin-2.4.1-1/winsup/cygwin/dcrt0.cc:1048
31 0x000000018004618c in _cygtls::call2 (this=0xffffce00, func=0x180047500 <dll_crt0_1(void*)>, arg=0x0, buf=buf@entry=0xffffcdf0) at /usr/src/debug/cygwin-2.4.1-1/winsup/cygwin/cygtls.cc:111
32 0x0000000180046224 in _cygtls::call (func=<optimized out>, arg=<optimized out>) at /usr/src/debug/cygwin-2.4.1-1/winsup/cygwin/cygtls.cc:30

winPositionWindowMultiWindow() calls MoveWindow(), which pumps the message
queue to process the message(s) it sends, which calls
winTopLevelWindowProc(), which can end up calling ConfigureWindow() again.

ConfigureWindow() is not designed for re-entrancy.  In particular, this will
bypass any layers which are in the middle of an unwrapped call.

It seems that composite handles this situation particularly badly, leading
to the composite window position getting set incorrectly, which later leads
to a crash as we try to access an area outside the window bitmap.

Address this in this specific case by deferring the call to MoveWindow(),
but I'm not sure there aren't other potential cases of this.  The real
solution is a different architecture.

3 files changed, 39 insertions, 21 deletions

diff --git a/hw/xwin/win.h b/hw/xwin/win.h
index 7bba32361..16e50f894 100644
--- a/hw/xwin/win.h
+++ b/hw/xwin/win.h
@@ -187,6 +187,7 @@
 #define WM_TRAYICON		(WM_USER + 1000)
 #define WM_INIT_SYS_MENU	(WM_USER + 1001)
 #define WM_GIVEUP		(WM_USER + 1002)
+#define WM_ASYNCMOVE		(WM_USER + 1003)
 
 /* Local includes */
 #include "winwindow.h"
@@ -976,6 +977,9 @@ XID
 
 int
  winAdjustXWindow(WindowPtr pWin, HWND hwnd);
+
+void
+ winAdjustWindowsWindow(WindowPtr pWin, HWND hwnd);
 #endif
 
 #ifdef XWIN_MULTIWINDOW
diff --git a/hw/xwin/winmultiwindowwindow.c b/hw/xwin/winmultiwindowwindow.c
index e81fe2fcc..10495c2c0 100644
--- a/hw/xwin/winmultiwindowwindow.c
+++ b/hw/xwin/winmultiwindowwindow.c
@@ -202,32 +202,16 @@ winDestroyWindowMultiWindow(WindowPtr pWin)
 
 /*
  * PositionWindow - See Porting Layer Definition - p. 37
- *
- * This function adjusts the position and size of Windows window
- * with respect to the underlying X window.  This is the inverse
- * of winAdjustXWindow, which adjusts X window to Windows window.
  */
 
 Bool
 winPositionWindowMultiWindow(WindowPtr pWin, int x, int y)
 {
     Bool fResult = TRUE;
-    int iX, iY, iWidth, iHeight;
     ScreenPtr pScreen = pWin->drawable.pScreen;
-
     winWindowPriv(pWin);
     winScreenPriv(pScreen);
-
     HWND hWnd = pWinPriv->hWnd;
-    RECT rcNew;
-    RECT rcOld;
-
-#if CYGMULTIWINDOW_DEBUG
-    RECT rcClient;
-    RECT *lpRc;
-#endif
-    DWORD dwExStyle;
-    DWORD dwStyle;
 
 #if CYGMULTIWINDOW_DEBUG
     winTrace("winPositionWindowMultiWindow - pWin: %p\n", pWin);
@@ -249,6 +233,36 @@ winPositionWindowMultiWindow(WindowPtr pWin, int x, int y)
         return fResult;
     }
 
+    /*
+      We can't call MoveWindow() here, because that sends messages and pumps the
+      message queue, which could re-entrantly call ConfigureWindow(), which
+      would be bad..., so instead just send a message to cause MoveWindow() to
+      be called.
+    */
+    PostMessage(hWnd, WM_ASYNCMOVE, 0, 0);
+
+    return fResult;
+}
+
+/*
+ * This function adjusts the position and size of Windows window
+ * with respect to the underlying X window.  This is the inverse
+ * of winAdjustXWindow, which adjusts X window to Windows window.
+ */
+void
+winAdjustWindowsWindow(WindowPtr pWin, HWND hWnd)
+{
+    int iX, iY, iWidth, iHeight;
+    RECT rcNew;
+    RECT rcOld;
+
+#if CYGMULTIWINDOW_DEBUG
+    RECT rcClient;
+    RECT *lpRc;
+#endif
+    DWORD dwExStyle;
+    DWORD dwStyle;
+
     if (!isToplevelWindow(pWin)) {
         POINT parentOrigin;
 
@@ -268,8 +282,6 @@ winPositionWindowMultiWindow(WindowPtr pWin, int x, int y)
         MoveWindow(hWnd,
                    iX - parentOrigin.x, iY - parentOrigin.y, iWidth, iHeight,
                    TRUE);
-
-        return fResult;
     }
 
     /* Get the Windows window style and extended style */
@@ -336,11 +348,9 @@ winPositionWindowMultiWindow(WindowPtr pWin, int x, int y)
     }
     else {
 #if CYGMULTIWINDOW_DEBUG
-        ErrorF("winPositionWindowMultiWindow - Not need to move\n");
+        ErrorF("winPositionWindowMultiWindow - No need to move\n");
 #endif
     }
-
-    return fResult;
 }
 
 /*
diff --git a/hw/xwin/winmultiwindowwndproc.c b/hw/xwin/winmultiwindowwndproc.c
index a6187e6d4..cecb9716c 100644
--- a/hw/xwin/winmultiwindowwndproc.c
+++ b/hw/xwin/winmultiwindowwndproc.c
@@ -1148,6 +1148,10 @@ winTopLevelWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
         }
         break;
 
+    case WM_ASYNCMOVE:
+        winAdjustWindowsWindow(pWin, hwnd);
+        break;
+
     default:
         break;
     }