[PATCH 0/5] MR204: win32u: Avoid calling PE code with user lock
From: Jacek Caban <jacek(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53082 Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- dlls/win32u/class.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dlls/win32u/class.c b/dlls/win32u/class.c index 1a6c92edc3a..320a1509a53 100644 --- a/dlls/win32u/class.c +++ b/dlls/win32u/class.c @@ -358,6 +358,7 @@ ATOM WINAPI NtUserRegisterClassExWOW( const WNDCLASSEXW *wc, UNICODE_STRING *nam { const BOOL is_builtin = fnid, ansi = flags; HINSTANCE instance; + HICON sm_icon = 0; CLASS *class; ATOM atom; BOOL ret; @@ -427,6 +428,11 @@ ATOM WINAPI NtUserRegisterClassExWOW( const WNDCLASSEXW *wc, UNICODE_STRING *nam } /* Other non-null values must be set by caller */ + if (wc->hIcon && !wc->hIconSm) + sm_icon = CopyImage( wc->hIcon, IMAGE_ICON, + get_system_metrics( SM_CXSMICON ), + get_system_metrics( SM_CYSMICON ), + LR_COPYFROMRESOURCE ); user_lock(); if (class->local) list_add_head( &class_list, &class->entry ); @@ -440,15 +446,11 @@ ATOM WINAPI NtUserRegisterClassExWOW( const WNDCLASSEXW *wc, UNICODE_STRING *nam class->hIcon = wc->hIcon; class->hIconSm = wc->hIconSm; + class->hIconSmIntern = sm_icon; class->hCursor = wc->hCursor; class->hbrBackground = wc->hbrBackground; class->winproc = alloc_winproc( wc->lpfnWndProc, ansi ); if (client_menu_name) class->menu_name = *client_menu_name; - if (wc->hIcon && !wc->hIconSm) - class->hIconSmIntern = CopyImage( wc->hIcon, IMAGE_ICON, - get_system_metrics( SM_CXSMICON ), - get_system_metrics( SM_CYSMICON ), - LR_COPYFROMRESOURCE ); release_class_ptr( class ); return atom; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/204
From: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- dlls/win32u/class.c | 62 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/dlls/win32u/class.c b/dlls/win32u/class.c index 320a1509a53..5af9e586e01 100644 --- a/dlls/win32u/class.c +++ b/dlls/win32u/class.c @@ -647,6 +647,7 @@ static BOOL set_server_info( HWND hwnd, INT offset, LONG_PTR newval, UINT size ) static ULONG_PTR set_class_long( HWND hwnd, INT offset, LONG_PTR newval, UINT size, BOOL ansi ) { ULONG_PTR retval = 0; + HICON small_icon = 0; CLASS *class; if (!(class = get_class_ptr( hwnd, TRUE ))) return 0; @@ -696,31 +697,62 @@ static ULONG_PTR set_class_long( HWND hwnd, INT offset, LONG_PTR newval, UINT si break; case GCLP_HICON: retval = (ULONG_PTR)class->hIcon; - if (class->hIconSmIntern) + if (retval == newval) break; + if (newval && !class->hIconSm) { - NtUserDestroyCursor( class->hIconSmIntern, 0 ); - class->hIconSmIntern = NULL; + release_class_ptr( class ); + + small_icon = CopyImage( (HICON)newval, IMAGE_ICON, + get_system_metrics( SM_CXSMICON ), + get_system_metrics( SM_CYSMICON ), + LR_COPYFROMRESOURCE ); + + if (!(class = get_class_ptr( hwnd, TRUE ))) + { + NtUserDestroyCursor( small_icon, 0 ); + return 0; + } + if (retval != HandleToUlong( class->hIcon ) || class->hIconSm) + { + /* someone beat us, restart */ + release_class_ptr( class ); + NtUserDestroyCursor( small_icon, 0 ); + return set_class_long( hwnd, offset, newval, size, ansi ); + } } - if (newval && !class->hIconSm) - class->hIconSmIntern = CopyImage( (HICON)newval, IMAGE_ICON, - get_system_metrics( SM_CXSMICON ), - get_system_metrics( SM_CYSMICON ), - LR_COPYFROMRESOURCE ); + if (class->hIconSmIntern) NtUserDestroyCursor( class->hIconSmIntern, 0 ); class->hIcon = (HICON)newval; + class->hIconSmIntern = small_icon; break; case GCLP_HICONSM: retval = (ULONG_PTR)class->hIconSm; + if (retval == newval) break; if (retval && !newval && class->hIcon) - class->hIconSmIntern = CopyImage( class->hIcon, IMAGE_ICON, - get_system_metrics( SM_CXSMICON ), - get_system_metrics( SM_CYSMICON ), - LR_COPYFROMRESOURCE ); - else if (newval && class->hIconSmIntern) { - NtUserDestroyCursor( class->hIconSmIntern, 0 ); - class->hIconSmIntern = NULL; + HICON icon = class->hIcon; + release_class_ptr( class ); + + small_icon = CopyImage( icon, IMAGE_ICON, + get_system_metrics( SM_CXSMICON ), + get_system_metrics( SM_CYSMICON ), + LR_COPYFROMRESOURCE ); + + if (!(class = get_class_ptr( hwnd, TRUE ))) + { + NtUserDestroyCursor( small_icon, 0 ); + return 0; + } + if (class->hIcon != icon || !class->hIconSm) + { + /* someone beat us, restart */ + release_class_ptr( class ); + NtUserDestroyCursor( small_icon, 0 ); + return set_class_long( hwnd, offset, newval, size, ansi ); + } } + if (class->hIconSmIntern) NtUserDestroyCursor( class->hIconSmIntern, 0 ); class->hIconSm = (HICON)newval; + class->hIconSmIntern = small_icon; break; case GCL_STYLE: if (!set_server_info( hwnd, offset, newval, size )) break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/204
From: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- dlls/user32/painting.c | 43 ----------------------------------------- dlls/user32/user32.spec | 2 +- dlls/win32u/dce.c | 17 ++++++++++++++++ dlls/win32u/syscall.c | 1 + dlls/win32u/win32u.spec | 2 +- dlls/wow64win/syscall.h | 1 + dlls/wow64win/user.c | 7 +++++++ include/ntuser.h | 1 + 8 files changed, 29 insertions(+), 45 deletions(-) diff --git a/dlls/user32/painting.c b/dlls/user32/painting.c index 29450e6e984..e7be362773a 100644 --- a/dlls/user32/painting.c +++ b/dlls/user32/painting.c @@ -112,49 +112,6 @@ HDC WINAPI GetWindowDC( HWND hwnd ) } -/*********************************************************************** - * LockWindowUpdate (USER32.@) - * - * Enables or disables painting in the chosen window. - * - * PARAMS - * hwnd [I] handle to a window. - * - * RETURNS - * If successful, returns nonzero value. Otherwise, - * returns 0. - * - * NOTES - * You can lock only one window at a time. - */ -BOOL WINAPI LockWindowUpdate( HWND hwnd ) -{ - static HWND lockedWnd; - - FIXME("(%p), partial stub!\n",hwnd); - - USER_Lock(); - if (lockedWnd) - { - if (!hwnd) - { - /* Unlock lockedWnd */ - /* FIXME: Do something */ - } - else - { - /* Attempted to lock a second window */ - /* Return FALSE and do nothing */ - USER_Unlock(); - return FALSE; - } - } - lockedWnd = hwnd; - USER_Unlock(); - return TRUE; -} - - /*********************************************************************** * UpdateWindow (USER32.@) */ diff --git a/dlls/user32/user32.spec b/dlls/user32/user32.spec index c153b6dbfe3..ee533305a6e 100644 --- a/dlls/user32/user32.spec +++ b/dlls/user32/user32.spec @@ -510,7 +510,7 @@ @ stdcall LoadStringW(long long ptr long) @ stdcall LockSetForegroundWindow (long) @ stub LockWindowStation -@ stdcall LockWindowUpdate(long) +@ stdcall LockWindowUpdate(long) NtUserLockWindowUpdate @ stdcall LockWorkStation() @ stdcall LogicalToPhysicalPoint(long ptr) @ stdcall LogicalToPhysicalPointForPerMonitorDPI(long ptr) diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index 62d84d7e77b..acbb213d7b7 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -1574,3 +1574,20 @@ INT WINAPI NtUserExcludeUpdateRgn( HDC hdc, HWND hwnd ) NtGdiDeleteObjectApp( update_rgn ); return ret; } + +/*********************************************************************** + * NtUserLockWindowUpdate (win32u.@) + */ +BOOL WINAPI NtUserLockWindowUpdate( HWND hwnd ) +{ + static HWND locked_hwnd; + + FIXME( "(%p), partial stub!\n", hwnd ); + + if (!hwnd) + { + locked_hwnd = NULL; + return TRUE; + } + return !InterlockedCompareExchangePointer( (void **)&locked_hwnd, hwnd, 0 ); +} diff --git a/dlls/win32u/syscall.c b/dlls/win32u/syscall.c index a5ad95cd4b6..ac13580308d 100644 --- a/dlls/win32u/syscall.c +++ b/dlls/win32u/syscall.c @@ -150,6 +150,7 @@ static void * const syscalls[] = NtUserInitializeClientPfnArrays, NtUserInternalGetWindowText, NtUserKillTimer, + NtUserLockWindowUpdate, NtUserNotifyWinEvent, NtUserOpenDesktop, NtUserOpenInputDesktop, diff --git a/dlls/win32u/win32u.spec b/dlls/win32u/win32u.spec index 48c10fd45d3..17be6894508 100644 --- a/dlls/win32u/win32u.spec +++ b/dlls/win32u/win32u.spec @@ -1065,7 +1065,7 @@ @ stub NtUserLoadKeyboardLayoutEx @ stub NtUserLockCursor @ stub NtUserLockWindowStation -@ stub NtUserLockWindowUpdate +@ stdcall -syscall NtUserLockWindowUpdate(long) @ stub NtUserLockWorkStation @ stub NtUserLogicalToPerMonitorDPIPhysicalPoint @ stub NtUserLogicalToPhysicalDpiPointForWindow diff --git a/dlls/wow64win/syscall.h b/dlls/wow64win/syscall.h index fde87498131..e30d367cda7 100644 --- a/dlls/wow64win/syscall.h +++ b/dlls/wow64win/syscall.h @@ -137,6 +137,7 @@ SYSCALL_ENTRY( NtUserInitializeClientPfnArrays ) \ SYSCALL_ENTRY( NtUserInternalGetWindowText ) \ SYSCALL_ENTRY( NtUserKillTimer ) \ + SYSCALL_ENTRY( NtUserLockWindowUpdate ) \ SYSCALL_ENTRY( NtUserNotifyWinEvent ) \ SYSCALL_ENTRY( NtUserOpenDesktop ) \ SYSCALL_ENTRY( NtUserOpenInputDesktop ) \ diff --git a/dlls/wow64win/user.c b/dlls/wow64win/user.c index e828dae5247..c2d134626f1 100644 --- a/dlls/wow64win/user.c +++ b/dlls/wow64win/user.c @@ -828,3 +828,10 @@ NTSTATUS WINAPI wow64_NtUserRemoveMenu( UINT *args ) return NtUserRemoveMenu( handle, id, flags ); } + +NTSTATUS WINAPI wow64_NtUserLockWindowUpdate( UINT *args ) +{ + HWND hwnd = get_handle( &args ); + + return NtUserLockWindowUpdate( hwnd ); +} diff --git a/include/ntuser.h b/include/ntuser.h index 091d34c3a5f..e0c65a9f532 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -588,6 +588,7 @@ NTSTATUS WINAPI NtUserInitializeClientPfnArrays( const struct user_client_procs INT WINAPI NtUserInternalGetWindowText( HWND hwnd, WCHAR *text, INT count ); BOOL WINAPI NtUserIsClipboardFormatAvailable( UINT format ); BOOL WINAPI NtUserKillTimer( HWND hwnd, UINT_PTR id ); +BOOL WINAPI NtUserLockWindowUpdate( HWND hwnd ); UINT WINAPI NtUserMapVirtualKeyEx( UINT code, UINT type, HKL layout ); LRESULT WINAPI NtUserMessageCall( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, void *result_info, DWORD type, BOOL ansi ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/204
From: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- dlls/user32/hook.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/dlls/user32/hook.c b/dlls/user32/hook.c index 357a7fdb25a..7dddfb02638 100644 --- a/dlls/user32/hook.c +++ b/dlls/user32/hook.c @@ -100,6 +100,16 @@ struct hook_info WCHAR module[MAX_PATH]; }; +static CRITICAL_SECTION api_hook_cs; +static CRITICAL_SECTION_DEBUG critsect_debug = +{ + 0, 0, &api_hook_cs, + { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": api_hook_cs") } +}; +static CRITICAL_SECTION api_hook_cs = { &critsect_debug, -1, 0, 0, 0, 0 }; + + #define WH_WINEVENT (WH_MAXHOOK+1) static const char * const hook_names[WH_WINEVENT - WH_MINHOOK + 1] = @@ -487,12 +497,12 @@ BOOL WINAPI RegisterUserApiHook(const struct user_api_hook *new_hook, struct use if (!new_hook) return FALSE; - USER_Lock(); + EnterCriticalSection( &api_hook_cs ); hooked_user_api = *new_hook; user_api = &hooked_user_api; if (old_hook) *old_hook = original_user_api; - USER_Unlock(); + LeaveCriticalSection( &api_hook_cs ); return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/204
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=116428 Your paranoid android. === debian11 (32 bit Chinese:China report) === user32: win.c:11787: Test failed: 011D00CE: expected NOT topmost win.c:11606: Test succeeded inside todo block: 5: hwnd 013F0054 is still topmost === debian11 (64 bit WoW report) === user32: msg.c:6881: Test failed: SetFocus(hwnd) on a button: 4: the msg 0x0007 was expected, but got msg 0x0005 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 5: the msg 0x0138 was expected, but got msg 0x030f instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 6: the msg 0x0111 was expected, but got msg 0x001c instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 8: the msg 0x8000 was expected, but got msg 0x0086 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 9: the msg sequence is not complete: expected 0000 - actual 0006
From: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- dlls/user32/user_main.c | 18 ------------------ dlls/user32/win.c | 2 +- dlls/user32/win.h | 6 +----- 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/dlls/user32/user_main.c b/dlls/user32/user_main.c index eeef0d57f62..d0c1cc616a7 100644 --- a/dlls/user32/user_main.c +++ b/dlls/user32/user_main.c @@ -42,24 +42,6 @@ static DWORD exiting_thread_id; extern void WDML_NotifyThreadDetach(void); -/*********************************************************************** - * USER_Lock - */ -void USER_Lock(void) -{ - NtUserCallOneParam( 0, NtUserLock ); -} - - -/*********************************************************************** - * USER_Unlock - */ -void USER_Unlock(void) -{ - NtUserCallOneParam( 1, NtUserLock ); -} - - /*********************************************************************** * USER_CheckNotLock * diff --git a/dlls/user32/win.c b/dlls/user32/win.c index 9b9dde82561..bac75063f7d 100644 --- a/dlls/user32/win.c +++ b/dlls/user32/win.c @@ -54,7 +54,7 @@ void *get_user_handle_ptr( HANDLE handle, unsigned int type ) void release_user_handle_ptr( void *ptr ) { assert( ptr && ptr != OBJ_OTHER_PROCESS ); - USER_Unlock(); + NtUserCallOneParam( 1, NtUserLock ); } diff --git a/dlls/user32/win.h b/dlls/user32/win.h index dfa5fdf3283..9d42aef6645 100644 --- a/dlls/user32/win.h +++ b/dlls/user32/win.h @@ -48,14 +48,10 @@ extern HWND *WIN_ListChildren( HWND hwnd ) DECLSPEC_HIDDEN; extern void MDI_CalcDefaultChildPos( HWND hwndClient, INT total, LPPOINT lpPos, INT delta, UINT *id ) DECLSPEC_HIDDEN; extern HDESK open_winstation_desktop( HWINSTA hwinsta, LPCWSTR name, DWORD flags, BOOL inherit, ACCESS_MASK access ) DECLSPEC_HIDDEN; -/* user lock */ -extern void USER_Lock(void) DECLSPEC_HIDDEN; -extern void USER_Unlock(void) DECLSPEC_HIDDEN; - /* to release pointers retrieved by WIN_GetPtr */ static inline void WIN_ReleasePtr( WND *ptr ) { - USER_Unlock(); + release_user_handle_ptr( ptr ); } extern LRESULT HOOK_CallHooks( INT id, INT code, WPARAM wparam, LPARAM lparam, BOOL unicode ) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/204
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=116429 Your paranoid android. === debian11 (32 bit report) === user32: menu.c:2324: Test failed: test 25
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/204
participants (4)
-
Huw Davies (＠huw) -
Jacek Caban
-
Jacek Caban (＠jacek)
-
Marvin