[PATCH v2 1/5] kernelbase: Check handle validity in unsafe_mem_from_HLOCAL.

[Rémi Bernon]

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/kernelbase/memory.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index 3cb596e5814..655778fe79f 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -598,9 +598,12 @@ struct mem_entry
  * the output jpeg's > 1 MB if not */
 #define HLOCAL_STORAGE      (sizeof(HLOCAL) * 2)
 
-static inline struct mem_entry *mem_from_HLOCAL( HLOCAL handle )
+static inline struct mem_entry *unsafe_mem_from_HLOCAL( HLOCAL handle )
 {
-    return (struct mem_entry *)((char *)handle - 2);
+    struct mem_entry *mem = CONTAINING_RECORD( handle, struct mem_entry, ptr );
+    if (!((ULONG_PTR)handle & 2)) return NULL;
+    if (mem->magic != MAGIC_LOCAL_USED) return NULL;
+    return mem;
 }
 
 static inline HLOCAL HLOCAL_from_mem( struct mem_entry *mem )
@@ -709,8 +712,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalFree( HLOCAL handle )
         }
         else  /* HANDLE */
         {
-            mem = mem_from_HLOCAL( handle );
-            if (mem->magic == MAGIC_LOCAL_USED)
+            if ((mem = unsafe_mem_from_HLOCAL( handle )))
             {
                 mem->magic = 0xdead;
                 if (mem->ptr)
@@ -745,6 +747,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalFree( HLOCAL handle )
  */
 LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
 {
+    struct mem_entry *mem;
     void *ret = NULL;
 
     TRACE_(globalmem)( "handle %p\n", handle );
@@ -767,8 +770,7 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
     RtlLockHeap( GetProcessHeap() );
     __TRY
     {
-        struct mem_entry *mem = mem_from_HLOCAL( handle );
-        if (mem->magic == MAGIC_LOCAL_USED)
+        if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
             ret = mem->ptr;
             if (!mem->ptr) SetLastError( ERROR_DISCARDED );
@@ -826,10 +828,9 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f
                 LocalFree( handle );
             }
         }
-        else if (!is_pointer( handle ) && (flags & LMEM_DISCARDABLE))
+        else if ((mem = unsafe_mem_from_HLOCAL( handle )) && (flags & LMEM_DISCARDABLE))
         {
             /* change the flags to make our block "discardable" */
-            mem = mem_from_HLOCAL( handle );
             mem->flags |= LMEM_DISCARDABLE >> 8;
             ret = handle;
         }
@@ -843,10 +844,9 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f
             if (!(flags & LMEM_MOVEABLE)) heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY;
             ret = HeapReAlloc( GetProcessHeap(), heap_flags, handle, size );
         }
-        else
+        else if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
             /* reallocate a moveable block */
-            mem = mem_from_HLOCAL( handle );
             if (size != 0)
             {
                 if (size <= INT_MAX - HLOCAL_STORAGE)
@@ -886,6 +886,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f
                 else WARN_(globalmem)( "not freeing memory associated with locked handle\n" );
             }
         }
+        else SetLastError( ERROR_INVALID_HANDLE );
     }
     RtlUnlockHeap( GetProcessHeap() );
     return ret;
