https://git.reactos.org/?p=reactos.git;a=commitdiff;h=44cddadba80aa9efab0da36aa57cf6f7a012df43
commit 44cddadba80aa9efab0da36aa57cf6f7a012df43 Author: Hermès Bélusca-Maïto <[email protected]> AuthorDate: Thu Aug 23 21:44:53 2018 +0200 Commit: Hermès Bélusca-Maïto <[email protected]> CommitDate: Thu Aug 1 19:12:49 2019 +0200 [KERNEL32] Minor enhancements for CreateRemoteThread(). (#804) - Add some cleanup code in failure code paths, instead of asserting. - Move BasepNotifyCsrOfThread() helper to the only file where it is used. - Don't use ERROR_DBGBREAK in failure paths but just DPRINT the error message: we handle the failures properly. - When creating the remote thread, sync its service tag with the parent thread's one. --- dll/win32/kernel32/client/proc.c | 30 ---------- dll/win32/kernel32/client/thread.c | 113 ++++++++++++++++++++++++------------- 2 files changed, 75 insertions(+), 68 deletions(-) diff --git a/dll/win32/kernel32/client/proc.c b/dll/win32/kernel32/client/proc.c index f36219457b2..b44624c381b 100644 --- a/dll/win32/kernel32/client/proc.c +++ b/dll/win32/kernel32/client/proc.c @@ -479,36 +479,6 @@ BaseProcessStartup(PPROCESS_START_ROUTINE lpStartAddress) _SEH2_END; } -NTSTATUS -WINAPI -BasepNotifyCsrOfThread(IN HANDLE ThreadHandle, - IN PCLIENT_ID ClientId) -{ - BASE_API_MESSAGE ApiMessage; - PBASE_CREATE_THREAD CreateThreadRequest = &ApiMessage.Data.CreateThreadRequest; - - DPRINT("BasepNotifyCsrOfThread: Thread: %p, Handle %p\n", - ClientId->UniqueThread, ThreadHandle); - - /* Fill out the request */ - CreateThreadRequest->ClientId = *ClientId; - CreateThreadRequest->ThreadHandle = ThreadHandle; - - /* Call CSR */ - CsrClientCallServer((PCSR_API_MESSAGE)&ApiMessage, - NULL, - CSR_CREATE_API_NUMBER(BASESRV_SERVERDLL_INDEX, BasepCreateThread), - sizeof(*CreateThreadRequest)); - if (!NT_SUCCESS(ApiMessage.Status)) - { - DPRINT1("Failed to tell CSRSS about new thread: %lx\n", ApiMessage.Status); - return ApiMessage.Status; - } - - /* Return Success */ - return STATUS_SUCCESS; -} - BOOLEAN WINAPI BasePushProcessParameters(IN ULONG ParameterFlags, diff --git a/dll/win32/kernel32/client/thread.c b/dll/win32/kernel32/client/thread.c index 3505d4bdae7..be33021fcf3 100644 --- a/dll/win32/kernel32/client/thread.c +++ b/dll/win32/kernel32/client/thread.c @@ -18,12 +18,37 @@ typedef NTSTATUS (NTAPI *PCSR_CREATE_REMOTE_THREAD)(IN HANDLE ThreadHandle, IN PCLIENT_ID ClientId); +/* FUNCTIONS ******************************************************************/ + NTSTATUS WINAPI BasepNotifyCsrOfThread(IN HANDLE ThreadHandle, - IN PCLIENT_ID ClientId); + IN PCLIENT_ID ClientId) +{ + BASE_API_MESSAGE ApiMessage; + PBASE_CREATE_THREAD CreateThreadRequest = &ApiMessage.Data.CreateThreadRequest; + + DPRINT("BasepNotifyCsrOfThread: Thread: %p, Handle %p\n", + ClientId->UniqueThread, ThreadHandle); + + /* Fill out the request */ + CreateThreadRequest->ClientId = *ClientId; + CreateThreadRequest->ThreadHandle = ThreadHandle; + + /* Call CSR */ + CsrClientCallServer((PCSR_API_MESSAGE)&ApiMessage, + NULL, + CSR_CREATE_API_NUMBER(BASESRV_SERVERDLL_INDEX, BasepCreateThread), + sizeof(*CreateThreadRequest)); + if (!NT_SUCCESS(ApiMessage.Status)) + { + DPRINT1("Failed to tell CSRSS about new thread: %lx\n", ApiMessage.Status); + return ApiMessage.Status; + } -/* FUNCTIONS ******************************************************************/ + /* Return Success */ + return STATUS_SUCCESS; +} __declspec(noreturn) VOID @@ -153,12 +178,13 @@ CreateRemoteThread(IN HANDLE hProcess, ULONG_PTR Cookie; ULONG ReturnLength; SIZE_T ReturnSize; + DPRINT("CreateRemoteThread: hProcess: %p dwStackSize: %lu lpStartAddress" - ": %p lpParameter: %p, dwCreationFlags: %lx\n", hProcess, - dwStackSize, lpStartAddress, lpParameter, dwCreationFlags); + ": %p lpParameter: %p, dwCreationFlags: %lx\n", hProcess, + dwStackSize, lpStartAddress, lpParameter, dwCreationFlags); /* Clear the Context */ - RtlZeroMemory(&Context, sizeof(CONTEXT)); + RtlZeroMemory(&Context, sizeof(Context)); /* Write PID */ ClientId.UniqueProcess = hProcess; @@ -176,14 +202,14 @@ CreateRemoteThread(IN HANDLE hProcess, return NULL; } - /* Create Initial Context */ + /* Create the Initial Context */ BaseInitializeContext(&Context, lpParameter, lpStartAddress, InitialTeb.StackBase, 1); - /* initialize the attributes for the thread object */ + /* Initialize the attributes for the thread object */ ObjectAttributes = BaseFormatObjectAttributes(&LocalObjectAttributes, lpThreadAttributes, NULL); @@ -217,10 +243,10 @@ CreateRemoteThread(IN HANDLE hProcess, if (!NT_SUCCESS(Status)) { /* Fail */ - ERROR_DBGBREAK("SXS: %s - Failing thread create because " - "NtQueryInformationThread() failed with status %08lx\n", - __FUNCTION__, Status); - return NULL; + DPRINT1("SXS: %s - Failing thread create because " + "NtQueryInformationThread() failed with status %08lx\n", + __FUNCTION__, Status); + goto Quit; } /* Allocate the Activation Context Stack */ @@ -228,10 +254,10 @@ CreateRemoteThread(IN HANDLE hProcess, if (!NT_SUCCESS(Status)) { /* Fail */ - ERROR_DBGBREAK("SXS: %s - Failing thread create because " - "RtlAllocateActivationContextStack() failed with status %08lx\n", - __FUNCTION__, Status); - return NULL; + DPRINT1("SXS: %s - Failing thread create because " + "RtlAllocateActivationContextStack() failed with status %08lx\n", + __FUNCTION__, Status); + goto Quit; } /* Save it */ @@ -249,15 +275,10 @@ CreateRemoteThread(IN HANDLE hProcess, if (!NT_SUCCESS(Status)) { /* Fail */ - ERROR_DBGBREAK("SXS: %s - Failing thread create because " - "RtlQueryInformationActivationContext() failed with status %08lx\n", - __FUNCTION__, Status); - - /* Free the activation context stack */ - // RtlFreeThreadActivationContextStack(); - RtlFreeActivationContextStack(Teb->ActivationContextStackPointer); - - return NULL; + DPRINT1("SXS: %s - Failing thread create because " + "RtlQueryInformationActivationContext() failed with status %08lx\n", + __FUNCTION__, Status); + goto Quit; } /* Does it need to be activated? */ @@ -271,24 +292,21 @@ CreateRemoteThread(IN HANDLE hProcess, if (!NT_SUCCESS(Status)) { /* Fail */ - ERROR_DBGBREAK("SXS: %s - Failing thread create because " - "RtlActivateActivationContextEx() failed with status %08lx\n", - __FUNCTION__, Status); - - /* Free the activation context stack */ - // RtlFreeThreadActivationContextStack(); - RtlFreeActivationContextStack(Teb->ActivationContextStackPointer); - - return NULL; + DPRINT1("SXS: %s - Failing thread create because " + "RtlActivateActivationContextEx() failed with status %08lx\n", + __FUNCTION__, Status); + goto Quit; } } + + /* Sync the service tag with the parent thread's one */ + Teb->SubProcessTag = NtCurrentTeb()->SubProcessTag; } /* Notify CSR */ if (!BaseRunningInServerProcess) { Status = BasepNotifyCsrOfThread(hThread, &ClientId); - ASSERT(NT_SUCCESS(Status)); } else { @@ -304,16 +322,35 @@ CreateRemoteThread(IN HANDLE hProcess, { /* Call it instead of going through LPC */ Status = CsrCreateRemoteThread(hThread, &ClientId); - ASSERT(NT_SUCCESS(Status)); } } } +Quit: + if (!NT_SUCCESS(Status)) + { + /* Failed to create the thread */ + + /* Free the activation context stack */ + if (ActivationContextStack) + RtlFreeActivationContextStack(ActivationContextStack); + + NtTerminateThread(hThread, Status); + // FIXME: Wait for the thread to terminate? + BaseFreeThreadStack(hProcess, &InitialTeb); + NtClose(hThread); + + BaseSetLastNTError(Status); + return NULL; + } + /* Success */ - if (lpThreadId) *lpThreadId = HandleToUlong(ClientId.UniqueThread); + if (lpThreadId) + *lpThreadId = HandleToUlong(ClientId.UniqueThread); - /* Resume it if asked */ - if (!(dwCreationFlags & CREATE_SUSPENDED)) NtResumeThread(hThread, &Dummy); + /* Resume the thread if asked */ + if (!(dwCreationFlags & CREATE_SUSPENDED)) + NtResumeThread(hThread, &Dummy); /* Return handle to thread */ return hThread;
