[PATCH v2 0/1] MR5777: win32u: Limit GDI object generation by 127 for a 64-bit build.

[Dmitry Timoshkov (＠dmitry)]

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

[PATCH v2 1/1] win32u: Limit GDI object generation by 128.

[Dmitry Timoshkov]

From: Dmitry Timoshkov

Signed-off-by: Dmitry Timoshkov 
---
 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,

[Dmitry Timoshkov (＠dmitry)]

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.

[Jinoh Kang (＠iamahuman)]

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.

[Dmitry Timoshkov (＠dmitry)]

> You need to update tests as well.

What do you mean by updating the tests?

[Dmitry Timoshkov (＠dmitry)]

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?

[PATCH v2 0/1] MR5777: win32u: Limit GDI object generation by 127 for a 64-bit build. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.