[PATCH v2 0/2] MR7517: ntdll: Do not fail NtCreateUserProcess() if requested access doesn't have PROCESS_CREATE_THREAD.
-- v2: ntdll: Do not fail NtCreateUserProcess() if requested access doesn't have PROCESS_CREATE_THREAD. ntdll/tests: Test NtCreateUserProcess() with limited access rights. https://gitlab.winehq.org/wine/wine/-/merge_requests/7517
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/thread.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/dlls/ntdll/tests/thread.c b/dlls/ntdll/tests/thread.c index a7ccf51e5e2..567a94a583d 100644 --- a/dlls/ntdll/tests/thread.c +++ b/dlls/ntdll/tests/thread.c @@ -204,6 +204,44 @@ static void test_errno(void) ok( val == 0xbeef, "wrong value %x\n", val ); } +static void test_NtCreateUserProcess(void) +{ + RTL_USER_PROCESS_PARAMETERS *params; + PS_CREATE_INFO create_info; + PS_ATTRIBUTE_LIST ps_attr; + WCHAR path[MAX_PATH + 4]; + HANDLE process, thread; + UNICODE_STRING imageW; + NTSTATUS status; + + lstrcpyW( path, L"\\??\\" ); + GetModuleFileNameW( NULL, path + 4, MAX_PATH ); + + RtlInitUnicodeString( &imageW, path ); + + memset( &ps_attr, 0, sizeof(ps_attr) ); + ps_attr.Attributes[0].Attribute = PS_ATTRIBUTE_IMAGE_NAME; + ps_attr.Attributes[0].Size = lstrlenW(path) * sizeof(WCHAR); + ps_attr.Attributes[0].ValuePtr = path; + ps_attr.TotalLength = sizeof(ps_attr); + + status = RtlCreateProcessParametersEx( ¶ms, &imageW, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, PROCESS_PARAMS_FLAG_NORMALIZED ); + ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); + + memset( &create_info, 0, sizeof(create_info) ); + create_info.Size = sizeof(create_info); + status = NtCreateUserProcess( &process, &thread, PROCESS_TERMINATE, SYNCHRONIZE, + NULL, NULL, 0, THREAD_CREATE_FLAGS_CREATE_SUSPENDED, params, + &create_info, &ps_attr ); + todo_wine ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); + status = NtTerminateProcess( process, 0 ); + todo_wine ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); + CloseHandle( process ); + CloseHandle( thread ); +} + START_TEST(thread) { init_function_pointers(); @@ -211,4 +249,5 @@ START_TEST(thread) test_dbg_hidden_thread_creation(); test_unique_teb(); test_errno(); + test_NtCreateUserProcess(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7517
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/thread.c | 4 ++-- dlls/ntdll/unix/process.c | 1 + server/protocol.def | 1 + server/thread.c | 4 ++-- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/tests/thread.c b/dlls/ntdll/tests/thread.c index 567a94a583d..f7806e0fe62 100644 --- a/dlls/ntdll/tests/thread.c +++ b/dlls/ntdll/tests/thread.c @@ -235,9 +235,9 @@ static void test_NtCreateUserProcess(void) status = NtCreateUserProcess( &process, &thread, PROCESS_TERMINATE, SYNCHRONIZE, NULL, NULL, 0, THREAD_CREATE_FLAGS_CREATE_SUSPENDED, params, &create_info, &ps_attr ); - todo_wine ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); + ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); status = NtTerminateProcess( process, 0 ); - todo_wine ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); + ok( status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status ); CloseHandle( process ); CloseHandle( thread ); } diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 533940280e6..d6fc0050086 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -900,6 +900,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ req->access = thread_access; req->flags = thread_flags; req->request_fd = -1; + req->new_process = 1; wine_server_add_data( req, objattr, attr_len ); if (!(status = wine_server_call( req ))) { diff --git a/server/protocol.def b/server/protocol.def index 4f712b4e4e6..972ccb7aebd 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1069,6 +1069,7 @@ struct obj_locator unsigned int access; /* wanted access rights */ unsigned int flags; /* thread creation flags */ int request_fd; /* fd for request pipe */ + int new_process; /* creating first thread in the new process */ VARARG(objattr,object_attributes); /* object attributes */ @REPLY thread_id_t tid; /* thread id */ diff --git a/server/thread.c b/server/thread.c index 5ad060108e3..f04fd2f38e1 100644 --- a/server/thread.c +++ b/server/thread.c @@ -1520,7 +1520,7 @@ DECL_HANDLER(new_thread) const struct object_attributes *objattr = get_req_object_attributes( &sd, &name, NULL ); int request_fd = thread_get_inflight_fd( current, req->request_fd ); - if (!(process = get_process_from_handle( req->process, PROCESS_CREATE_THREAD ))) + if (!(process = get_process_from_handle( req->process, req->new_process ? 0 : PROCESS_CREATE_THREAD ))) { if (request_fd != -1) close( request_fd ); return; @@ -1534,7 +1534,7 @@ DECL_HANDLER(new_thread) set_error( STATUS_INVALID_PARAMETER ); goto done; } - if (process->running_threads) /* only the initial thread can be created in another process */ + if (!req->new_process) /* only the initial thread can be created in another process */ { set_error( STATUS_ACCESS_DENIED ); goto done; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7517
v2: - add a test; - add 'new_process' parameter to server call and handle new process case as a special case in (new_thread) request. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7517#note_97323
Why not reuse the already existing `process->running_threads` check instead of adding a new flag? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7517#note_97408
participants (3)
-
Alexandre Julliard (＠julliard) -
Paul Gofman
-
Paul Gofman (＠gofman)