[PATCH v2 0/1] MR5777: win32u: Limit GDI object generation by 127 for a 64-bit build.
I have a .Net application that during handling of WM_NCPAINT for one of its windows genarates exception "Arithmetic operation resulted in an overflow.". This happens only in a 64-bit prefix and only for WPARAM that contains an HRGN handle with value over than 0x7fffffff. The application does the following: public static IntPtr GetNativeDC(Message m, IntPtr handle) { if (m.Msg != 0x85) // WM_NCPAINT { return NUser32.GetWindowDC(handle); } int num = 0x200013; // DCX_VALIDATE | DCX_CLIPSIBLINGS | DCX_CACHE | DCX_WINDOW IntPtr intPtr = IntPtr.Zero; IntPtr wParam = m.WParam; if (wParam.ToInt32() != 1) { num |= 0x80; // DCX_INTERSECTRGN intPtr = CreateRectRgn(0, 0, 1, 1); CombineRgn(intPtr, wParam, IntPtr.Zero, 5); // RGN_COPY } return NUser32.GetDCEx(handle, intPtr, num); } The exception is generated by wParam.ToInt32(). MSDN description for ToInt32() https://learn.microsoft.com/en-us/dotnet/api/system.intptr.toint32?view=net-... states Exceptions OverflowException In a 64-bit process, the value of this instance is too large or too small to represent as a 32-bit signed integer. MSDN also has a reference to ToInt32() source: https://github.com/dotnet/runtime/blob/5535e31a712343a63f5d7d796cd874e563e5a... public int ToInt32() { #if TARGET_64BIT return checked((int)_value); #else return (int)_value; #endif } It's not clear why a sign extension may lead to an overflow exception. This patch fixes the problem, and provides a test case that confirms that entry.Generation field of a GDI32 object is limited by 127 on a 64-bit platform, while 32-bit Windows doesn't have such a limitation. -- v2: win32u: Limit GDI object generation by 128. https://gitlab.winehq.org/wine/wine/-/merge_requests/5777
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/gdi32/tests/gdiobj.c | 2 ++ dlls/win32u/gdiobj.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/tests/gdiobj.c b/dlls/gdi32/tests/gdiobj.c index b9671c0fb9f..ffe376abed7 100644 --- a/dlls/gdi32/tests/gdiobj.c +++ b/dlls/gdi32/tests/gdiobj.c @@ -403,6 +403,7 @@ static void test_shared_handle_entry( HGDIOBJ obj, unsigned int type, BOOL is_st "Type = %x, expected %x\n", entry->Type, type & 0x1f); ok(entry->Object, "Object = NULL\n"); ok(entry->Owner.Count == 0, "Count = %u\n", entry->Owner.Count); + ok(entry->Generation <= (sizeof(void *) == 8) ? 127 : 255, "Generation = %u\n", entry->Generation); } static void test_shared_handle_table(void) @@ -445,6 +446,7 @@ static void test_shared_handle_table(void) ok(entry->Owner.ProcessId == GetCurrentProcessId(), "ProcessId = %x, expected %lx\n", entry->Owner.ProcessId, GetCurrentProcessId()); ok(entry->Owner.Count == 0, "Count = %u\n", entry->Owner.Count); + ok(entry->Generation <= (sizeof(void *) == 8) ? 127 : 255, "Generation = %u\n", entry->Generation); test_shared_handle_entry( GetStockObject( WHITE_PEN ), NTGDI_OBJ_PEN, TRUE ); test_shared_handle_entry( GetStockObject( WHITE_BRUSH ), NTGDI_OBJ_BRUSH, TRUE ); diff --git a/dlls/win32u/gdiobj.c b/dlls/win32u/gdiobj.c index 057988e99e9..5cf99e39ff0 100644 --- a/dlls/win32u/gdiobj.c +++ b/dlls/win32u/gdiobj.c @@ -743,7 +743,7 @@ HGDIOBJ alloc_gdi_handle( struct gdi_obj_header *obj, DWORD type, const struct g entry->Object = (UINT_PTR)obj; entry->ExtType = type >> NTGDI_HANDLE_TYPE_SHIFT; entry->Type = entry->ExtType & 0x1f; - if (++entry->Generation == 0xff) entry->Generation = 1; + if (++entry->Generation == 0x80) entry->Generation = 1; ret = entry_to_handle( entry ); pthread_mutex_unlock( &gdi_lock ); TRACE( "allocated %s %p %u/%u\n", gdi_obj_type(type), ret, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5777
On Mon Jun 3 18:25:26 2024 +0000, Alexandre Julliard wrote:
You could probably limit to 127 for all platforms. Thanks. Updated the patch with limit 127.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5777#note_72247
On Mon Jun 3 18:25:26 2024 +0000, Dmitry Timoshkov wrote:
Thanks. Updated the patch with limit 127. You need to update tests as well.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5777#note_72266
You need to update tests as well.
What do you mean by updating the tests? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5777#note_72279
On Tue Jun 4 07:08:29 2024 +0000, Dmitry Timoshkov wrote:
You need to update tests as well. What do you mean by updating the tests? Is there anything else that should be improved in the patch?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5777#note_72564
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5777
participants (4)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Huw Davies (＠huw)
-
Jinoh Kang (＠iamahuman)