May 2024 - Wine-commits - list.winehq.org

Yuxuan Shui : rtworkq: Avoid use-after-free.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: aa963129d98a11c85a1ab4862beb47ee3731bf25 URL: https://gitlab.winehq.org/wine/wine/-/commit/aa963129d98a11c85a1ab4862beb47… Author: Yuxuan Shui <yshui(a)codeweavers.com> Date: Wed Nov 1 01:55:28 2023 +0000 rtworkq: Avoid use-after-free. queue_release_pending_item releases the work_item reference but later accesses `item->queue`, which is a potential use-after-free. --- dlls/rtworkq/queue.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c index eebb096ad31..00b77bf6953 100644 --- a/dlls/rtworkq/queue.c +++ b/dlls/rtworkq/queue.c @@ -734,9 +734,10 @@ static HRESULT invoke_async_callback(IRtwqAsyncResult *result) * removed from pending items when it got canceled. */ static BOOL queue_release_pending_item(struct work_item *item) { + struct queue *queue = item->queue; BOOL ret = FALSE; - EnterCriticalSection(&item->queue->cs); + EnterCriticalSection(&queue->cs); if (item->key) { list_remove(&item->entry); @@ -744,7 +745,7 @@ static BOOL queue_release_pending_item(struct work_item *item) item->key = 0; IUnknown_Release(&item->IUnknown_iface); } - LeaveCriticalSection(&item->queue->cs); + LeaveCriticalSection(&queue->cs); return ret; }

1 0

Yuxuan Shui : mfplat/tests: Validate MFCancelWorkItem releases the callback.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: a40c1bc01e0321a7ee73dd603b162647ad8dbd5a URL: https://gitlab.winehq.org/wine/wine/-/commit/a40c1bc01e0321a7ee73dd603b1626… Author: Yuxuan Shui <yshui(a)codeweavers.com> Date: Wed Nov 1 02:23:20 2023 +0000 mfplat/tests: Validate MFCancelWorkItem releases the callback. --- dlls/mfplat/tests/mfplat.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 72f9526c079..5cd9d28f074 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -3599,6 +3599,7 @@ static void test_scheduled_items(void) IMFAsyncResult *result; MFWORKITEM_KEY key, key2; HRESULT hr; + ULONG refcount; callback = create_test_callback(NULL); @@ -3611,6 +3612,9 @@ static void test_scheduled_items(void) hr = MFCancelWorkItem(key); ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); + refcount = IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); + todo_wine ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); + hr = MFCancelWorkItem(key); ok(hr == MF_E_NOT_FOUND || broken(hr == S_OK) /* < win10 */, "Unexpected hr %#lx.\n", hr); @@ -3620,6 +3624,8 @@ static void test_scheduled_items(void) return; } + callback = create_test_callback(NULL); + hr = MFCreateAsyncResult(NULL, &callback->IMFAsyncCallback_iface, NULL, &result); ok(hr == S_OK, "Failed to create result, hr %#lx.\n", hr); @@ -3716,6 +3722,7 @@ static void test_periodic_callback(void) { DWORD period, key; HRESULT hr; + struct test_callback *context = create_test_callback(NULL); hr = MFStartup(MF_VERSION, MFSTARTUP_FULL); ok(hr == S_OK, "Failed to start up, hr %#lx.\n", hr); @@ -3746,6 +3753,16 @@ static void test_periodic_callback(void) ok(periodic_counter > 0, "Unexpected counter value %lu.\n", periodic_counter); + hr= pMFAddPeriodicCallback(periodic_callback, (IUnknown *)&context->IMFAsyncCallback_iface, &key); + ok(hr == S_OK, "Failed to add periodic callback, hr %#lx.\n", hr); + EXPECT_REF(&context->IMFAsyncCallback_iface, 2); + + hr = pMFRemovePeriodicCallback(key); + ok(hr == S_OK, "Failed to remove callback, hr %#lx.\n", hr); + Sleep(500); + todo_wine EXPECT_REF(&context->IMFAsyncCallback_iface, 1); + IMFAsyncCallback_Release(&context->IMFAsyncCallback_iface); + hr = MFShutdown(); ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr); }

