[ros-diffs] [reactos] 04/04: [SMSS] Fix three SmpLoadSubSystem bugs related to the SB_CREATE_SESSION callback.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2dddbd5c5495a8a521e5b…

commit 2dddbd5c5495a8a521e5b4addc0396fe2cde1f9f
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Sun Nov 13 01:47:26 2022 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Mon Nov 14 00:10:35 2022 +0100

    [SMSS] Fix three SmpLoadSubSystem bugs related to the SB_CREATE_SESSION callback.
    
    This fixes starting the Windows 2000 POSIX subsystem in ReactOS.
    
    - The CreateSession pointer was initialized against the SbApiMsg variable, but
      it was the other SbApiMsg2 that was being initialized and sent through LPC.
    
    - Do not overwrite the MuSessionId (Terminal Services session ID) variable with
      the generated environment subsystem session ID from SmpAllocateSessionId().
    
    - Actually initialize the SbApiMsg ApiNumber for the CreateSession LPC call.
    
    (dll\win32\kernel32\client\proc.c:3690) Retrying with: POSIX /P
C:\ReactOS\system32\posix\ls.exe /C ls
    Breakpoint 1 hit
    csrsrv!CsrSbApiRequestThread+0x64:
    001b:1000ac34 837dfc00        cmp     dword ptr [ebp-4],0
    kd> ??ReceiveMsg
    struct _SB_API_MSG
       +0x000 h                : _PORT_MESSAGE
       +0x018 ConnectionInfo   : _SB_CONNECTION_INFO
       +0x018 ApiNumber        : 0xcccccccc (No matching name)
       +0x01c ReturnValue      : 0n0
       +0x020 u                : <unnamed-tag>
    kd> p
    ...
    (base\system\smss\smsubsys.c:393) SMSS: SmpLoadSubSystem - NtRequestWaitReplyPort
Failed with  Status c0000002 for sessionid 2
    ...
    <Retrying>
    ...
    (base\system\smss\smsubsys.c:393) SMSS: SmpLoadSubSystem - NtRequestWaitReplyPort
Failed with  Status c0000002 for sessionid 3
    
    All those bugs could have been avoided *IF*, rather than (badly) duplicating
    its code, the existing SmpSbCreateSession() function had been used instead.
    
    - "Not sure these field mean what I think they do -- but clear them" ...
◔_◔
      Those fields are related to the debug client interface (DbgUi) and session
      in case the subsystem being started is going to be debugged. These have
      nothing to do with the MuSessionId. Clarify this in the SB_CREATE_SESSION_MSG
      structure and in the SmpSbCreateSession() function.
---
 base/system/smss/smsbapi.c            | 46 +++++++++++++++++++++--------------
 base/system/smss/smss.h               |  4 +--
 base/system/smss/smsubsys.c           | 31 +++++++++++------------
 sdk/include/reactos/subsys/sm/smmsg.h |  6 ++---
 4 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/base/system/smss/smsbapi.c b/base/system/smss/smsbapi.c