@@ -897,6 +898,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f
  */
 BOOL WINAPI DECLSPEC_HOTPATCH LocalUnlock( HLOCAL handle )
 {
+    struct mem_entry *mem;
     BOOL ret = FALSE;
 
     TRACE_(globalmem)( "handle %p\n", handle );
@@ -910,8 +912,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH LocalUnlock( HLOCAL handle )
     RtlLockHeap( GetProcessHeap() );
     __TRY
     {
-        struct mem_entry *mem = mem_from_HLOCAL( handle );
-        if (mem->magic == MAGIC_LOCAL_USED)
+        if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
             if (mem->lock)
             {
-- 
2.35.1

[PATCH v2 2/5] kernelbase: Check pointer validity in unsafe_ptr_from_HLOCAL.

[Rémi Bernon]

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/kernelbase/memory.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index 655778fe79f..e81d38564e7 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -611,9 +611,10 @@ static inline HLOCAL HLOCAL_from_mem( struct mem_entry *mem )
     return &mem->ptr;
 }
 
-static inline BOOL is_pointer( HLOCAL handle )
+static inline void *unsafe_ptr_from_HLOCAL( HLOCAL handle )
 {
-    return !((ULONG_PTR)handle & 2);
+    if ((ULONG_PTR)handle & 2) return NULL;
+    return handle;
 }
 
 /***********************************************************************
@@ -695,6 +696,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalFree( HLOCAL handle )
 {
     struct mem_entry *mem;
     HLOCAL ret;
+    void *ptr;
 
     TRACE_(globalmem)( "handle %p\n", handle );
 
@@ -702,9 +704,9 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalFree( HLOCAL handle )
     __TRY
     {
         ret = 0;
-        if (is_pointer( handle )) /* POINTER */
+        if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
         {
-            if (!HeapFree( GetProcessHeap(), HEAP_NO_SERIALIZE, handle ))
+            if (!HeapFree( GetProcessHeap(), HEAP_NO_SERIALIZE, ptr ))
             {
                 SetLastError( ERROR_INVALID_HANDLE );
                 ret = handle;
@@ -752,11 +754,11 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
 
     TRACE_(globalmem)( "handle %p\n", handle );
 
-    if (is_pointer( handle ))
+    if ((ret = unsafe_ptr_from_HLOCAL( handle )))
     {
         __TRY
         {
-            volatile char *p = handle;
+            volatile char *p = ret;
             *p |= 0;
         }
         __EXCEPT_PAGE_FAULT
@@ -764,7 +766,7 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
             return NULL;
         }
         __ENDTRY
-        return handle;
+        return ret;
     }
 
     RtlLockHeap( GetProcessHeap() );
@@ -799,16 +801,16 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT flags )
 {
     struct mem_entry *mem;
-    void *ptr;
     HLOCAL ret = 0;
     DWORD heap_flags = (flags & LMEM_ZEROINIT) ? HEAP_ZERO_MEMORY : 0;
+    void *ptr;
 
     TRACE_(globalmem)( "handle %p, size %#Ix, flags %#x\n", handle, size, flags );
 
     RtlLockHeap( GetProcessHeap() );
     if (flags & LMEM_MODIFY) /* modify flags */
     {
-        if (is_pointer( handle ) && (flags & LMEM_MOVEABLE))
+        if (unsafe_ptr_from_HLOCAL( handle ) && (flags & LMEM_MOVEABLE))
         {
             /* make a fixed block moveable
              * actually only NT is able to do this. But it's soo simple
@@ -838,11 +840,11 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f
     }
     else
     {
-        if (is_pointer( handle ))
+        if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
         {
             /* reallocate fixed memory */
             if (!(flags & LMEM_MOVEABLE)) heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY;
-            ret = HeapReAlloc( GetProcessHeap(), heap_flags, handle, size );
+            ret = HeapReAlloc( GetProcessHeap(), heap_flags, ptr, size );
         }
         else if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
@@ -903,7 +905,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH LocalUnlock( HLOCAL handle )
 
     TRACE_(globalmem)( "handle %p\n", handle );
 
-    if (is_pointer( handle ))
+    if (unsafe_ptr_from_HLOCAL( handle ))
     {
         SetLastError( ERROR_NOT_LOCKED );
         return FALSE;
-- 
2.35.1

[PATCH v2 3/5] kernel32: Synchronize the HLOCAL helpers with kernelbase.

[Rémi Bernon]

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/kernel32/heap.c | 112 +++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 53 deletions(-)

diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c
index fefb88d6857..36353c82b90 100644
--- a/dlls/kernel32/heap.c
+++ b/dlls/kernel32/heap.c
@@ -140,43 +140,51 @@ BOOL WINAPI HeapDestroy( HANDLE heap /* [in] Handle of heap */ )
 }
 
 
-/*
- * Win32 Global heap functions (GlobalXXX).
- * These functions included in Win32 for compatibility with 16 bit Windows
- * Especially the moveable blocks and handles are oldish.
- * But the ability to directly allocate memory with GPTR and LPTR is widely
- * used.
- *
- * The handle stuff looks horrible, but it's implemented almost like Win95
- * does it.
- *
- */
-
-#define MAGIC_GLOBAL_USED 0x5342
-#define HANDLE_TO_INTERN(h)  ((PGLOBAL32_INTERN)(((char *)(h))-2))
-#define INTERN_TO_HANDLE(i)  (&((i)->Pointer))
-#define POINTER_TO_HANDLE(p) (*(((const HGLOBAL *)(p))-2))
-#define ISHANDLE(h)          (((ULONG_PTR)(h)&2)!=0)
-#define ISPOINTER(h)         (((ULONG_PTR)(h)&2)==0)
-/* align the storage needed for the HGLOBAL on an 8byte boundary thus
- * GlobalAlloc/GlobalReAlloc'ing with GMEM_MOVEABLE of memory with
- * size = 8*k, where k=1,2,3,... alloc's exactly the given size.
- * The Minolta DiMAGE Image Viewer heavily relies on this, corrupting
- * the output jpeg's > 1 MB if not */
-#define HGLOBAL_STORAGE      (sizeof(HGLOBAL)*2)
+/***********************************************************************
+ * Global/local heap functions, keep in sync with kernelbase/memory.c
+ ***********************************************************************/
 
 #include "pshpack1.h"
 
-typedef struct __GLOBAL32_INTERN
+struct mem_entry
 {
-   WORD         Magic;
-   LPVOID       Pointer;
-   BYTE         Flags;
-   BYTE         LockCount;
-} GLOBAL32_INTERN, *PGLOBAL32_INTERN;
+    WORD magic;
+    void *ptr;
+    BYTE flags;
+    BYTE lock;
+};
 
 #include "poppack.h"
 
+#define MAGIC_LOCAL_USED    0x5342
+#define POINTER_TO_HANDLE( p ) (*(((const HGLOBAL *)( p )) - 2))
+/* align the storage needed for the HLOCAL on an 8-byte boundary thus
+ * LocalAlloc/LocalReAlloc'ing with LMEM_MOVEABLE of memory with
+ * size = 8*k, where k=1,2,3,... allocs exactly the given size.
+ * The Minolta DiMAGE Image Viewer heavily relies on this, corrupting
+ * the output jpeg's > 1 MB if not */
+#define HLOCAL_STORAGE      (sizeof(HLOCAL) * 2)
+
+static inline struct mem_entry *unsafe_mem_from_HLOCAL( HLOCAL handle )
+{
+    struct mem_entry *mem = CONTAINING_RECORD( handle, struct mem_entry, ptr );
+    if (!((ULONG_PTR)handle & 2)) return NULL;
+    if (mem->magic != MAGIC_LOCAL_USED) return NULL;
+    return mem;
+}
+
+static inline HLOCAL HLOCAL_from_mem( struct mem_entry *mem )
+{
+    return &mem->ptr;
+}
+
+static inline void *unsafe_ptr_from_HLOCAL( HLOCAL handle )
+{
+    if ((ULONG_PTR)handle & 2) return NULL;
+    return handle;
+}
+
+
 /***********************************************************************
  *           GlobalLock   (KERNEL32.@)
  *
@@ -217,7 +225,7 @@ void *WINAPI GlobalLock( HGLOBAL handle )
  */
 BOOL WINAPI GlobalUnlock( HGLOBAL handle )
 {
-    if (ISPOINTER( handle )) return TRUE;
+    if (unsafe_ptr_from_HLOCAL( handle )) return TRUE;
     return LocalUnlock( handle );
 }
 
@@ -233,7 +241,7 @@ BOOL WINAPI GlobalUnlock( HGLOBAL handle )
  */
 HGLOBAL WINAPI GlobalHandle( const void *ptr )
 {
-    PGLOBAL32_INTERN mem;
+    struct mem_entry *mem;
     HGLOBAL handle;
     LPCVOID test;
 
@@ -253,7 +261,7 @@ HGLOBAL WINAPI GlobalHandle( const void *ptr )
         /* note that if ptr is a pointer to a block allocated by           */
         /* GlobalAlloc with GMEM_MOVEABLE then magic test in HeapValidate  */
         /* will fail.                                                      */
-        if (ISPOINTER( ptr ))
+        if ((ptr = unsafe_ptr_from_HLOCAL( (HLOCAL)ptr )))
         {
             if (HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, ptr ))
             {
@@ -265,11 +273,10 @@ HGLOBAL WINAPI GlobalHandle( const void *ptr )
         else handle = (HGLOBAL)ptr;
 
         /* Now test handle either passed in or retrieved from pointer */
-        mem = HANDLE_TO_INTERN( handle );
-        if (mem->Magic == MAGIC_GLOBAL_USED)
+        if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
-            test = mem->Pointer;
-            if (HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, (const char *)test - HGLOBAL_STORAGE ) && /* obj(-handle) valid arena? */
+            test = mem->ptr;
+            if (HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, (const char *)test - HLOCAL_STORAGE ) && /* obj(-handle) valid arena? */
                 HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, mem )) /* intern valid arena? */
                 break; /* valid moveable block */
         }
@@ -320,8 +327,9 @@ HGLOBAL WINAPI GlobalReAlloc( HGLOBAL handle, SIZE_T size, UINT flags )
  */
 SIZE_T WINAPI GlobalSize( HGLOBAL handle )
 {
-    PGLOBAL32_INTERN mem;
+    struct mem_entry *mem;
     SIZE_T retval;
+    void *ptr;
 
     TRACE_(globalmem)( "handle %p\n", handle );
 
@@ -331,27 +339,26 @@ SIZE_T WINAPI GlobalSize( HGLOBAL handle )
         return 0;
     }
 
-    if (ISPOINTER( handle ))
+    if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
     {
-        retval = HeapSize( GetProcessHeap(), 0, handle );
+        retval = HeapSize( GetProcessHeap(), 0, ptr );
         if (retval == ~(SIZE_T)0) /* It might be a GMEM_MOVEABLE data pointer */
         {
-            retval = HeapSize( GetProcessHeap(), 0, (char *)handle - HGLOBAL_STORAGE );
-            if (retval != ~(SIZE_T)0) retval -= HGLOBAL_STORAGE;
+            retval = HeapSize( GetProcessHeap(), 0, (char *)ptr - HLOCAL_STORAGE );
+            if (retval != ~(SIZE_T)0) retval -= HLOCAL_STORAGE;
         }
     }
     else
     {
         RtlLockHeap( GetProcessHeap() );
-        mem = HANDLE_TO_INTERN( handle );
-        if (mem->Magic == MAGIC_GLOBAL_USED)
+        if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
-            if (!mem->Pointer) /* handle case of GlobalAlloc( ??,0) */
+            if (!mem->ptr) /* handle case of GlobalAlloc( ??,0) */
                 retval = 0;
             else
             {
-                retval = HeapSize( GetProcessHeap(), 0, (char *)mem->Pointer - HGLOBAL_STORAGE );
-                if (retval != ~(SIZE_T)0) retval -= HGLOBAL_STORAGE;
+                retval = HeapSize( GetProcessHeap(), 0, (char *)mem->ptr - HLOCAL_STORAGE );
+                if (retval != ~(SIZE_T)0) retval -= HLOCAL_STORAGE;
             }
         }
         else
@@ -418,23 +425,22 @@ VOID WINAPI GlobalUnfix( HGLOBAL handle )
  */
 UINT WINAPI GlobalFlags( HGLOBAL handle )
 {
-    PGLOBAL32_INTERN mem;
+    struct mem_entry *mem;
     DWORD retval;
 
     TRACE_(globalmem)( "handle %p\n", handle );
 
-    if (ISPOINTER( handle ))
+    if (unsafe_ptr_from_HLOCAL( handle ))
     {
         retval = 0;
     }
     else
     {
         RtlLockHeap( GetProcessHeap() );
-        mem = HANDLE_TO_INTERN( handle );
-        if (mem->Magic == MAGIC_GLOBAL_USED)
+        if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
-            retval = mem->LockCount + (mem->Flags << 8);
-            if (mem->Pointer == 0) retval |= GMEM_DISCARDED;
+            retval = mem->lock + (mem->flags << 8);
+            if (mem->ptr == 0) retval |= GMEM_DISCARDED;
         }
         else
         {
-- 
2.35.1

[PATCH v2 4/5] kernelbase: Implement internal KernelBaseGetGlobalData.

[Rémi Bernon]

And call it from kernel32 to get access to kernelbase global data.

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/kernel32/heap.c            |  5 +++++
 dlls/kernel32/kernel_main.c     |  1 +
 dlls/kernel32/kernel_private.h  | 11 +++++++++++
 dlls/kernelbase/kernelbase.spec |  2 +-
 dlls/kernelbase/memory.c        | 20 ++++++++++++++++++++
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c
index 36353c82b90..cf8af2bd0c6 100644
--- a/dlls/kernel32/heap.c
+++ b/dlls/kernel32/heap.c
@@ -33,6 +33,9 @@
 #include "winerror.h"
 #include "winnt.h"
 #include "winternl.h"
+
+#include "kernel_private.h"
+
 #include "wine/exception.h"
 #include "wine/debug.h"
 
@@ -156,6 +159,8 @@ struct mem_entry
 
 #include "poppack.h"
 
+struct kernelbase_global_data *kernelbase_global_data;
+
 #define MAGIC_LOCAL_USED    0x5342
 #define POINTER_TO_HANDLE( p ) (*(((const HGLOBAL *)( p )) - 2))
 /* align the storage needed for the HLOCAL on an 8-byte boundary thus
diff --git a/dlls/kernel32/kernel_main.c b/dlls/kernel32/kernel_main.c
index 352a873933f..7b45db73257 100644
--- a/dlls/kernel32/kernel_main.c
+++ b/dlls/kernel32/kernel_main.c
@@ -123,6 +123,7 @@ static BOOL process_attach( HMODULE module )
     RtlSetUnhandledExceptionFilter( UnhandledExceptionFilter );
 
     NtQuerySystemInformation( SystemBasicInformation, &system_info, sizeof(system_info), NULL );
+    kernelbase_global_data = KernelBaseGetGlobalData();
 
     copy_startup_info();
 
diff --git a/dlls/kernel32/kernel_private.h b/dlls/kernel32/kernel_private.h
index 633511d6140..980a12b25a6 100644
--- a/dlls/kernel32/kernel_private.h
+++ b/dlls/kernel32/kernel_private.h
@@ -21,6 +21,17 @@
 #ifndef __WINE_KERNEL_PRIVATE_H
 #define __WINE_KERNEL_PRIVATE_H
 
+/* not compatible with windows */
+struct kernelbase_global_data
+{
+    struct mem_entry *mem_entries;
+    struct mem_entry *mem_entries_end;
+};
+
+void *WINAPI KernelBaseGetGlobalData(void);
+
+extern struct kernelbase_global_data *kernelbase_global_data;
+
 NTSTATUS WINAPI BaseGetNamedObjectDirectory( HANDLE *dir );
 
 static inline BOOL set_ntstatus( NTSTATUS status )
diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec
index a59d8581ff0..b5116ae9b5a 100644
--- a/dlls/kernelbase/kernelbase.spec
+++ b/dlls/kernelbase/kernelbase.spec
@@ -943,7 +943,7 @@
 @ stdcall K32InitializeProcessForWsWatch(long) InitializeProcessForWsWatch
 @ stdcall K32QueryWorkingSet(long ptr long) QueryWorkingSet
 @ stdcall K32QueryWorkingSetEx(long ptr long) QueryWorkingSetEx
-@ stub KernelBaseGetGlobalData
+@ stdcall KernelBaseGetGlobalData()
 @ stdcall LCIDToLocaleName(long ptr long long)
 @ stdcall LCMapStringA(long long str long ptr long)
 @ stdcall LCMapStringEx(wstr long wstr long ptr long ptr ptr long)
diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index e81d38564e7..6d42316760b 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -578,6 +578,13 @@ BOOL WINAPI DECLSPEC_HOTPATCH HeapWalk( HANDLE heap, PROCESS_HEAP_ENTRY *entry )
  * Global/local heap functions
  ***********************************************************************/
 
+/* not compatible with windows */
+struct kernelbase_global_data
+{
+    struct mem_entry *mem_entries;
+    struct mem_entry *mem_entries_end;
+};
+
 #include "pshpack1.h"
 
 struct mem_entry
@@ -590,6 +597,8 @@ struct mem_entry
 
 #include "poppack.h"
 
+static struct kernelbase_global_data kernelbase_global_data = {0};
+
 #define MAGIC_LOCAL_USED    0x5342
 /* align the storage needed for the HLOCAL on an 8-byte boundary thus
  * LocalAlloc/LocalReAlloc'ing with LMEM_MOVEABLE of memory with
@@ -617,6 +626,17 @@ static inline void *unsafe_ptr_from_HLOCAL( HLOCAL handle )
     return handle;
 }
 
+
+/***********************************************************************
+ *           KernelBaseGetGlobalData   (kernelbase.@)
+ */
+void *WINAPI KernelBaseGetGlobalData(void)
+{
+    WARN_(globalmem)( "semi-stub!\n" );
+    return &kernelbase_global_data;
+}
+
+
 /***********************************************************************
  *           GlobalAlloc   (kernelbase.@)
  */
-- 
2.35.1

[PATCH v2 4/5] kernelbase: Implement internal KernelBaseGetGlobalData.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=111657

Your paranoid android.


=== debian11 (32 bit report) ===

kernel32:
change.c:309: Test failed: should be ready
change.c:339: Test failed: should be ready

[PATCH v2 5/5] kernelbase: Allocate HLOCAL / HGLOBAL from a static handle table.

[Rémi Bernon]

Sharing the table pointers through KernelBaseGetGlobalData to check for
handle validity in kernel32, and as native does it.

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/kernel32/heap.c       |  20 +++++--
 dlls/kernel32/tests/heap.c |   9 ---
 dlls/kernelbase/memory.c   | 115 ++++++++++++++++++++-----------------
 3 files changed, 75 insertions(+), 69 deletions(-)

diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c
index cf8af2bd0c6..50b877111c3 100644
--- a/dlls/kernel32/heap.c
+++ b/dlls/kernel32/heap.c
@@ -151,10 +151,17 @@ BOOL WINAPI HeapDestroy( HANDLE heap /* [in] Handle of heap */ )
 
 struct mem_entry
 {
-    WORD magic;
-    void *ptr;
-    BYTE flags;
-    BYTE lock;
+    union
+    {
+        struct
+        {
+            WORD magic;
+            void *ptr;
+            BYTE flags;
+            BYTE lock;
+        };
+        void *next_free;
+    };
 };
 
 #include "poppack.h"
@@ -173,7 +180,9 @@ struct kernelbase_global_data *kernelbase_global_data;
 static inline struct mem_entry *unsafe_mem_from_HLOCAL( HLOCAL handle )
 {
     struct mem_entry *mem = CONTAINING_RECORD( handle, struct mem_entry, ptr );
+    struct kernelbase_global_data *data = kernelbase_global_data;
     if (!((ULONG_PTR)handle & 2)) return NULL;
+    if (mem < data->mem_entries || mem >= data->mem_entries_end) return NULL;
     if (mem->magic != MAGIC_LOCAL_USED) return NULL;
     return mem;
 }
@@ -281,8 +290,7 @@ HGLOBAL WINAPI GlobalHandle( const void *ptr )
         if ((mem = unsafe_mem_from_HLOCAL( handle )))
         {
             test = mem->ptr;
-            if (HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, (const char *)test - HLOCAL_STORAGE ) && /* obj(-handle) valid arena? */
-                HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, mem )) /* intern valid arena? */
+            if (HeapValidate( GetProcessHeap(), HEAP_NO_SERIALIZE, (const char *)test - HLOCAL_STORAGE )) /* obj(-handle) valid arena? */
                 break; /* valid moveable block */
         }
         handle = 0;
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index f6fb95cdddd..476be96cea8 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -290,15 +290,11 @@ static void test_GlobalAlloc(void)
 
     SetLastError( 0xdeadbeef );
     mem = GlobalAlloc( GMEM_MOVEABLE | GMEM_DISCARDABLE, 0 );
-    todo_wine
     ok( !mem, "GlobalAlloc succeeded\n" );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_ENOUGH_MEMORY, "got error %lu\n", GetLastError() );
     SetLastError( 0xdeadbeef );
     mem = LocalAlloc( LMEM_MOVEABLE | LMEM_DISCARDABLE, 0 );
-    todo_wine
     ok( !mem, "LocalAlloc succeeded\n" );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_ENOUGH_MEMORY, "got error %lu\n", GetLastError() );
 
     mem = GlobalAlloc( GMEM_DISCARDABLE, 0 );