1 0

Yuxuan Shui : rtworkq: Release cancelled work items.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: cf243818996ca12304cad2ba5cee4ec3495d8878 URL: https://gitlab.winehq.org/wine/wine/-/commit/cf243818996ca12304cad2ba5cee4e… Author: Yuxuan Shui <yshui(a)codeweavers.com> Date: Wed Nov 1 02:05:38 2023 +0000 rtworkq: Release cancelled work items. Usually the threadpool holds a reference to the `work_item`, which is released when the `work_item`'s callback is invoked. On the other hand, `queue_cancel_item` closes the threadpool object without releasing the `work_item`. So if the callbacks don't get a chance to run - which is not guaranteed - the `work_item` will be leaked. The fix is not as simple as adding a `IUnknown_Release` to `queue_cancel_item`, because the `work_item`'s callback can still be called after CloseThreadpoolTimer/Wait has returned. In fact its callback might currently be running. In which case the callback will access freed memory if `queue_cancel_item` frees the `work_item`. We have to stop any further callbacks to be queued, wait for any currently running callbacks to finish, then finally we can release the `work_item` if it hasn't already been freed during the wait. --- dlls/mf/tests/mf.c | 1 - dlls/mfplat/tests/mfplat.c | 4 ++-- dlls/rtworkq/queue.c | 48 +++++++++++++++++++++++++++++++++++++--------- 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 8ba199557a1..fa5b0f55391 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -4562,7 +4562,6 @@ if (SUCCEEDED(hr)) check_sar_rate_support(sink); ref = IMFMediaSink_Release(sink); - todo_wine ok(ref == 0, "Release returned %ld\n", ref); /* Activation */ diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 5cd9d28f074..a4ca567367c 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -3613,7 +3613,7 @@ static void test_scheduled_items(void) ok(hr == S_OK, "Failed to cancel item, hr %#lx.\n", hr); refcount = IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface); - todo_wine ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); + ok(refcount == 0, "Unexpected refcount %lu.\n", refcount); hr = MFCancelWorkItem(key); ok(hr == MF_E_NOT_FOUND || broken(hr == S_OK) /* < win10 */, "Unexpected hr %#lx.\n", hr); @@ -3760,7 +3760,7 @@ static void test_periodic_callback(void) hr = pMFRemovePeriodicCallback(key); ok(hr == S_OK, "Failed to remove callback, hr %#lx.\n", hr); Sleep(500); - todo_wine EXPECT_REF(&context->IMFAsyncCallback_iface, 1); + EXPECT_REF(&context->IMFAsyncCallback_iface, 1); IMFAsyncCallback_Release(&context->IMFAsyncCallback_iface); hr = MFShutdown(); diff --git a/dlls/rtworkq/queue.c b/dlls/rtworkq/queue.c index 00b77bf6953..9046a70b359 100644 --- a/dlls/rtworkq/queue.c +++ b/dlls/rtworkq/queue.c @@ -883,7 +883,8 @@ static HRESULT queue_submit_timer(struct queue *queue, IRtwqAsyncResult *result, static HRESULT queue_cancel_item(struct queue *queue, RTWQWORKITEM_KEY key) { - HRESULT hr = RTWQ_E_NOT_FOUND; + TP_WAIT *wait_object; + TP_TIMER *timer_object; struct work_item *item; EnterCriticalSection(&queue->cs); @@ -891,29 +892,58 @@ static HRESULT queue_cancel_item(struct queue *queue, RTWQWORKITEM_KEY key) { if (item->key == key) { + /* We can't immediately release the item here, because the callback could already be + * running somewhere else. And if we release it here, the callback will access freed memory. + * So instead we have to make sure the callback is really stopped, or has really finished + * running before we do that. And we can't do that in this critical section, which would be a + * deadlock. So we first keep an extra reference to it, then leave the critical section to + * wait for the thread-pool objects, finally we re-enter critical section to release it. */ key >>= 32; + IUnknown_AddRef(&item->IUnknown_iface); if ((key & WAIT_ITEM_KEY_MASK) == WAIT_ITEM_KEY_MASK) { - IRtwqAsyncResult_SetStatus(item->result, RTWQ_E_OPERATION_CANCELLED); - invoke_async_callback(item->result); - CloseThreadpoolWait(item->u.wait_object); + wait_object = item->u.wait_object; item->u.wait_object = NULL; + LeaveCriticalSection(&queue->cs); + + SetThreadpoolWait(wait_object, NULL, NULL); + WaitForThreadpoolWaitCallbacks(wait_object, TRUE); + CloseThreadpoolWait(wait_object); } else if ((key & SCHEDULED_ITEM_KEY_MASK) == SCHEDULED_ITEM_KEY_MASK) { - CloseThreadpoolTimer(item->u.timer_object); + timer_object = item->u.timer_object; item->u.timer_object = NULL; + LeaveCriticalSection(&queue->cs); + + SetThreadpoolTimer(timer_object, NULL, 0, 0); + WaitForThreadpoolTimerCallbacks(timer_object, TRUE); + CloseThreadpoolTimer(timer_object); } else + { WARN("Unknown item key mask %#I64x.\n", key); - queue_release_pending_item(item); - hr = S_OK; - break; + LeaveCriticalSection(&queue->cs); + } + + if (queue_release_pending_item(item)) + { + /* This means the callback wasn't run during our wait, so we can invoke the + * callback with a canceled status, and release the work item. */ + if ((key & WAIT_ITEM_KEY_MASK) == WAIT_ITEM_KEY_MASK) + { + IRtwqAsyncResult_SetStatus(item->result, RTWQ_E_OPERATION_CANCELLED); + invoke_async_callback(item->result); + } + IUnknown_Release(&item->IUnknown_iface); + } + IUnknown_Release(&item->IUnknown_iface); + return S_OK; } } LeaveCriticalSection(&queue->cs); - return hr; + return RTWQ_E_NOT_FOUND; } static HRESULT alloc_user_queue(const struct queue_desc *desc, DWORD *queue_id)

