Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
537bb7a8aee278d285cb77669fd9258dfaa3222f was potentially involved in a Diablo III regression, where NtSetInformationThread previously returned success for any thread, where it then only succeeded with the current thread.
This series should implement the HideFromDebugger request correctly, checking handles and access rights on the server side. The hiding part is still not implemented, but it could then use the dbg_hidden field on the thread structure if we want to implement the debugger side.
According to the kernel32 thread test, HideFromDebugger can only be queried with QUERY_INFORMATION rights, and not QUERY_LIMITED_INFORMATION thus the additional field in the get_thread_info request.
dlls/ntdll/tests/info.c | 46 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index eaf2f1a45b7..b91d54375ce 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -2346,7 +2346,6 @@ static void test_affinity(void) DWORD_PTR proc_affinity, thread_affinity; THREAD_BASIC_INFORMATION tbi; SYSTEM_INFO si; - ULONG dummy;
GetSystemInfo(&si); status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessBasicInformation, &pbi, sizeof(pbi), NULL ); @@ -2451,6 +2450,20 @@ static void test_affinity(void) ok( status == STATUS_SUCCESS, "Expected STATUS_SUCCESS, got %08x\n", status); ok( tbi.AffinityMask == (1 << si.dwNumberOfProcessors) - 1, "Unexpected thread affinity\n" ); +} + +static DWORD WINAPI hide_from_debugger_thread(void *arg) +{ + HANDLE stop_event = arg; + WaitForSingleObject( stop_event, INFINITE ); + return 0; +} + +static void test_HideFromDebugger(void) +{ + NTSTATUS status; + HANDLE thread, stop_event; + ULONG dummy;
dummy = 0; status = pNtSetInformationThread( GetCurrentThread(), ThreadHideFromDebugger, &dummy, sizeof(ULONG) ); @@ -2477,6 +2490,34 @@ static void test_affinity(void) ok( status == STATUS_SUCCESS, "Expected STATUS_SUCCESS, got %08x\n", status ); if (status == STATUS_SUCCESS) ok( dummy == 1, "Expected dummy == 1, got %08x\n", dummy ); } + + stop_event = CreateEventA( NULL, FALSE, FALSE, NULL ); + ok( stop_event != NULL, "CreateEvent failed\n" ); + thread = CreateThread( NULL, 0, hide_from_debugger_thread, stop_event, 0, NULL ); + ok( thread != INVALID_HANDLE_VALUE, "CreateThread failed with %d\n", GetLastError() ); + + dummy = 0; + status = NtQueryInformationThread( thread, ThreadHideFromDebugger, &dummy, 1, NULL ); + todo_wine + ok( status == STATUS_SUCCESS || status == STATUS_INVALID_INFO_CLASS, + "Expected STATUS_SUCCESS, got %08x\n", status ); + if (status == STATUS_SUCCESS) ok( dummy == 0, "Expected dummy == 0, got %08x\n", dummy ); + + status = pNtSetInformationThread( thread, ThreadHideFromDebugger, NULL, 0 ); + todo_wine + ok( status == STATUS_SUCCESS, "Expected STATUS_SUCCESS, got %08x\n", status ); + + dummy = 0; + status = NtQueryInformationThread( thread, ThreadHideFromDebugger, &dummy, 1, NULL ); + todo_wine + ok( status == STATUS_SUCCESS || status == STATUS_INVALID_INFO_CLASS, + "Expected STATUS_SUCCESS, got %08x\n", status ); + if (status == STATUS_SUCCESS) ok( dummy == 1, "Expected dummy == 1, got %08x\n", dummy ); + + SetEvent( stop_event ); + WaitForSingleObject( thread, INFINITE ); + CloseHandle( thread ); + CloseHandle( stop_event ); }
static void test_NtGetCurrentProcessorNumber(void) @@ -2817,6 +2858,9 @@ START_TEST(info) trace("Starting test_affinity()\n"); test_affinity();
+ trace("Starting test_HideFromDebugger()\n"); + test_HideFromDebugger(); + trace("Starting test_NtGetCurrentProcessorNumber()\n"); test_NtGetCurrentProcessorNumber();
At least, store the thread information, instead of pretending and failing to correctly validate handles and access rights.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/tests/info.c | 3 --- dlls/ntdll/unix/thread.c | 22 +++++++++++++++++----- server/protocol.def | 3 +++ server/thread.c | 8 +++++++- server/thread.h | 1 + 5 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index b91d54375ce..aae729dccff 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -2498,18 +2498,15 @@ static void test_HideFromDebugger(void)
dummy = 0; status = NtQueryInformationThread( thread, ThreadHideFromDebugger, &dummy, 1, NULL ); - todo_wine ok( status == STATUS_SUCCESS || status == STATUS_INVALID_INFO_CLASS, "Expected STATUS_SUCCESS, got %08x\n", status ); if (status == STATUS_SUCCESS) ok( dummy == 0, "Expected dummy == 0, got %08x\n", dummy );
status = pNtSetInformationThread( thread, ThreadHideFromDebugger, NULL, 0 ); - todo_wine ok( status == STATUS_SUCCESS, "Expected STATUS_SUCCESS, got %08x\n", status );
dummy = 0; status = NtQueryInformationThread( thread, ThreadHideFromDebugger, &dummy, 1, NULL ); - todo_wine ok( status == STATUS_SUCCESS || status == STATUS_INVALID_INFO_CLASS, "Expected STATUS_SUCCESS, got %08x\n", status ); if (status == STATUS_SUCCESS) ok( dummy == 1, "Expected dummy == 1, got %08x\n", dummy ); diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c index f3dddd2b02a..8463119a5ed 100644 --- a/dlls/ntdll/unix/thread.c +++ b/dlls/ntdll/unix/thread.c @@ -1067,8 +1067,15 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE handle, THREADINFOCLASS class, case ThreadHideFromDebugger: if (length != sizeof(BOOLEAN)) return STATUS_INFO_LENGTH_MISMATCH; if (!data) return STATUS_ACCESS_VIOLATION; - if (handle != GetCurrentThread()) return STATUS_ACCESS_DENIED; - *(BOOLEAN*)data = TRUE; + SERVER_START_REQ( get_thread_info ) + { + req->handle = wine_server_obj_handle( handle ); + req->tid_in = 0; + req->access = THREAD_QUERY_INFORMATION; + if ((status = wine_server_call( req ))) return status; + *(BOOLEAN*)data = reply->dbg_hidden; + } + SERVER_END_REQ; if (ret_len) *ret_len = sizeof(BOOLEAN); return STATUS_SUCCESS;
@@ -1164,9 +1171,14 @@ NTSTATUS WINAPI NtSetInformationThread( HANDLE handle, THREADINFOCLASS class,
case ThreadHideFromDebugger: if (length) return STATUS_INFO_LENGTH_MISMATCH; - if (handle != GetCurrentThread()) return STATUS_INVALID_HANDLE; - /* pretend the call succeeded to satisfy some code protectors */ - return STATUS_SUCCESS; + SERVER_START_REQ( set_thread_info ) + { + req->handle = wine_server_obj_handle( handle ); + req->mask = SET_THREAD_INFO_DBG_HIDDEN; + status = wine_server_call( req ); + } + SERVER_END_REQ; + return status;
case ThreadQuerySetWin32StartAddress: { diff --git a/server/protocol.def b/server/protocol.def index c3442c06e9b..ff2f3ed31ab 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -946,6 +946,7 @@ struct rawinput_device @REQ(get_thread_info) obj_handle_t handle; /* thread handle */ thread_id_t tid_in; /* thread id (optional) */ + unsigned int access; /* required access rights */ @REPLY process_id_t pid; /* server process id */ thread_id_t tid; /* server thread id */ @@ -956,6 +957,7 @@ struct rawinput_device int priority; /* thread priority level */ int last; /* last thread in process */ int suspend_count; /* thread suspend count */ + int dbg_hidden; /* thread hidden from debugger */ data_size_t desc_len; /* description length in bytes */ VARARG(desc,unicode_str); /* description string */ @END @@ -985,6 +987,7 @@ struct rawinput_device #define SET_THREAD_INFO_TOKEN 0x04 #define SET_THREAD_INFO_ENTRYPOINT 0x08 #define SET_THREAD_INFO_DESCRIPTION 0x10 +#define SET_THREAD_INFO_DBG_HIDDEN 0x20
/* Retrieve information about a module */ diff --git a/server/thread.c b/server/thread.c index e2bfa50c7ba..eb287d74184 100644 --- a/server/thread.c +++ b/server/thread.c @@ -234,6 +234,7 @@ static inline void init_thread_structure( struct thread *thread ) thread->exit_code = 0; thread->priority = 0; thread->suspend = 0; + thread->dbg_hidden = 0; thread->desktop_users = 0; thread->token = NULL; thread->desc = NULL; @@ -624,6 +625,8 @@ static void set_thread_info( struct thread *thread, security_set_thread_token( thread, req->token ); if (req->mask & SET_THREAD_INFO_ENTRYPOINT) thread->entry_point = req->entry_point; + if (req->mask & SET_THREAD_INFO_DBG_HIDDEN) + thread->dbg_hidden = 1; if (req->mask & SET_THREAD_INFO_DESCRIPTION) { WCHAR *desc; @@ -1516,9 +1519,11 @@ DECL_HANDLER(get_thread_info) { struct thread *thread; obj_handle_t handle = req->handle; + unsigned int access = req->access;
+ if (!access) access = THREAD_QUERY_LIMITED_INFORMATION; if (!handle) thread = get_thread_from_id( req->tid_in ); - else thread = get_thread_from_handle( req->handle, THREAD_QUERY_LIMITED_INFORMATION ); + else thread = get_thread_from_handle( req->handle, access );
if (thread) { @@ -1531,6 +1536,7 @@ DECL_HANDLER(get_thread_info) reply->affinity = thread->affinity; reply->last = thread->process->running_threads == 1; reply->suspend_count = thread->suspend; + reply->dbg_hidden = thread->dbg_hidden; reply->desc_len = thread->desc_len;
if (thread->desc && get_reply_max_size()) diff --git a/server/thread.h b/server/thread.h index 5d12d24dd89..183f6baa0f2 100644 --- a/server/thread.h +++ b/server/thread.h @@ -82,6 +82,7 @@ struct thread affinity_t affinity; /* affinity mask */ int priority; /* priority level */ int suspend; /* suspend count */ + int dbg_hidden; /* hidden from debugger */ obj_handle_t desktop; /* desktop handle */ int desktop_users; /* number of objects using the thread desktop */ timeout_t creation_time; /* Thread creation time */
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=74303
Your paranoid android.
=== wxppro (testbot log) ===
The task timed out
-
Marvin -
Rémi Bernon