index e1b21f7bdb3..f8eafe45a3f 100644
--- a/base/system/smss/smsbapi.c
+++ b/base/system/smss/smsbapi.c
@@ -36,28 +36,28 @@ NTAPI
 SmpSbCreateSession(IN PVOID Reserved,
                    IN PSMP_SUBSYSTEM OtherSubsystem,
                    IN PRTL_USER_PROCESS_INFORMATION ProcessInformation,
-                   IN ULONG MuSessionId,
-                   IN PCLIENT_ID DbgClientId)
+                   IN ULONG DbgSessionId,
+                   IN PCLIENT_ID DbgUiClientId)
 {
     NTSTATUS Status;
     ULONG SubSystemType = ProcessInformation->ImageInformation.SubSystemType;
-    PSMP_SUBSYSTEM KnownSubsys;
-    SB_API_MSG SbApiMsg;
+    ULONG MuSessionId;
     ULONG SessionId;
-    PSB_CREATE_SESSION_MSG CreateSessionMsg;
+    PSMP_SUBSYSTEM KnownSubsys;
+    SB_API_MSG SbApiMsg = {0};
+    PSB_CREATE_SESSION_MSG CreateSessionMsg = &SbApiMsg.u.CreateSession;
 
     /* Write out the create session message including its initial process */
-    CreateSessionMsg = &SbApiMsg.u.CreateSession;
     CreateSessionMsg->ProcessInfo = *ProcessInformation;
-    CreateSessionMsg->MuSessionId = MuSessionId;
-    if (DbgClientId)
+    CreateSessionMsg->DbgSessionId = DbgSessionId;
+    if (DbgUiClientId)
     {
-        CreateSessionMsg->ClientId = *DbgClientId;
+        CreateSessionMsg->DbgUiClientId = *DbgUiClientId;
     }
     else
     {
-        CreateSessionMsg->ClientId.UniqueThread = NULL;
-        CreateSessionMsg->ClientId.UniqueProcess = NULL;
+        CreateSessionMsg->DbgUiClientId.UniqueThread = NULL;
+        CreateSessionMsg->DbgUiClientId.UniqueProcess = NULL;
     }
 
     /* Find a subsystem responsible for this session */
@@ -70,7 +70,7 @@ SmpSbCreateSession(IN PVOID Reserved,
         return STATUS_OBJECT_NAME_NOT_FOUND;
     }
 
-    /* Find the subsystem we have for this initial process */
+    /* Find the subsystem suitable for this initial process */
     KnownSubsys = SmpLocateKnownSubSysByType(MuSessionId, SubSystemType);
     if (KnownSubsys)
     {
@@ -169,8 +169,13 @@ SmpSbCreateSession(IN PVOID Reserved,
     }
 
 #if 0
-    /* This code handles debug applications, but it seems vestigial... */
-    if ((*(ULONGLONG)&CreateSessionMsg.ClientId) && (SmpDbgSsLoaded))
+    /*
+     * This code is part of the LPC-based legacy debugging support for native
+     * applications, implemented with the debug client interface (DbgUi) and
+     * debug subsystem (DbgSs). It is now vestigial since WinXP+ and is here
+     * for informational purposes only.
+     */
+    if ((*(ULONGLONG)&CreateSessionMsg.DbgUiClientId) && SmpDbgSsLoaded)
     {
         Process = RtlAllocateHeap(SmpHeap, SmBaseTag, sizeof(SMP_PROCESS));
         if (!Process)
@@ -183,12 +188,17 @@ SmpSbCreateSession(IN PVOID Reserved,
             return STATUS_NO_MEMORY;
         }
 
-        Process->DbgClientId = CreateSessionMsg->ClientId;
+        Process->DbgUiClientId = CreateSessionMsg->DbgUiClientId;
         Process->ClientId = ProcessInformation->ClientId;
         InsertHeadList(&NativeProcessList, &Process->Entry);
-        DPRINT1("Native Debug App %lx.%lx\n",
Process->ClientId.UniqueProcess, Process->ClientId.UniqueThread);
-
-        Status = NtSetInformationProcess(ProcessInformation->ProcessHandle, 7,
&SmpDebugPort, 4);
+        DPRINT1("Native Debug App %lx.%lx\n",
+                Process->ClientId.UniqueProcess,
+                Process->ClientId.UniqueThread);
+
+        Status = NtSetInformationProcess(ProcessInformation->ProcessHandle,
+                                         ProcessDebugPort,
+                                         &SmpDebugPort,
+                                         sizeof(SmpDebugPort));
         ASSERT(NT_SUCCESS(Status));
     }
 #endif
diff --git a/base/system/smss/smss.h b/base/system/smss/smss.h
index fb63117cea5..4798f570d2f 100644
--- a/base/system/smss/smss.h
+++ b/base/system/smss/smss.h
@@ -168,8 +168,8 @@ SmpSbCreateSession(
     IN PVOID Reserved,
     IN PSMP_SUBSYSTEM OtherSubsystem,
     IN PRTL_USER_PROCESS_INFORMATION ProcessInformation,
-    IN ULONG MuSessionId,
-    IN PCLIENT_ID DbgClientId
+    IN ULONG DbgSessionId,
+    IN PCLIENT_ID DbgUiClientId
 );
 
 /* smsessn.c */
diff --git a/base/system/smss/smsubsys.c b/base/system/smss/smsubsys.c
index 1d6738c8237..40bcfa946f6 100644
--- a/base/system/smss/smsubsys.c
+++ b/base/system/smss/smsubsys.c
@@ -145,7 +145,7 @@ SmpLoadSubSystem(IN PUNICODE_STRING FileName,
     PSMP_SUBSYSTEM Subsystem, NewSubsystem, KnownSubsystem = NULL;
     HANDLE SubSysProcessId;
     NTSTATUS Status = STATUS_SUCCESS;
-    SB_API_MSG SbApiMsg, SbApiMsg2;
+    SB_API_MSG SbApiMsg;
     RTL_USER_PROCESS_INFORMATION ProcessInformation;
     LARGE_INTEGER Timeout;
     PVOID State;
@@ -316,13 +316,10 @@ SmpLoadSubSystem(IN PUNICODE_STRING FileName,
     else
     {
         /* This is the POSIX or OS/2 subsystem process, copy its information */
-        RtlCopyMemory(&CreateSession->ProcessInfo,
-                      &ProcessInformation,
-                      sizeof(CreateSession->ProcessInfo));
+        CreateSession->ProcessInfo = ProcessInformation;
 
-        /* Not sure these field mean what I think they do -- but clear them */
-        *(PULONGLONG)&CreateSession->ClientId = 0;
-        CreateSession->MuSessionId = 0;
+        CreateSession->DbgSessionId = 0;
+        *(PULONGLONG)&CreateSession->DbgUiClientId = 0;
 
         /* This should find CSRSS because they are POSIX or OS/2 subsystems */
         Subsystem = SmpLocateKnownSubSysByType(MuSessionId,
@@ -372,25 +369,25 @@ SmpLoadSubSystem(IN PUNICODE_STRING FileName,
         }
 
         /* Allocate an internal Session ID for this subsystem */
-        MuSessionId = SmpAllocateSessionId(Subsystem, 0);
-        CreateSession->SessionId = MuSessionId;
+        CreateSession->SessionId = SmpAllocateSessionId(Subsystem, NULL);
 
         /* Send the create session message to the subsystem */
-        SbApiMsg2.ReturnValue = STATUS_SUCCESS;
-        SbApiMsg2.h.u2.ZeroInit = 0;
-        SbApiMsg2.h.u1.s1.DataLength = sizeof(SB_CREATE_SESSION_MSG) + 8;
-        SbApiMsg2.h.u1.s1.TotalLength = sizeof(SB_API_MSG);
+        SbApiMsg.ReturnValue = STATUS_SUCCESS;
+        SbApiMsg.h.u2.ZeroInit = 0;
+        SbApiMsg.h.u1.s1.DataLength = sizeof(SB_CREATE_SESSION_MSG) + 8;
+        SbApiMsg.h.u1.s1.TotalLength = sizeof(SB_API_MSG);
+        SbApiMsg.ApiNumber = SbpCreateSession;
         Status = NtRequestWaitReplyPort(Subsystem->SbApiPort,
-                                        &SbApiMsg2.h,
-                                        &SbApiMsg2.h);
-        if (NT_SUCCESS(Status)) Status = SbApiMsg2.ReturnValue;
+                                        &SbApiMsg.h,
+                                        &SbApiMsg.h);
+        if (NT_SUCCESS(Status)) Status = SbApiMsg.ReturnValue;
         if (!NT_SUCCESS(Status))
         {
             /* Delete the session and handle failure if the LPC call failed */
             SmpDeleteSession(CreateSession->SessionId);
             DPRINT1("SMSS: SmpLoadSubSystem - NtRequestWaitReplyPort Failed with 
Status %lx for sessionid %lu\n",
                     Status,
-                    CreateSession->SessionId);
+                    MuSessionId);
             goto Quickie;
         }
     }
diff --git a/sdk/include/reactos/subsys/sm/smmsg.h
b/sdk/include/reactos/subsys/sm/smmsg.h
index ada3ae670bc..50601e38409 100644
--- a/sdk/include/reactos/subsys/sm/smmsg.h
+++ b/sdk/include/reactos/subsys/sm/smmsg.h
@@ -159,9 +159,9 @@ typedef struct _SB_CREATE_SESSION_MSG
 {
     ULONG SessionId;
     RTL_USER_PROCESS_INFORMATION ProcessInfo;
-    PVOID Unknown;
-    ULONG MuSessionId;
-    CLIENT_ID ClientId;
+    PVOID Reserved;
+    ULONG DbgSessionId;
+    CLIENT_ID DbgUiClientId;
 } SB_CREATE_SESSION_MSG, *PSB_CREATE_SESSION_MSG;
 #ifndef _WIN64
 C_ASSERT(sizeof(SB_CREATE_SESSION_MSG) == 0x58);