[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.

diff --git a/dll/win32/kernel32/client/proc.c b/dll/win32/kernel32/client/proc.c
index f362194..b44624c 100644 (file)
--- 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;
 }
 
     _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,
 BOOLEAN
 WINAPI
 BasePushProcessParameters(IN ULONG ParameterFlags,

diff --git a/dll/win32/kernel32/client/thread.c b/dll/win32/kernel32/client/thread.c
index 3505d4b..be33021 100644 (file)
--- 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);
 
 
 typedef NTSTATUS (NTAPI *PCSR_CREATE_REMOTE_THREAD)(IN HANDLE ThreadHandle, IN PCLIENT_ID ClientId);
 

+/* FUNCTIONS ******************************************************************/
+

 NTSTATUS
 WINAPI
 BasepNotifyCsrOfThread(IN HANDLE ThreadHandle,
 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
 
 __declspec(noreturn)
 VOID


@@ -153,12 +178,13 @@ CreateRemoteThread(IN HANDLE hProcess,
     ULONG_PTR Cookie;
     ULONG ReturnLength;
     SIZE_T ReturnSize;
     ULONG_PTR Cookie;
     ULONG ReturnLength;
     SIZE_T ReturnSize;

+

     DPRINT("CreateRemoteThread: hProcess: %p dwStackSize: %lu lpStartAddress"
     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 */
 
     /* Clear the Context */

-    RtlZeroMemory(&Context, sizeof(CONTEXT));
+    RtlZeroMemory(&Context, sizeof(Context));

 
     /* Write PID */
     ClientId.UniqueProcess = hProcess;
 
     /* Write PID */
     ClientId.UniqueProcess = hProcess;


@@ -176,14 +202,14 @@ CreateRemoteThread(IN HANDLE hProcess,
         return NULL;
     }
 
         return NULL;
     }
 

-    /* Create Initial Context */
+    /* Create the Initial Context */

     BaseInitializeContext(&Context,
                           lpParameter,
                           lpStartAddress,
                           InitialTeb.StackBase,
                           1);
 
     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);
     ObjectAttributes = BaseFormatObjectAttributes(&LocalObjectAttributes,
                                                   lpThreadAttributes,
                                                   NULL);


@@ -217,10 +243,10 @@ CreateRemoteThread(IN HANDLE hProcess,
         if (!NT_SUCCESS(Status))
         {
             /* Fail */
         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 */
         }
 
         /* Allocate the Activation Context Stack */


@@ -228,10 +254,10 @@ CreateRemoteThread(IN HANDLE hProcess,
         if (!NT_SUCCESS(Status))
         {
             /* Fail */
         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 */
         }
 
         /* Save it */


@@ -249,15 +275,10 @@ CreateRemoteThread(IN HANDLE hProcess,
         if (!NT_SUCCESS(Status))
         {
             /* Fail */
         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? */
         }
 
         /* Does it need to be activated? */


@@ -271,24 +292,21 @@ CreateRemoteThread(IN HANDLE hProcess,
             if (!NT_SUCCESS(Status))
             {
                 /* Fail */
             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);
     }
 
     /* Notify CSR */
     if (!BaseRunningInServerProcess)
     {
         Status = BasepNotifyCsrOfThread(hThread, &ClientId);

-        ASSERT(NT_SUCCESS(Status));

     }
     else
     {
     }
     else
     {


@@ -304,16 +322,35 @@ CreateRemoteThread(IN HANDLE hProcess,
             {
                 /* Call it instead of going through LPC */
                 Status = CsrCreateRemoteThread(hThread, &ClientId);
             {
                 /* 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 */
     /* 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;
 
     /* Return handle to thread */
     return hThread;