1 0

Esme Povirk : gdiplus: Bracket HDC use in draw_poly.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: 96a65cbfd10431aa1c5846333002ed9e6b65f7f1 URL: https://gitlab.winehq.org/wine/wine/-/commit/96a65cbfd10431aa1c5846333002ed… Author: Esme Povirk <esme(a)codeweavers.com> Date: Thu May 2 15:03:02 2024 -0500 gdiplus: Bracket HDC use in draw_poly. --- dlls/gdiplus/graphics.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index bd77758eb51..9e08ddeb5bf 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -1973,6 +1973,7 @@ static void shorten_bezier_amt(GpPointF * pt, REAL amt, BOOL rev) static GpStatus draw_poly(GpGraphics *graphics, GpPen *pen, GDIPCONST GpPointF * pt, GDIPCONST BYTE * types, INT count, BOOL caps) { + HDC hdc; POINT *pti = malloc(count * sizeof(POINT)); BYTE *tp = malloc(count); GpPointF *ptcopy = malloc(count * sizeof(GpPointF)); @@ -2085,7 +2086,13 @@ static GpStatus draw_poly(GpGraphics *graphics, GpPen *pen, GDIPCONST GpPointF * tp[i] = convert_path_point_type(types[i]); } - PolyDraw(graphics->hdc, pti, tp, count); + status = gdi_dc_acquire(graphics, &hdc); + if (status != Ok) + goto end; + + PolyDraw(hdc, pti, tp, count); + + gdi_dc_release(graphics, hdc); status = Ok;