@@ -417,7 +413,6 @@ static void test_GlobalAlloc(void)
         tmp_mem = GlobalFree( mem );
         ok( !tmp_mem, "GlobalFree failed, error %lu\n", GetLastError() );
         ok( !!entry->flags, "got unexpected flags %#Ix\n", entry->flags );
-        todo_wine_if(sizeof(void *) == 4)
         ok( !((UINT_PTR)entry->flags & sizeof(void *)), "got unexpected ptr align\n" );
         todo_wine_if(sizeof(void *) == 4)
         ok( !((UINT_PTR)entry->flags & (sizeof(void *) - 1)), "got unexpected ptr align\n" );
@@ -787,15 +782,11 @@ static void test_LocalAlloc(void)
 
     SetLastError( 0xdeadbeef );
     mem = LocalAlloc( LMEM_MOVEABLE | LMEM_DISCARDABLE, 0 );
-    todo_wine
     ok( !mem, "LocalAlloc succeeded\n" );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_ENOUGH_MEMORY, "got error %lu\n", GetLastError() );
     SetLastError( 0xdeadbeef );
     mem = GlobalAlloc( GMEM_MOVEABLE | GMEM_DISCARDABLE, 0 );
-    todo_wine
     ok( !mem, "GlobalAlloc succeeded\n" );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_ENOUGH_MEMORY, "got error %lu\n", GetLastError() );
 
     mem = LocalAlloc( LMEM_DISCARDABLE, 0 );
diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index 6d42316760b..5bdab31589d 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -589,15 +589,30 @@ struct kernelbase_global_data
 
 struct mem_entry
 {
-   WORD  magic;
-   void *ptr;
-   BYTE flags;
-   BYTE lock;
+    union
+    {
+        struct
+        {
+            WORD magic;
+            void *ptr;
+            BYTE flags;
+            BYTE lock;
+        };
+        void *next_free;
+    };
 };
 
 #include "poppack.h"
 
-static struct kernelbase_global_data kernelbase_global_data = {0};
+#define MAX_MEM_HANDLES  0x10000
+static struct mem_entry mem_entries[MAX_MEM_HANDLES];
+static struct mem_entry *next_free_mem = mem_entries;
+
+static struct kernelbase_global_data kernelbase_global_data =
+{
+    .mem_entries = mem_entries,
+    .mem_entries_end = mem_entries + MAX_MEM_HANDLES,
+};
 
 #define MAGIC_LOCAL_USED    0x5342
 /* align the storage needed for the HLOCAL on an 8-byte boundary thus
@@ -610,7 +625,9 @@ static struct kernelbase_global_data kernelbase_global_data = {0};
 static inline struct mem_entry *unsafe_mem_from_HLOCAL( HLOCAL handle )
 {
     struct mem_entry *mem = CONTAINING_RECORD( handle, struct mem_entry, ptr );
+    struct kernelbase_global_data *data = &kernelbase_global_data;
     if (!((ULONG_PTR)handle & 2)) return NULL;
+    if (mem < data->mem_entries || mem >= data->mem_entries_end) return NULL;
     if (mem->magic != MAGIC_LOCAL_USED) return NULL;
     return mem;
 }
@@ -666,8 +683,10 @@ HGLOBAL WINAPI DECLSPEC_HOTPATCH GlobalFree( HLOCAL handle )
  */
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size )
 {
+    HANDLE heap = GetProcessHeap();
     struct mem_entry *mem;
     DWORD heap_flags = 0;
+    HLOCAL handle;
     void *ptr;
 
     TRACE_(globalmem)( "flags %#x, size %#Ix\n", flags, size );
@@ -676,36 +695,44 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size )
 
     if (!(flags & LMEM_MOVEABLE)) /* pointer */
     {
-        ptr = HeapAlloc( GetProcessHeap(), heap_flags, size );
+        ptr = HeapAlloc( heap, heap_flags, size );
         TRACE_(globalmem)( "return %p\n", ptr );
         return ptr;
     }
 
