[PATCH 3/5] ddraw: Don't read clipper methods from the vtable.

Stefan Dösinger stefan at codeweavers.com
Mon Mar 4 13:59:02 CST 2019


This fixes a memory leak on alt-tab in Deus Ex: GOTY. It steals a
clipper reference from the primary surface, destroying the clipper. The
primary will try to release its reference, causing a segfault that is
handled fairly gracefully by the game. The game will seemingly continue
fine, but the ddraw object is not destroyed, leaving the wined3d
swapchain and its textures behind.

Signed-off-by: Stefan Dösinger <stefan at codeweavers.com>
---
 dlls/ddraw/clipper.c       | 18 ++++++++++++------
 dlls/ddraw/ddraw_private.h |  9 +++++++++
 dlls/ddraw/surface.c       | 17 ++++++++---------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/dlls/ddraw/clipper.c b/dlls/ddraw/clipper.c
index 95036538d8f..e7e96eb643d 100644
--- a/dlls/ddraw/clipper.c
+++ b/dlls/ddraw/clipper.c
@@ -73,7 +73,7 @@ static HRESULT WINAPI ddraw_clipper_QueryInterface(IDirectDrawClipper *iface, RE
     return E_NOINTERFACE;
 }
 
-static ULONG WINAPI ddraw_clipper_AddRef(IDirectDrawClipper *iface)
+ULONG WINAPI ddraw_clipper_AddRef(IDirectDrawClipper *iface)
 {
     struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
     ULONG refcount;
@@ -89,7 +89,7 @@ static ULONG WINAPI ddraw_clipper_AddRef(IDirectDrawClipper *iface)
     return refcount;
 }
 