1 0

Esme Povirk : gdiplus: Bracket HDC use in trace_path.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: 8cc552a12363adb47c5b8c2c437dda4d82386075 URL: https://gitlab.winehq.org/wine/wine/-/commit/8cc552a12363adb47c5b8c2c437dda… Author: Esme Povirk <esme(a)codeweavers.com> Date: Thu May 2 15:11:06 2024 -0500 gdiplus: Bracket HDC use in trace_path. --- dlls/gdiplus/graphics.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 9e08ddeb5bf..e849446d2a7 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -2106,12 +2106,20 @@ end: GpStatus trace_path(GpGraphics *graphics, GpPath *path) { + HDC hdc; GpStatus result; - BeginPath(graphics->hdc); + result = gdi_dc_acquire(graphics, &hdc); + if (result != Ok) + return result; + + BeginPath(hdc); result = draw_poly(graphics, NULL, path->pathdata.Points, path->pathdata.Types, path->pathdata.Count, FALSE); - EndPath(graphics->hdc); + EndPath(hdc); + + gdi_dc_release(graphics, hdc); + return result; }

1 0

Esme Povirk : gdiplus: Do not use hdc directly in get_graphics_bounds.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: b95a40a28fa88f2d7c1d13a55e7037e729ce94b7 URL: https://gitlab.winehq.org/wine/wine/-/commit/b95a40a28fa88f2d7c1d13a55e7037… Author: Esme Povirk <esme(a)codeweavers.com> Date: Thu May 2 15:18:50 2024 -0500 gdiplus: Do not use hdc directly in get_graphics_bounds. --- dlls/gdiplus/graphics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index e849446d2a7..229518307de 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -2268,7 +2268,7 @@ static GpStatus get_graphics_bounds(GpGraphics* graphics, GpRectF* rect) { GpStatus stat = get_graphics_device_bounds(graphics, rect); - if (stat == Ok && graphics->hdc) + if (stat == Ok && has_gdi_dc(graphics)) { GpPointF points[4], min_point, max_point; int i;

1 0

Esme Povirk : gdiplus: Accept an HDC in get_gdi_transform.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: 7b300bae8af31860a45f32e27e1699bc1ea9fbe0 URL: https://gitlab.winehq.org/wine/wine/-/commit/7b300bae8af31860a45f32e27e1699… Author: Esme Povirk <esme(a)codeweavers.com> Date: Thu May 2 15:25:53 2024 -0500 gdiplus: Accept an HDC in get_gdi_transform. --- dlls/gdiplus/graphics.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 229518307de..14dfb41376b 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -2418,17 +2418,17 @@ GpStatus WINGDIPAPI GdipCreateFromHDC(HDC hdc, GpGraphics **graphics) return GdipCreateFromHDC2(hdc, NULL, graphics); } -static void get_gdi_transform(GpGraphics *graphics, GpMatrix *matrix) +static void get_gdi_transform(HDC hdc, GpMatrix *matrix) { XFORM xform; - if (graphics->hdc == NULL) + if (hdc == NULL) { GdipSetMatrixElements(matrix, 1.0, 0.0, 0.0, 1.0, 0.0, 0.0); return; } - GetTransform(graphics->hdc, 0x204, &xform); + GetTransform(hdc, 0x204, &xform); GdipSetMatrixElements(matrix, xform.eM11, xform.eM12, xform.eM21, xform.eM22, xform.eDx, xform.eDy); } @@ -2483,7 +2483,7 @@ GpStatus WINGDIPAPI GdipCreateFromHDC2(HDC hdc, HANDLE hDevice, GpGraphics **gra list_init(&(*graphics)->containers); (*graphics)->contid = 0; (*graphics)->printer_display = (GetDeviceCaps(hdc, TECHNOLOGY) == DT_RASPRINTER); - get_gdi_transform(*graphics, &(*graphics)->gdi_transform); + get_gdi_transform(hdc, &(*graphics)->gdi_transform); (*graphics)->gdi_clip = CreateRectRgn(0,0,0,0); if (!GetClipRgn(hdc, (*graphics)->gdi_clip))

