[CSRSRV] Only when CSRSRV is compiled in debugging mode, should we display debugging messages and support debug breakpoints.

Also, trigger the less fatal breakpoints only if CSRSS/CSRSRV is being
debugged (the 'BeingDebugged' flag is set in the current PEB). This will
avoid any unhandled breakpoint exceptions when testing/fuzzing running
debug builds of ReactOS without any debugger attached.
This commit is contained in:
Hermès Bélusca-Maïto 2020-04-14 22:53:49 +02:00
parent 37b2c1450c
commit 835f3ef13d
No known key found for this signature in database
GPG key ID: 3B2539C65E7B93D0
2 changed files with 40 additions and 13 deletions

View file

@ -78,16 +78,16 @@ CsrCallServerFromServer(IN PCSR_API_MESSAGE ReceiveMsg,
((ServerDll->ValidTable) && !(ServerDll->ValidTable[ApiId]))) ((ServerDll->ValidTable) && !(ServerDll->ValidTable[ApiId])))
{ {
/* We are beyond the Maximum API ID, or it doesn't exist */ /* We are beyond the Maximum API ID, or it doesn't exist */
DPRINT1("API: %d\n", ApiId);
#ifdef CSR_DBG #ifdef CSR_DBG
DPRINT1("API: %d\n", ApiId);
DPRINT1("CSRSS: %lx (%s) is invalid ApiTableIndex for %Z or is an " DPRINT1("CSRSS: %lx (%s) is invalid ApiTableIndex for %Z or is an "
"invalid API to call from the server.\n", "invalid API to call from the server.\n",
ApiId, ApiId,
((ServerDll->NameTable) && (ServerDll->NameTable[ApiId])) ? ((ServerDll->NameTable) && (ServerDll->NameTable[ApiId])) ?
ServerDll->NameTable[ApiId] : "*** UNKNOWN ***", ServerDll->NameTable[ApiId] : "*** UNKNOWN ***",
&ServerDll->Name); &ServerDll->Name);
if (NtCurrentPeb()->BeingDebugged) DbgBreakPoint();
#endif #endif
// DbgBreakPoint();
ReplyMsg->Status = STATUS_ILLEGAL_FUNCTION; ReplyMsg->Status = STATUS_ILLEGAL_FUNCTION;
return STATUS_ILLEGAL_FUNCTION; return STATUS_ILLEGAL_FUNCTION;
} }
@ -189,6 +189,7 @@ CsrApiHandleConnectionRequest(IN PCSR_API_MESSAGE ApiMessage)
ConnectInfo->ServerProcessId = NtCurrentTeb()->ClientId.UniqueProcess; ConnectInfo->ServerProcessId = NtCurrentTeb()->ClientId.UniqueProcess;
/* Accept the Connection */ /* Accept the Connection */
ASSERT(!AllowConnection || (AllowConnection && CsrProcess));
Status = NtAcceptConnectPort(&ServerPort, Status = NtAcceptConnectPort(&ServerPort,
AllowConnection ? UlongToPtr(CsrProcess->SequenceNumber) : 0, AllowConnection ? UlongToPtr(CsrProcess->SequenceNumber) : 0,
&ApiMessage->Header, &ApiMessage->Header,
@ -382,6 +383,7 @@ CsrApiRequestThread(IN PVOID Parameter)
/* Make sure the real CID is set */ /* Make sure the real CID is set */
Teb->RealClientId = Teb->ClientId; Teb->RealClientId = Teb->ClientId;
#ifdef CSR_DBG
/* Debug check */ /* Debug check */
if (Teb->CountOfOwnedCriticalSections) if (Teb->CountOfOwnedCriticalSections)
{ {
@ -391,6 +393,7 @@ CsrApiRequestThread(IN PVOID Parameter)
&ReceiveMsg, ReplyMsg); &ReceiveMsg, ReplyMsg);
DbgBreakPoint(); DbgBreakPoint();
} }
#endif
/* Wait for a message to come through */ /* Wait for a message to come through */
Status = NtReplyWaitReceivePort(ReplyPort, Status = NtReplyWaitReceivePort(ReplyPort,
@ -404,15 +407,17 @@ CsrApiRequestThread(IN PVOID Parameter)
/* Was it a failure or another success code? */ /* Was it a failure or another success code? */
if (!NT_SUCCESS(Status)) if (!NT_SUCCESS(Status))
{ {
#ifdef CSR_DBG
/* Check for specific status cases */ /* Check for specific status cases */
if ((Status != STATUS_INVALID_CID) && if ((Status != STATUS_INVALID_CID) &&
(Status != STATUS_UNSUCCESSFUL) && (Status != STATUS_UNSUCCESSFUL) &&
((Status == STATUS_INVALID_HANDLE) || (ReplyPort == CsrApiPort))) ((Status != STATUS_INVALID_HANDLE) || (ReplyPort == CsrApiPort)))
{ {
/* Notify the debugger */ /* Notify the debugger */
DPRINT1("CSRSS: ReceivePort failed - Status == %X\n", Status); DPRINT1("CSRSS: ReceivePort failed - Status == %X\n", Status);
DPRINT1("CSRSS: ReplyPortHandle %lx CsrApiPort %lx\n", ReplyPort, CsrApiPort); DPRINT1("CSRSS: ReplyPortHandle %lx CsrApiPort %lx\n", ReplyPort, CsrApiPort);
} }
#endif
/* We failed big time, so start out fresh */ /* We failed big time, so start out fresh */
ReplyMsg = NULL; ReplyMsg = NULL;
@ -427,6 +432,9 @@ CsrApiRequestThread(IN PVOID Parameter)
} }
} }
// ASSERT(ReceiveMsg.Header.u1.s1.TotalLength >= sizeof(PORT_MESSAGE));
// ASSERT(ReceiveMsg.Header.u1.s1.TotalLength < sizeof(ReceiveMsg));
/* Use whatever Client ID we got */ /* Use whatever Client ID we got */
Teb->RealClientId = ReceiveMsg.Header.ClientId; Teb->RealClientId = ReceiveMsg.Header.ClientId;
@ -534,9 +542,11 @@ CsrApiRequestThread(IN PVOID Parameter)
(!(ServerDll = CsrLoadedServerDll[ServerId]))) (!(ServerDll = CsrLoadedServerDll[ServerId])))
{ {
/* We are beyond the Maximum Server ID */ /* We are beyond the Maximum Server ID */
#ifdef CSR_DBG
DPRINT1("CSRSS: %lx is invalid ServerDllIndex (%08x)\n", DPRINT1("CSRSS: %lx is invalid ServerDllIndex (%08x)\n",
ServerId, ServerDll); ServerId, ServerDll);
// DbgBreakPoint(); if (NtCurrentPeb()->BeingDebugged) DbgBreakPoint();
#endif
ReplyMsg = NULL; ReplyMsg = NULL;
ReplyPort = CsrApiPort; ReplyPort = CsrApiPort;
@ -736,9 +746,11 @@ CsrApiRequestThread(IN PVOID Parameter)
(!(ServerDll = CsrLoadedServerDll[ServerId]))) (!(ServerDll = CsrLoadedServerDll[ServerId])))
{ {
/* We are beyond the Maximum Server ID */ /* We are beyond the Maximum Server ID */
#ifdef CSR_DBG
DPRINT1("CSRSS: %lx is invalid ServerDllIndex (%08x)\n", DPRINT1("CSRSS: %lx is invalid ServerDllIndex (%08x)\n",
ServerId, ServerDll); ServerId, ServerDll);
// DbgBreakPoint(); if (NtCurrentPeb()->BeingDebugged) DbgBreakPoint();
#endif
ReplyPort = CsrApiPort; ReplyPort = CsrApiPort;
ReplyMsg = &ReceiveMsg; ReplyMsg = &ReceiveMsg;
@ -828,7 +840,10 @@ CsrApiRequestThread(IN PVOID Parameter)
else if (ReplyCode == CsrReplyDeadClient) else if (ReplyCode == CsrReplyDeadClient)
{ {
/* Reply to the death message */ /* Reply to the death message */
NtReplyPort(ReplyPort, &ReplyMsg->Header); NTSTATUS Status2;
Status2 = NtReplyPort(ReplyPort, &ReplyMsg->Header);
if (!NT_SUCCESS(Status2))
DPRINT1("CSRSS: Error while replying to the death message, Status 0x%lx\n", Status2);
/* Reply back to the API port now */ /* Reply back to the API port now */
ReplyMsg = NULL; ReplyMsg = NULL;
@ -1042,7 +1057,8 @@ CsrConnectToUser(VOID)
_SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
{ {
Connected = FALSE; Connected = FALSE;
} _SEH2_END; }
_SEH2_END;
if (!Connected) if (!Connected)
{ {
@ -1133,9 +1149,11 @@ CsrCaptureArguments(IN PCSR_THREAD CsrThread,
(LocalCaptureBuffer->PointerCount * sizeof(PVOID)) > Length) || (LocalCaptureBuffer->PointerCount * sizeof(PVOID)) > Length) ||
(LocalCaptureBuffer->PointerCount > MAXUSHORT)) (LocalCaptureBuffer->PointerCount > MAXUSHORT))
{ {
/* Return failure */ #ifdef CSR_DBG
DPRINT1("*** CSRSS: CaptureBuffer %p has bad length\n", LocalCaptureBuffer); DPRINT1("*** CSRSS: CaptureBuffer %p has bad length\n", LocalCaptureBuffer);
DbgBreakPoint(); if (NtCurrentPeb()->BeingDebugged) DbgBreakPoint();
#endif
/* Return failure */
ApiMessage->Status = STATUS_INVALID_PARAMETER; ApiMessage->Status = STATUS_INVALID_PARAMETER;
_SEH2_YIELD(return FALSE); _SEH2_YIELD(return FALSE);
} }
@ -1186,9 +1204,11 @@ CsrCaptureArguments(IN PCSR_THREAD CsrThread,
} }
else else
{ {
/* Invalid pointer, fail */ #ifdef CSR_DBG
DPRINT1("*** CSRSS: CaptureBuffer MessagePointer outside of ClientView\n"); DPRINT1("*** CSRSS: CaptureBuffer MessagePointer outside of ClientView\n");
DbgBreakPoint(); if (NtCurrentPeb()->BeingDebugged) DbgBreakPoint();
#endif
/* Invalid pointer, fail */
ApiMessage->Status = STATUS_INVALID_PARAMETER; ApiMessage->Status = STATUS_INVALID_PARAMETER;
} }
} }
@ -1375,8 +1395,10 @@ CsrValidateMessageBuffer(IN PCSR_API_MESSAGE ApiMessage,
} }
/* Failure */ /* Failure */
#ifdef CSR_DBG
DPRINT1("CSRSRV: Bad message buffer %p\n", ApiMessage); DPRINT1("CSRSRV: Bad message buffer %p\n", ApiMessage);
DbgBreakPoint(); if (NtCurrentPeb()->BeingDebugged) DbgBreakPoint();
#endif
return FALSE; return FALSE;
} }

