[PATCH v2 0/2] MR4611: ntdll: implement NtGetNextProcess
Largely based on Nikolay Sivov's original patch and !3096. The files modified from running make_requests and make_specfile have been omitted. -- v2: ntdll: Implement NtGetNextProcess. https://gitlab.winehq.org/wine/wine/-/merge_requests/4611
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- dlls/ntdll/tests/om.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c index 62659fc8cb4..bfeb76fa88a 100644 --- a/dlls/ntdll/tests/om.c +++ b/dlls/ntdll/tests/om.c @@ -65,6 +65,7 @@ static NTSTATUS (WINAPI *pNtOpenIoCompletion)( PHANDLE, ACCESS_MASK, POBJECT_ATT static NTSTATUS (WINAPI *pNtQueryInformationFile)(HANDLE, PIO_STATUS_BLOCK, void *, ULONG, FILE_INFORMATION_CLASS); static NTSTATUS (WINAPI *pNtOpenProcess)( HANDLE *, ACCESS_MASK, const OBJECT_ATTRIBUTES *, const CLIENT_ID * ); static NTSTATUS (WINAPI *pNtCreateDebugObject)( HANDLE *, ACCESS_MASK, OBJECT_ATTRIBUTES *, ULONG ); +static NTSTATUS (WINAPI *pNtGetNextProcess)(HANDLE process, ACCESS_MASK access, ULONG attributes, ULONG flags, HANDLE *handle); static NTSTATUS (WINAPI *pNtGetNextThread)(HANDLE process, HANDLE thread, ACCESS_MASK access, ULONG attributes, ULONG flags, HANDLE *handle); static NTSTATUS (WINAPI *pNtOpenProcessToken)(HANDLE,DWORD,HANDLE*); @@ -3156,6 +3157,32 @@ static void test_null_in_object_name(void) skip("Limited access to \\Registry\\Machine\\Software key, skipping the tests\n"); } +static void test_get_next_process(void) +{ + NTSTATUS status; + HANDLE handle; + + if (!pNtGetNextProcess) + { + win_skip("NtGetNextProcess is not available.\n"); + return; + } + + status = pNtGetNextProcess(0, PROCESS_QUERY_LIMITED_INFORMATION, OBJ_INHERIT, 0, &handle); + ok(!status, "Unexpected status %#lx.\n", status); + pNtClose(handle); + + /* Reversed search only supported in recent enough Win10 */ + status = pNtGetNextProcess(0, PROCESS_QUERY_LIMITED_INFORMATION, OBJ_INHERIT, 1, &handle); + ok(!status || broken(status == STATUS_INVALID_PARAMETER), "Unexpected status %#lx.\n", status); + if (status) + pNtClose(handle); + + status = pNtGetNextProcess(0, PROCESS_QUERY_LIMITED_INFORMATION, OBJ_INHERIT, 2, &handle); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %#lx.\n", status); +} + + START_TEST(om) { HMODULE hntdll = GetModuleHandleA("ntdll.dll"); @@ -3194,6 +3221,7 @@ START_TEST(om) pNtQueryInformationFile = (void *)GetProcAddress(hntdll, "NtQueryInformationFile"); pNtOpenProcess = (void *)GetProcAddress(hntdll, "NtOpenProcess"); pNtCreateDebugObject = (void *)GetProcAddress(hntdll, "NtCreateDebugObject"); + pNtGetNextProcess = (void *)GetProcAddress(hntdll, "NtGetNextProcess"); pNtGetNextThread = (void *)GetProcAddress(hntdll, "NtGetNextThread"); pNtOpenProcessToken = (void *)GetProcAddress(hntdll, "NtOpenProcessToken"); pNtOpenThreadToken = (void *)GetProcAddress(hntdll, "NtOpenThreadToken"); @@ -3215,6 +3243,7 @@ START_TEST(om) test_token(); test_duplicate_object(); test_object_types(); + test_get_next_process(); test_get_next_thread(); test_globalroot(); test_object_identity(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4611
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- dlls/ntdll/ntdll.spec | 1 + dlls/ntdll/unix/process.c | 28 ++++++++++++++++++++++++++ dlls/wow64/process.c | 21 ++++++++++++++++++++ include/winternl.h | 1 + server/process.c | 41 +++++++++++++++++++++++++++++++++++++++ server/protocol.def | 9 +++++++++ 6 files changed, 101 insertions(+) diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 1908a089d44..3644c666fce 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -230,6 +230,7 @@ @ stdcall -norelay -syscall NtGetContextThread(long ptr) @ stdcall -syscall NtGetCurrentProcessorNumber() # @ stub NtGetDevicePowerState +@ stdcall -syscall NtGetNextProcess(ptr long long long ptr) @ stdcall -syscall NtGetNextThread(ptr ptr long long long ptr) @ stdcall -syscall NtGetNlsSectionPtr(long long long ptr ptr) # @ stub NtGetPlugPlayEvent diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 71eab55920d..fb82a0a622c 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -1768,6 +1768,34 @@ NTSTATUS WINAPI NtResumeProcess( HANDLE handle ) return ret; } +/********************************************************************** + * NtGetNextProcess (NTDLL.@) + */ +NTSTATUS WINAPI NtGetNextProcess( HANDLE process, ACCESS_MASK access, ULONG attributes, + ULONG flags, HANDLE *handle ) +{ + unsigned int ret; + HANDLE ret_handle; + + TRACE( "process %p, access %#x, attributes %#x, flags %#x, handle %p.\n", + process, (int)access, (int)attributes, (int)flags, handle ); + + + SERVER_START_REQ( get_next_process ) + { + req->last = wine_server_obj_handle( process ); + req->access = access; + req->attributes = attributes; + req->flags = flags; + if (!(ret = wine_server_call( req ))) ret_handle = wine_server_ptr_handle( reply->handle ); + } + SERVER_END_REQ; + + *handle = ret_handle; + return ret; +} + + /********************************************************************** * NtDebugActiveProcess (NTDLL.@) diff --git a/dlls/wow64/process.c b/dlls/wow64/process.c index 817926d0f13..6fe997bb0b4 100644 --- a/dlls/wow64/process.c +++ b/dlls/wow64/process.c @@ -435,6 +435,27 @@ NTSTATUS WINAPI wow64_NtFlushProcessWriteBuffers( UINT *args ) } +/********************************************************************** + * wow64_NtGetNextProcess + */ +NTSTATUS WINAPI wow64_NtGetNextProcess( UINT *args ) +{ + HANDLE process = get_handle( &args ); + ACCESS_MASK access = get_ulong( &args ); + ULONG attributes = get_ulong( &args ); + ULONG flags = get_ulong( &args ); + ULONG *handle_ptr = get_ptr( &args ); + + HANDLE handle = 0; + NTSTATUS status; + + *handle_ptr = 0; + status = NtGetNextProcess( process, access, attributes, flags, &handle ); + put_handle( handle_ptr, handle ); + return status; +} + + /********************************************************************** * wow64_NtGetNextThread */ diff --git a/include/winternl.h b/include/winternl.h index ff8756211a1..ab79940e8e7 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4411,6 +4411,7 @@ NTSYSAPI NTSTATUS WINAPI NtFreeVirtualMemory(HANDLE,PVOID*,SIZE_T*,ULONG); NTSYSAPI NTSTATUS WINAPI NtFsControlFile(HANDLE,HANDLE,PIO_APC_ROUTINE,PVOID,PIO_STATUS_BLOCK,ULONG,PVOID,ULONG,PVOID,ULONG); NTSYSAPI NTSTATUS WINAPI NtGetContextThread(HANDLE,CONTEXT*); NTSYSAPI ULONG WINAPI NtGetCurrentProcessorNumber(void); +NTSYSAPI NTSTATUS WINAPI NtGetNextProcess(HANDLE,ACCESS_MASK,ULONG,ULONG,HANDLE*); NTSYSAPI NTSTATUS WINAPI NtGetNextThread(HANDLE,HANDLE,ACCESS_MASK,ULONG,ULONG,HANDLE*); NTSYSAPI NTSTATUS WINAPI NtGetNlsSectionPtr(ULONG,ULONG,void*,void**,SIZE_T*); NTSYSAPI NTSTATUS WINAPI NtGetPlugPlayEvent(ULONG,ULONG,PVOID,ULONG); diff --git a/server/process.c b/server/process.c index a0d5ea64d97..f19e87a7a46 100644 --- a/server/process.c +++ b/server/process.c @@ -1390,6 +1390,47 @@ DECL_HANDLER(get_new_process_info) } } +/* Itererate processes using global process list */ +DECL_HANDLER(get_next_process) +{ + struct process *process = NULL; + struct list *ptr; + + if (req->flags > 1) + { + set_error( STATUS_INVALID_PARAMETER ); + return; + } + + if (!req->last) + { + ptr = req->flags ? list_tail( &process_list ) : list_head( &process_list ); + } + else + { + if (!(process = get_process_from_id( req->last ))) + return; + ptr = req->flags ? list_prev( &process_list, &process->entry ) + : list_next( &process_list, &process->entry ); + release_object( process ); + } + + while (ptr) + { + process = LIST_ENTRY( ptr, struct process, entry ); + if ((reply->handle = alloc_handle( current->process, process, req->access, req->attributes ))) + break; + ptr = req->flags ? list_prev( &process_list, &process->entry ) + : list_next( &process_list, &process->entry ); + } + + if (!reply->handle) + set_error( STATUS_NO_MORE_ENTRIES ); + + if (process) + release_object( process ); +} + /* Retrieve the new process startup info */ DECL_HANDLER(get_startup_info) { diff --git a/server/protocol.def b/server/protocol.def index 5d60e7fcda3..267ca60182c 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3873,6 +3873,15 @@ struct handle_info obj_handle_t handle; /* process handle */ @END +/* Itererate processes using global process list */ +(a)REQ(get_next_process) + obj_handle_t last; /* process handle to start with */ + unsigned int access; /* desired access for returned handle */ + unsigned int attributes; /* returned handle attributes */ + unsigned int flags; /* controls iteration direction */ +(a)REPLY + obj_handle_t handle; /* next process handle */ +(a)END /* Iterate thread list for process */ @REQ(get_next_thread) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4611
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=140805 Your paranoid android. === debian11 (build log) === /usr/bin/i686-w64-mingw32-ld: tmp65ac144b/ntdll-00000000.spec.o:fake:(.edata+0x308): undefined reference to `NtGetNextProcess(a)20' collect2: error: ld returned 1 exit status Task: The win32 Wine build failed === debian11b (build log) === /usr/bin/x86_64-w64-mingw32-ld: tmp6573ab45/ntdll-00000000.spec.o:fake:(.edata+0x304): undefined reference to `NtGetNextProcess' collect2: error: ld returned 1 exit status Task: The wow64 Wine build failed
Paul Gofman (@gofman) commented about server/process.c:
+ struct list *ptr; + + if (req->flags > 1) + { + set_error( STATUS_INVALID_PARAMETER ); + return; + } + + if (!req->last) + { + ptr = req->flags ? list_tail( &process_list ) : list_head( &process_list ); + } + else + { + if (!(process = get_process_from_id( req->last ))) + return; This doesn't look right, you probably want get_process_from_handle().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4611#note_55170
Paul Gofman (@gofman) commented about server/process.c:
+ } + + while (ptr) + { + process = LIST_ENTRY( ptr, struct process, entry ); + if ((reply->handle = alloc_handle( current->process, process, req->access, req->attributes ))) + break; + ptr = req->flags ? list_prev( &process_list, &process->entry ) + : list_next( &process_list, &process->entry ); + } + + if (!reply->handle) + set_error( STATUS_NO_MORE_ENTRIES ); + + if (process) + release_object( process ); This release_object is off, it looks like for the only case when you are trying to reference the process above there is already matching release.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4611#note_55171
Paul Gofman (@gofman) commented about dlls/ntdll/tests/om.c:
+ } + + status = pNtGetNextProcess(0, PROCESS_QUERY_LIMITED_INFORMATION, OBJ_INHERIT, 0, &handle); + ok(!status, "Unexpected status %#lx.\n", status); + pNtClose(handle); + + /* Reversed search only supported in recent enough Win10 */ + status = pNtGetNextProcess(0, PROCESS_QUERY_LIMITED_INFORMATION, OBJ_INHERIT, 1, &handle); + ok(!status || broken(status == STATUS_INVALID_PARAMETER), "Unexpected status %#lx.\n", status); + if (status) + pNtClose(handle); + + status = pNtGetNextProcess(0, PROCESS_QUERY_LIMITED_INFORMATION, OBJ_INHERIT, 2, &handle); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %#lx.\n", status); +} + This deserves a bit more detailed test IMO. I'd create a separate process for that to see if you can find it, and then also test whether terminated process (but for which you still have the open handle) is returned or not this way. It might probably be easier to move this test into process.c then where there are appropriate process creation helpers. In the current form the test apparently doesn't catch the bug in the implementation patch (get_process_by_id vs get_process_by_handle) with which actual processes enumeration is not possible.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4611#note_55172
participants (4)
-
Etaash Mathamsetty -
Etaash Mathamsetty (＠etaash.mathamsetty)
-
Marvin -
Paul Gofman (＠gofman)