-    if (size > INT_MAX - HLOCAL_STORAGE)
+    RtlLockHeap( heap );
+    if ((mem = next_free_mem) < mem_entries || mem >= mem_entries + MAX_MEM_HANDLES)
+        mem = NULL;
+    else
     {
-        SetLastError( ERROR_OUTOFMEMORY );
-        return 0;
+        if (!mem->next_free) next_free_mem++;
+        else next_free_mem = mem->next_free;
+        mem->next_free = NULL;
     }
-    if (!(mem = HeapAlloc( GetProcessHeap(), 0, sizeof(*mem) ))) return 0;
+    RtlUnlockHeap( heap );
+
+    if (!mem) goto failed;
+    handle = HLOCAL_from_mem( mem );
 
     mem->magic = MAGIC_LOCAL_USED;
     mem->flags = flags >> 8;
     mem->lock  = 0;
+    mem->ptr   = NULL;
 
     if (size)
     {
-        if (!(ptr = HeapAlloc(GetProcessHeap(), heap_flags, size + HLOCAL_STORAGE )))
-        {
-            HeapFree( GetProcessHeap(), 0, mem );
-            return 0;
-        }
-        *(HLOCAL *)ptr = HLOCAL_from_mem( mem );
+        if (!(ptr = HeapAlloc( heap, heap_flags, size + HLOCAL_STORAGE ))) goto failed;
+        *(HLOCAL *)ptr = handle;
         mem->ptr = (char *)ptr + HLOCAL_STORAGE;
     }
