From: Zhiyi Zhang zzhang@codeweavers.com
Before this patch, WM_COPYDATA message data was packed at the end of the message and then passed to KeUserModeCallback() to call user mode callbacks. However, WM_COPYDATA can contain a large amount of data so KeUserModeCallback() can cause stack overflows. Manual tests on Windows show that WM_COPYDATA messages with data <= 2048 bytes do have their data packed at the end of the message. But for WM_COPYDATA messages with data > 2048 bytes, KiUserCallbackDispatcher() gets a 120 bytes packed arguments, which is the same size when packing WM_COPYDATA messages with no data. So for WM_COPYDATA messages with data > 2048 bytes, the data must be packed differently.
Fix FCSExpress hangs after sending WM_COPYDATA messages. --- dlls/user32/winproc.c | 5 ++- dlls/win32u/hook.c | 13 ++++--- dlls/win32u/message.c | 66 ++++++++++++++++++++++++++++++++---- dlls/win32u/win32u_private.h | 2 +- 4 files changed, 73 insertions(+), 13 deletions(-)
diff --git a/dlls/user32/winproc.c b/dlls/user32/winproc.c index 33b4c6f884f..be16c9e3bd4 100644 --- a/dlls/user32/winproc.c +++ b/dlls/user32/winproc.c @@ -771,7 +771,10 @@ void unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lparam, vo case WM_COPYDATA: { COPYDATASTRUCT *cds = buffer; - if (cds->lpData) cds->lpData = cds + 1; + /* If cbData <= 2048 bytes, the data is packed at the end of message buffer. Otherwise, + * cds->lpData points to an extra user buffer. See pack_user_message() for WM_COPYDATA */ + if (cds->lpData && cds->cbData <= 2048) + cds->lpData = cds + 1; break; } case EM_GETSEL: diff --git a/dlls/win32u/hook.c b/dlls/win32u/hook.c index d47aed4c6d6..1acaabe35ab 100644 --- a/dlls/win32u/hook.c +++ b/dlls/win32u/hook.c @@ -244,8 +244,9 @@ static LRESULT call_hook( struct win_hook_params *info, const WCHAR *module, siz HHOOK prev = thread_info->hook; BOOL prev_unicode = thread_info->hook_unicode; struct win_hook_params *params = info; + void *ret_ptr, *extra_buffer = NULL; + SIZE_T extra_buffer_size = 0; size_t reply_size; - void *ret_ptr; ULONG ret_len;
size = FIELD_OFFSET( struct win_hook_params, module[module ? lstrlenW( module ) + 1 : 1] ); @@ -295,21 +296,21 @@ static LRESULT call_hook( struct win_hook_params *info, const WCHAR *module, siz CBT_CREATEWNDW *cbtc = (CBT_CREATEWNDW *)params->lparam; LPARAM lp = (LPARAM)cbtc->lpcs; pack_user_message( (char *)params + message_offset, message_size, - WM_CREATE, 0, lp, FALSE ); + WM_CREATE, 0, lp, FALSE, &extra_buffer ); } break; case WH_CALLWNDPROC: { CWPSTRUCT *cwp = (CWPSTRUCT *)((char *)params + lparam_offset); pack_user_message( (char *)params + message_offset, message_size, - cwp->message, cwp->wParam, cwp->lParam, ansi ); + cwp->message, cwp->wParam, cwp->lParam, ansi, &extra_buffer ); } break; case WH_CALLWNDPROCRET: { CWPRETSTRUCT *cwpret = (CWPRETSTRUCT *)((char *)params + lparam_offset); pack_user_message( (char *)params + message_offset, message_size, - cwpret->message, cwpret->wParam, cwpret->lParam, ansi ); + cwpret->message, cwpret->wParam, cwpret->lParam, ansi, &extra_buffer ); } break; } @@ -323,6 +324,8 @@ static LRESULT call_hook( struct win_hook_params *info, const WCHAR *module, siz { WARN("Too many hooks called recursively, skipping call.\n"); if (params != info) free( params ); + if (extra_buffer) + NtFreeVirtualMemory( GetCurrentProcess(), &extra_buffer, &extra_buffer_size, MEM_RELEASE ); return 0; }
@@ -346,6 +349,8 @@ static LRESULT call_hook( struct win_hook_params *info, const WCHAR *module, siz thread_info->hook_call_depth--;
if (params != info) free( params ); + if (extra_buffer) + NtFreeVirtualMemory( GetCurrentProcess(), &extra_buffer, &extra_buffer_size, MEM_RELEASE ); }
return ret; diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 2f1e14c82c8..0f075a87675 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -1688,7 +1688,11 @@ size_t user_message_size( HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam, case WM_COPYDATA: { const COPYDATASTRUCT *cds = lparam_ptr; - size = sizeof(*cds) + cds->cbData; + /* If cbData <= 2048 bytes, pack the data at the end of the message. Otherwise, pack the data + * in an extra user buffer to avoid potential stack overflows when calling KeUserModeCallback() + * because cbData can be very large. Manual tests of KiUserCallbackDispatcher() when receiving + * WM_COPYDATA messages show that Windows does similar things */ + size = cds->cbData <= 2048 ? sizeof(*cds) + cds->cbData : sizeof(*cds); break; } case WM_HELP: @@ -1777,15 +1781,20 @@ size_t user_message_size( HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam, * pack_user_message * * Copy message to a buffer for passing to client. + * + * ret_extra_buffer returns an extra user buffer allocated to store large message data, for example, + * for WM_COPYDATA. Call NtFreeVirtualMemory() for *ret_extra_buffer when done using it if it's not NULL. */ void pack_user_message( void *buffer, size_t size, UINT message, - WPARAM wparam, LPARAM lparam, BOOL ansi ) + WPARAM wparam, LPARAM lparam, BOOL ansi, void **ret_extra_buffer ) { const void *lparam_ptr = (const void *)lparam; void const *inline_ptr = (void *)0xffffffff;
if (!size) return;
+ *ret_extra_buffer = NULL; + switch (message) { case WM_NCCREATE: @@ -1822,9 +1831,49 @@ void pack_user_message( void *buffer, size_t size, UINT message, case WM_COPYDATA: { const COPYDATASTRUCT *cds = lparam_ptr; - if (cds->lpData && cds->cbData) - memcpy( (char *)buffer + sizeof(*cds), cds->lpData, cds->cbData ); - size = sizeof(*cds); + + if (!cds->lpData) + { + size = sizeof(*cds); + } + else + { + /* If cbData <= 2048 bytes, pack the data at the end of the message. Otherwise, pack the data + * in an extra user buffer to avoid potential stack overflow when calling KeUserModeCallback() + * because cbData can be very large. Manual tests of KiUserCallbackDispatcher() when receiving + * WM_COPYDATA messages show that Windows does similar things */ + if (cds->cbData <= 2048) + { + memcpy( (char *)buffer + sizeof(*cds), cds->lpData, cds->cbData ); + size = sizeof(*cds); + } + else + { + COPYDATASTRUCT *tmp_cds = buffer; + SIZE_T extra_buffer_size; + unsigned int status; + + memcpy( tmp_cds, cds, sizeof(*cds) ); + + extra_buffer_size = cds->cbData; + status = NtAllocateVirtualMemory( GetCurrentProcess(), ret_extra_buffer, 0, + &extra_buffer_size, MEM_RESERVE | MEM_COMMIT, + PAGE_READWRITE ); + if (!status) + { + memcpy( *ret_extra_buffer, cds->lpData, cds->cbData ); + tmp_cds->lpData = *ret_extra_buffer; + return; + } + else + { + ERR( "Failed to allocate memory for packing WM_COPYDATA, status %#x.\n", status); + tmp_cds->lpData = NULL; + tmp_cds->cbData = 0; + return; + } + } + } break; } case EM_GETSEL: @@ -2216,10 +2265,11 @@ static LRESULT call_window_proc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lpar struct win_proc_params p, *params = &p; BOOL ansi = ansi_dst && type == MSG_ASCII; size_t packed_size = 0, offset = sizeof(*params), reply_size; + void *ret_ptr, *extra_buffer = NULL; + SIZE_T extra_buffer_size = 0; LRESULT result = 0; CWPSTRUCT cwp; CWPRETSTRUCT cwpret; - void *ret_ptr; size_t ret_len = 0;
if (msg & 0x80000000) @@ -2251,10 +2301,12 @@ static LRESULT call_window_proc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lpar
if (type == MSG_OTHER_PROCESS) params->ansi = FALSE; if (packed_size) - pack_user_message( (char *)params + offset, packed_size, msg, wparam, lparam, ansi ); + pack_user_message( (char *)params + offset, packed_size, msg, wparam, lparam, ansi, &extra_buffer );
result = dispatch_win_proc_params( params, offset + packed_size, &ret_ptr, &ret_len ); if (params != &p) free( params ); + if (extra_buffer) + NtFreeVirtualMemory( GetCurrentProcess(), &extra_buffer, &extra_buffer_size, MEM_RELEASE );
copy_user_result( ret_ptr, min( ret_len, reply_size ), result, msg, wparam, lparam, ansi );
diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index f1e286b74e7..6f0cfca5724 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -145,7 +145,7 @@ extern LRESULT send_message_timeout( HWND hwnd, UINT msg, WPARAM wparam, LPARAM extern size_t user_message_size( HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam, BOOL other_process, BOOL ansi, size_t *reply_size ); extern void pack_user_message( void *buffer, size_t size, UINT message, - WPARAM wparam, LPARAM lparam, BOOL ansi ); + WPARAM wparam, LPARAM lparam, BOOL ansi, void **extra_buffer );
/* rawinput.c */ extern BOOL process_rawinput_message( MSG *msg, UINT hw_id, const struct hardware_msg_data *msg_data );