1 0

Esme Povirk : gdiplus: Fix background key.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: c6abeee9f0ce325ba53d83bd82663b171a91b6d8 URL: https://gitlab.winehq.org/wine/wine/-/commit/c6abeee9f0ce325ba53d83bd82663b… Author: Esme Povirk <esme(a)codeweavers.com> Date: Fri May 3 13:15:53 2024 -0500 gdiplus: Fix background key. --- dlls/gdiplus/graphics.c | 2 +- dlls/gdiplus/tests/graphics.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 14dfb41376b..86c156354ef 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -6908,7 +6908,7 @@ GpStatus WINGDIPAPI GdipMultiplyWorldTransform(GpGraphics *graphics, GDIPCONST G } /* Color used to fill bitmaps so we can tell which parts have been drawn over by gdi32. */ -static const COLORREF DC_BACKGROUND_KEY = 0x0c0b0d; +static const COLORREF DC_BACKGROUND_KEY = 0x0d0b0c; GpStatus WINGDIPAPI GdipGetDC(GpGraphics *graphics, HDC *hdc) { diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 4af7f179ec2..ff6a64e1fb5 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -2699,7 +2699,7 @@ static void test_fromMemoryBitmap(void) color = GetPixel(hdc, 0, 0); /* The HDC is write-only, and native fills with a solid color to figure out * which pixels have changed. */ - todo_wine expect(0x0c0b0d, color); + expect(0x0c0b0d, color); SetPixel(hdc, 0, 0, 0x797979); SetPixel(hdc, 1, 0, 0x0c0b0d); @@ -2710,7 +2710,7 @@ static void test_fromMemoryBitmap(void) GdipDeleteGraphics(graphics); expect(0x79, bits[0]); - todo_wine expect(0x68, bits[3]); + expect(0x68, bits[3]); GdipDisposeImage((GpImage*)bitmap); @@ -2747,7 +2747,7 @@ static void test_fromMemoryBitmap(void) ok(hdc != NULL, "got NULL hdc\n"); color = GetPixel(hdc, 0, 0); - todo_wine expect(0x0c0b0d, color); + expect(0x0c0b0d, color); status = GdipReleaseDC(graphics, hdc); expect(Ok, status);

1 0

Georg Lehmann : winevulkan: Update to VK spec version 1.3.285.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: 6d896d9be083eb11b1ec3e1c39b3017dbe21cb04 URL: https://gitlab.winehq.org/wine/wine/-/commit/6d896d9be083eb11b1ec3e1c39b301… Author: Georg Lehmann <dadschoorse(a)gmail.com> Date: Sat May 11 09:45:48 2024 +0200 winevulkan: Update to VK spec version 1.3.285. --- dlls/winevulkan/make_vulkan | 2 +- dlls/winevulkan/vulkan_thunks.c | 233 ++++++++++++++++++++++++++++++++++++++++ dlls/winevulkan/winevulkan.json | 2 +- include/wine/vulkan.h | 62 ++++++++++- 4 files changed, 294 insertions(+), 5 deletions(-)

1 0

Elizabeth Figura : d3d9/tests: Clarify and expand point size tests.
by Alexandre Julliard 12 May '24

12 May '24

Module: wine Branch: master Commit: b01b5c70ea439c79c1ea22a80999d9a8cd24f434 URL: https://gitlab.winehq.org/wine/wine/-/commit/b01b5c70ea439c79c1ea22a80999d9… Author: Elizabeth Figura <zfigura(a)codeweavers.com> Date: Tue Apr 30 19:21:21 2024 -0500 d3d9/tests: Clarify and expand point size tests. --- dlls/d3d9/tests/visual.c | 209 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 177 insertions(+), 32 deletions(-)

1 0