-    else mem->ptr = NULL;
 
-    TRACE_(globalmem)( "return handle %p, ptr %p\n", HLOCAL_from_mem( mem ), mem->ptr );
-    return HLOCAL_from_mem( mem );
+    TRACE_(globalmem)( "return handle %p, ptr %p\n", handle, mem->ptr );
+    return handle;
+
+failed:
+    if (mem) LocalFree( handle );
+    SetLastError( ERROR_NOT_ENOUGH_MEMORY );
+    return 0;
 }
 
 
@@ -714,52 +741,32 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size )
  */
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalFree( HLOCAL handle )
 {
+    HANDLE heap = GetProcessHeap();
     struct mem_entry *mem;
-    HLOCAL ret;
+    HLOCAL ret = handle;
     void *ptr;
 
     TRACE_(globalmem)( "handle %p\n", handle );
 
-    RtlLockHeap( GetProcessHeap() );
-    __TRY
+    RtlLockHeap( heap );
+    if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
     {
-        ret = 0;
-        if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
-        {
-            if (!HeapFree( GetProcessHeap(), HEAP_NO_SERIALIZE, ptr ))
-            {
-                SetLastError( ERROR_INVALID_HANDLE );
-                ret = handle;
-            }
-        }
-        else  /* HANDLE */
-        {
-            if ((mem = unsafe_mem_from_HLOCAL( handle )))
-            {
-                mem->magic = 0xdead;
-                if (mem->ptr)
-                {
-                    if (!HeapFree( GetProcessHeap(), HEAP_NO_SERIALIZE, (char *)mem->ptr - HLOCAL_STORAGE ))
-                        ret = handle;
-                }
-                if (!HeapFree( GetProcessHeap(), HEAP_NO_SERIALIZE, mem )) ret = handle;
-            }
-            else
-            {
-                WARN_(globalmem)( "invalid handle %p\n", handle );
-                SetLastError( ERROR_INVALID_HANDLE );
-                ret = handle;
-            }
-        }
+        if (HeapFree( heap, HEAP_NO_SERIALIZE, ptr )) ret = 0;
     }
-    __EXCEPT_PAGE_FAULT
+    else if ((mem = unsafe_mem_from_HLOCAL( handle )))
+    {
+        if (!mem->ptr || HeapFree( heap, HEAP_NO_SERIALIZE, (char *)mem->ptr - HLOCAL_STORAGE )) ret = 0;
+        mem->ptr = NULL;
+        mem->next_free = next_free_mem;
+        next_free_mem = mem;
+    }
+    RtlUnlockHeap( heap );
+
+    if (ret)
     {
         WARN_(globalmem)( "invalid handle %p\n", handle );
         SetLastError( ERROR_INVALID_HANDLE );
-        ret = handle;
     }
-    __ENDTRY
-    RtlUnlockHeap( GetProcessHeap() );
     return ret;
 }
 
-- 
2.35.1