View file

@ -945,8 +945,10 @@ CsrImpersonateClient(IN PCSR_THREAD CsrThread)
if (!NT_SUCCESS(Status)) if (!NT_SUCCESS(Status))
{ {
/* Failure */ /* Failure */
#ifdef CSR_DBG
DPRINT1("CSRSS: Can't impersonate client thread - Status = %lx\n", Status); DPRINT1("CSRSS: Can't impersonate client thread - Status = %lx\n", Status);
// if (Status != STATUS_BAD_IMPERSONATION_LEVEL) DbgBreakPoint(); // if (Status != STATUS_BAD_IMPERSONATION_LEVEL) DbgBreakPoint();
#endif
return FALSE; return FALSE;
} }
@ -1337,6 +1339,7 @@ CsrShutdownProcesses(IN PLUID CallerLuid,
} }
else if (Result == CsrShutdownCancelled) else if (Result == CsrShutdownCancelled)
{ {
#ifdef CSR_DBG
/* Check if this was a forced shutdown */ /* Check if this was a forced shutdown */
if (Flags & EWX_FORCE) if (Flags & EWX_FORCE)
{ {
@ -1344,6 +1347,7 @@ CsrShutdownProcesses(IN PLUID CallerLuid,
CsrProcess->ClientId.UniqueProcess, i); CsrProcess->ClientId.UniqueProcess, i);
DbgBreakPoint(); DbgBreakPoint();
} }
#endif
/* Shutdown was cancelled, unlock and exit */ /* Shutdown was cancelled, unlock and exit */
CsrReleaseProcessLock(); CsrReleaseProcessLock();
@ -1365,7 +1369,8 @@ CsrShutdownProcesses(IN PLUID CallerLuid,
} }
/* We've reached the final loop here, so dereference */ /* We've reached the final loop here, so dereference */
if (i == CSR_SERVER_DLL_MAX) CsrLockedDereferenceProcess(CsrProcess); if (i == CSR_SERVER_DLL_MAX)
CsrLockedDereferenceProcess(CsrProcess);
} }
/* Success path */ /* Success path */