[PATCH] user32: Fail if an invalid window handle is passed to GetMessage().
Fixes a hang when closing Windows Media Player 10. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/user32/message.c | 8 +++++++- dlls/user32/tests/msg.c | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 860ceefc38d..b0ab5be902b 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -3836,6 +3836,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT HANDLE server_queue = get_server_queue_handle(); unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE; /* Always selected */ + if (hwnd && !IsWindow( hwnd )) + { + SetLastError( ERROR_INVALID_WINDOW_HANDLE ); + return -1; + } + USER_CheckNotLock(); check_for_driver_events( 0 ); @@ -3866,7 +3872,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT BOOL WINAPI DECLSPEC_HOTPATCH GetMessageA( MSG *msg, HWND hwnd, UINT first, UINT last ) { if (get_pending_wmchar( msg, first, last, TRUE )) return TRUE; - GetMessageW( msg, hwnd, first, last ); + if (GetMessageW( msg, hwnd, first, last ) == -1) return -1; map_wparam_WtoA( msg, TRUE ); return (msg->message != WM_QUIT); } diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 91d3fcc4846..aa5b89d15ec 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -17692,6 +17692,17 @@ static void test_SendMessage_pump(void) DestroyWindow(hwnd); } +static void test_invalid_window(void) +{ + MSG msg; + BOOL ret; + + SetLastError(0xdeadbeef); + ret = GetMessageA(&msg, (HWND)0xdeadbeef, 0, 0); + ok(ret == -1, "wrong ret %d\n", ret); + ok(GetLastError() == ERROR_INVALID_WINDOW_HANDLE, "wrong error %u\n", GetLastError()); +} + static void init_funcs(void) { HMODULE hKernel32 = GetModuleHandleA("kernel32.dll"); @@ -17817,6 +17828,7 @@ START_TEST(msg) test_notify_message(); test_SetActiveWindow(); test_restore_messages(); + test_invalid_window(); if (!pTrackMouseEvent) win_skip("TrackMouseEvent is not available\n"); -- 2.23.0
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=56418 Your paranoid android. === w7pro64 (64 bit report) === user32: msg.c:4292: Test failed: WM_MDICREATE for maximized visible MDI child window: 56: in msg 0x0047 expecting wParam 0x1012 got 0x101a msg.c:4292: Test failed: WM_MDICREATE for maximized visible MDI child window: 61: in msg 0x0047 expecting wParam 0x1012 got 0x101a msg.c:4309: Test failed: Destroy visible maximized MDI child window: 2: the msg 0x0046 was expected, but got msg 0x0009 instead msg.c:4342: Test failed: Maximize MDI child window with invisible parent: 31: in msg 0x0047 expecting wParam 0x33 got 0x3b Report errors: user32:msg prints too much data (55884 bytes) === debian10 (32 bit report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit French report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35222 bytes) === debian10 (32 bit Japanese:Japan report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35222 bytes) === debian10 (32 bit Chinese:China report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit WoW report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (64 bit WoW report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes)
Zebediah Figura <z.figura12(a)gmail.com> writes:
diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 860ceefc38d..b0ab5be902b 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -3836,6 +3836,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT HANDLE server_queue = get_server_queue_handle(); unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE; /* Always selected */
+ if (hwnd && !IsWindow( hwnd )) + { + SetLastError( ERROR_INVALID_WINDOW_HANDLE ); + return -1; + }
It needs to accept at least -1, and probably also 0xffff. It would also be interesting to check other functions like PeekMessage(), and maybe move the test to some inner function. -- Alexandre Julliard julliard(a)winehq.org
On 9/11/19 4:23 PM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 860ceefc38d..b0ab5be902b 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -3836,6 +3836,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT HANDLE server_queue = get_server_queue_handle(); unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE; /* Always selected */
+ if (hwnd && !IsWindow( hwnd )) + { + SetLastError( ERROR_INVALID_WINDOW_HANDLE ); + return -1; + }
It needs to accept at least -1, and probably also 0xffff.
It would also be interesting to check other functions like PeekMessage(), and maybe move the test to some inner function.
Thanks for the review; will do.
participants (3)
-
Alexandre Julliard -
Marvin -
Zebediah Figura