From: Fabian Maurer <dark.shadow4@web.de>
Subject: [PATCH] WIP/krnl386: When freeing task, also reset internal variable
Message-Id: <20220117212633.247016-1-dark.shadow4@web.de>
Date: Mon, 17 Jan 2022 22:26:33 +0100

This is a WIP patch fixing crashes in the win16 program Nota Bene.
The crash is caused by alloc_win16_thunk calling GlobalAlloc16,
GlobalLock16 and keeping the pinned address as "thunk_array".

Later we have the following chain:
TASK_DeleteTask -> GlobalFreeAll16 -> GlobalFree16
This deletes the buffer behind "thunk_array", but since it's not NULL
wine will continue to use it, causing funny heap corruption issues.

This patch fixes the issue and seems logically correct to me,
though it's very ugly architecturally.
Any hints on how we should handle this case properly?

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=12732
---
 dlls/krnl386.exe16/krnl386.exe16.spec | 2 ++
 dlls/krnl386.exe16/task.c             | 9 +++++++++
 dlls/user.exe16/message.c             | 6 ++++++
 include/wine/winbase16.h              | 3 +++
 4 files changed, 20 insertions(+)

diff --git a/dlls/krnl386.exe16/krnl386.exe16.spec b/dlls/krnl386.exe16/krnl386.exe16.spec
index d98e136c77e..a1e7db46c0f 100644
--- a/dlls/krnl386.exe16/krnl386.exe16.spec
+++ b/dlls/krnl386.exe16/krnl386.exe16.spec
@@ -751,3 +751,5 @@
 # Snoop support
 2001 pascal -register __wine_snoop_entry()
 2002 pascal -register __wine_snoop_return()
+
+@ cdecl -arch=win32 __wine_set_taskkill_func(ptr)
diff --git a/dlls/krnl386.exe16/task.c b/dlls/krnl386.exe16/task.c
index 30a40328c86..140b9190666 100644
--- a/dlls/krnl386.exe16/task.c
+++ b/dlls/krnl386.exe16/task.c
@@ -351,6 +351,12 @@ static TDB *TASK_Create( NE_MODULE *pModule, UINT16 cmdShow, LPCSTR cmdline, BYT
     return pTask;
 }

+static wine_taskkill_func taskkill;
+
+void __wine_set_taskkill_func(wine_taskkill_func func)
+{
+    taskkill = func;
+}

 /***********************************************************************
  *           TASK_DeleteTask
@@ -382,6 +388,9 @@ static void TASK_DeleteTask( HTASK16 hTask )
     /* the environment block and the thunk segments). */

     GlobalFreeAll16( hPDB );
+
+    if (taskkill)
+        taskkill();
 }


diff --git a/dlls/user.exe16/message.c b/dlls/user.exe16/message.c
index b79b0cb7751..f5cadc5e387 100644
--- a/dlls/user.exe16/message.c
+++ b/dlls/user.exe16/message.c
@@ -144,6 +144,11 @@ static int winproc_to_index( WNDPROC16 handle )
     return index;
 }

+static void taskkill(void)
+{
+    thunk_array = 0;
+}
+
 /* allocate a 16-bit thunk for an existing window proc */
 static WNDPROC16 alloc_win16_thunk( WNDPROC handle )
 {
@@ -156,6 +161,7 @@ static WNDPROC16 alloc_win16_thunk( WNDPROC handle )

     if (!thunk_array)  /* allocate the array and its selector */
     {
+        __wine_set_taskkill_func(taskkill);
         assert( MAX_WINPROCS16 * sizeof(WINPROC_THUNK) <= 0x10000 );

         if (!(thunk_selector = GlobalAlloc16( GMEM_FIXED | GMEM_ZEROINIT,
diff --git a/include/wine/winbase16.h b/include/wine/winbase16.h
index 1d052130e33..338a2af4e9e 100644
--- a/include/wine/winbase16.h
+++ b/include/wine/winbase16.h
@@ -567,4 +567,7 @@ BOOL16      WINAPI WriteProfileSection16(LPCSTR,LPCSTR);
 #define CURRENT_SP      (((WORD *)NtCurrentTeb()->SystemReserved1)[0])
 #define CURRENT_SS      (((WORD *)NtCurrentTeb()->SystemReserved1)[1])

+typedef void (*wine_taskkill_func)(void);
+extern void __wine_set_taskkill_func(wine_taskkill_func func);
+
 #endif /* __WINE_WINE_WINBASE16_H */
--
2.34.1