-static ULONG WINAPI ddraw_clipper_Release(IDirectDrawClipper *iface)
+ULONG WINAPI ddraw_clipper_Release(IDirectDrawClipper *iface)
 {
     struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
     ULONG refcount;
@@ -192,7 +192,7 @@ static HRGN get_window_region(HWND window)
  * RETURNS
  *  Either DD_OK or DDERR_*
  ************************************************************************/
-static HRESULT WINAPI ddraw_clipper_GetClipList(IDirectDrawClipper *iface, RECT *rect,
+HRESULT WINAPI ddraw_clipper_GetClipList(IDirectDrawClipper *iface, RECT *rect,
         RGNDATA *clip_list, DWORD *clip_list_size)
 {
     struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
@@ -308,7 +308,7 @@ static HRESULT WINAPI ddraw_clipper_SetClipList(IDirectDrawClipper *iface, RGNDA
     return DD_OK;
 }
 
-static HRESULT WINAPI ddraw_clipper_GetHWnd(IDirectDrawClipper *iface, HWND *window)
+HRESULT WINAPI ddraw_clipper_GetHWnd(IDirectDrawClipper *iface, HWND *window)
 {
     struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
 
@@ -386,9 +386,15 @@ HRESULT ddraw_clipper_init(struct ddraw_clipper *clipper)
 
 struct ddraw_clipper *unsafe_impl_from_IDirectDrawClipper(IDirectDrawClipper *iface)
 {
+    struct ddraw_clipper *clipper;
+
     if (!iface)
         return NULL;
-    assert(iface->lpVtbl == &ddraw_clipper_vtbl);
 
-    return impl_from_IDirectDrawClipper(iface);
+    clipper = impl_from_IDirectDrawClipper(iface);
+
+    if (!clipper || iface->lpVtbl != &ddraw_clipper_vtbl)
+        WARN("The application passed us a bad clipper object.\n");
+
+    return clipper;
 }
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index e6971e9bc7f..4a3d3027249 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -393,6 +393,15 @@ struct ddraw_clipper
 HRESULT ddraw_clipper_init(struct ddraw_clipper *clipper) DECLSPEC_HIDDEN;
 struct ddraw_clipper *unsafe_impl_from_IDirectDrawClipper(IDirectDrawClipper *iface) DECLSPEC_HIDDEN;
 
+/* Invoke clipper methods directly instead of going through the vtable. Clipper methods have
+ * protections against invalid clipper objects, but that won't work if we crash when reading
+ * the vtable. */
+ULONG WINAPI ddraw_clipper_AddRef(IDirectDrawClipper *iface);
+ULONG WINAPI ddraw_clipper_Release(IDirectDrawClipper *iface);
+HRESULT WINAPI ddraw_clipper_GetClipList(IDirectDrawClipper *iface, RECT *rect,
+        RGNDATA *clip_list, DWORD *clip_list_size);
+HRESULT WINAPI ddraw_clipper_GetHWnd(IDirectDrawClipper *iface, HWND *window);
+
 /*****************************************************************************
  * IDirectDrawPalette implementation structure
  *****************************************************************************/
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 0063a1eddf8..2a5404f03fd 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -1552,7 +1552,7 @@ static HRESULT ddraw_surface_blt_clipped(struct ddraw_surface *dst_surface, cons
     scale_x = (float)(src_rect.right - src_rect.left) / (float)(dst_rect.right - dst_rect.left);
     scale_y = (float)(src_rect.bottom - src_rect.top) / (float)(dst_rect.bottom - dst_rect.top);
 
-    if (FAILED(hr = IDirectDrawClipper_GetClipList(&dst_surface->clipper->IDirectDrawClipper_iface,
+    if (FAILED(hr = ddraw_clipper_GetClipList(&dst_surface->clipper->IDirectDrawClipper_iface,
             &dst_rect, NULL, &clip_list_size)))
     {
         WARN("Failed to get clip list size, hr %#x.\n", hr);
@@ -1565,7 +1565,7 @@ static HRESULT ddraw_surface_blt_clipped(struct ddraw_surface *dst_surface, cons
         return E_OUTOFMEMORY;
     }
 
-    if (FAILED(hr = IDirectDrawClipper_GetClipList(&dst_surface->clipper->IDirectDrawClipper_iface,
+    if (FAILED(hr = ddraw_clipper_GetClipList(&dst_surface->clipper->IDirectDrawClipper_iface,
             &dst_rect, clip_list, &clip_list_size)))
     {
         WARN("Failed to get clip list, hr %#x.\n", hr);
@@ -4441,7 +4441,7 @@ static HRESULT WINAPI ddraw_surface7_GetClipper(IDirectDrawSurface7 *iface, IDir
     }
 
     *Clipper = &surface->clipper->IDirectDrawClipper_iface;
-    IDirectDrawClipper_AddRef(*Clipper);
+    ddraw_clipper_AddRef(*Clipper);
     wined3d_mutex_unlock();
 
     return DD_OK;
@@ -4515,16 +4515,15 @@ static HRESULT WINAPI ddraw_surface7_SetClipper(IDirectDrawSurface7 *iface,
     This->clipper = clipper;
 
     if (clipper != NULL)
-        IDirectDrawClipper_AddRef(iclipper);
+        ddraw_clipper_AddRef(iclipper);
     if (old_clipper)
-        IDirectDrawClipper_Release(&old_clipper->IDirectDrawClipper_iface);
+        ddraw_clipper_Release(&old_clipper->IDirectDrawClipper_iface);
 
     if ((This->surface_desc.ddsCaps.dwCaps & DDSCAPS_PRIMARYSURFACE) && This->ddraw->wined3d_swapchain)
     {
         clipWindow = NULL;
-        if(clipper) {
-            IDirectDrawClipper_GetHWnd(iclipper, &clipWindow);
-        }
+        if (clipper)
+            ddraw_clipper_GetHWnd(iclipper, &clipWindow);
 
         if (clipWindow)
         {
@@ -5798,7 +5797,7 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
     list_remove(&surface->surface_list_entry);
 
     if (surface->clipper)
-        IDirectDrawClipper_Release(&surface->clipper->IDirectDrawClipper_iface);
+        ddraw_clipper_Release(&surface->clipper->IDirectDrawClipper_iface);
 
     if (surface == surface->ddraw->primary)
     {
-- 
2.19.2




More information about the wine-devel mailing list