https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b3c55b9e6c9b0f62142f7d...
commit b3c55b9e6c9b0f62142f7dbe3e3059f3f81c7f0d Author: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org AuthorDate: Sun Mar 20 22:55:06 2022 +0100 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org CommitDate: Wed May 17 17:40:37 2023 +0200
[NTOS:LPC] Ensure debug-traced pointer-given user-mode data is captured. (#4399)
CORE-18098 --- ntoskrnl/lpc/connect.c | 64 ++++++++++++++++++++++++++++++++++---------------- ntoskrnl/lpc/create.c | 21 ++++++++++++----- ntoskrnl/lpc/send.c | 27 +++++++++++---------- 3 files changed, 74 insertions(+), 38 deletions(-)
diff --git a/ntoskrnl/lpc/connect.c b/ntoskrnl/lpc/connect.c index d1f2426ef73..69dff778ab4 100644 --- a/ntoskrnl/lpc/connect.c +++ b/ntoskrnl/lpc/connect.c @@ -90,6 +90,9 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, NTSTATUS Status = STATUS_SUCCESS; KPROCESSOR_MODE PreviousMode = KeGetPreviousMode(); PETHREAD Thread = PsGetCurrentThread(); +#if DBG + UNICODE_STRING CapturedPortName; +#endif SECURITY_QUALITY_OF_SERVICE CapturedQos; PORT_VIEW CapturedClientView; PSID CapturedServerSid; @@ -105,13 +108,6 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, PTOKEN_USER TokenUserInfo;
PAGED_CODE(); - LPCTRACE(LPC_CONNECT_DEBUG, - "Name: %wZ. SecurityQos: %p. Views: %p/%p. Sid: %p\n", - PortName, - SecurityQos, - ClientView, - ServerView, - ServerSid);
/* Check if the call comes from user mode */ if (PreviousMode != KernelMode) @@ -141,7 +137,6 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, /* Invalid size */ _SEH2_YIELD(return STATUS_INVALID_PARAMETER); } - }
/* Capture the server view */ @@ -172,7 +167,7 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, ProbeForWrite(ConnectionInformation, ConnectionInfoLength, sizeof(ULONG));
CapturedServerSid = ServerSid; - if (ServerSid != NULL) + if (ServerSid) { /* Capture it */ Status = SepCaptureSid(ServerSid, @@ -231,6 +226,21 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, CapturedServerSid = ServerSid; }
+#if DBG + /* Capture the port name for DPRINT only - ObReferenceObjectByName does + * its own capture. As it is used only for debugging, ignore any failure; + * the string is zeroed out in such case. */ + ProbeAndCaptureUnicodeString(&CapturedPortName, PreviousMode, PortName); + + LPCTRACE(LPC_CONNECT_DEBUG, + "Name: %wZ. SecurityQos: %p. Views: %p/%p. Sid: %p\n", + &CapturedPortName, + SecurityQos, + ClientView, + ServerView, + ServerSid); +#endif + /* Get the port */ Status = ObReferenceObjectByName(PortName, 0, @@ -242,7 +252,10 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, (PVOID*)&Port); if (!NT_SUCCESS(Status)) { - DPRINT1("Failed to reference port '%wZ': 0x%lx\n", PortName, Status); +#if DBG + DPRINT1("Failed to reference port '%wZ': 0x%lx\n", &CapturedPortName, Status); + ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode); +#endif
if (CapturedServerSid != ServerSid) SepReleaseSid(CapturedServerSid, PreviousMode, TRUE); @@ -253,7 +266,10 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, /* This has to be a connection port */ if ((Port->Flags & LPCP_PORT_TYPE_MASK) != LPCP_CONNECTION_PORT) { - DPRINT1("Port '%wZ' is not a connection port (Flags: 0x%lx)\n", PortName, Port->Flags); +#if DBG + DPRINT1("Port '%wZ' is not a connection port (Flags: 0x%lx)\n", &CapturedPortName, Port->Flags); + ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode); +#endif
/* It isn't, so fail */ ObDereferenceObject(Port); @@ -282,7 +298,9 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, if (!RtlEqualSid(CapturedServerSid, TokenUserInfo->User.Sid)) { /* Fail */ - DPRINT1("Port '%wZ': server SID mismatch\n", PortName); +#if DBG + DPRINT1("Port '%wZ': server SID mismatch\n", &CapturedPortName); +#endif Status = STATUS_SERVER_SID_MISMATCH; }
@@ -293,21 +311,27 @@ NtSecureConnectPort(OUT PHANDLE PortHandle, else { /* Invalid SID */ - DPRINT1("Port '%wZ': server SID mismatch\n", PortName); +#if DBG + DPRINT1("Port '%wZ': server SID mismatch\n", &CapturedPortName); +#endif Status = STATUS_SERVER_SID_MISMATCH; }
/* Finally release the captured SID, we don't need it anymore */ if (CapturedServerSid != ServerSid) SepReleaseSid(CapturedServerSid, PreviousMode, TRUE); + }
- /* Check if SID failed */ - if (!NT_SUCCESS(Status)) - { - /* Quit */ - ObDereferenceObject(Port); - return Status; - } +#if DBG + ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode); +#endif + + /* Check if SID failed */ + if (ServerSid && !NT_SUCCESS(Status)) + { + /* Quit */ + ObDereferenceObject(Port); + return Status; }
/* Create the client port */ diff --git a/ntoskrnl/lpc/create.c b/ntoskrnl/lpc/create.c index aaa5ed70518..7c017b6b7e1 100644 --- a/ntoskrnl/lpc/create.c +++ b/ntoskrnl/lpc/create.c @@ -50,11 +50,13 @@ LpcpCreatePort(OUT PHANDLE PortHandle, NTSTATUS Status; KPROCESSOR_MODE PreviousMode = KeGetPreviousMode(); UNICODE_STRING CapturedObjectName, *ObjectName; +#if DBG + UNICODE_STRING CapturedPortName; +#endif PLPCP_PORT_OBJECT Port; HANDLE Handle;
PAGED_CODE(); - LPCTRACE(LPC_CREATE_DEBUG, "Name: %wZ\n", ObjectAttributes->ObjectName);
RtlInitEmptyUnicodeString(&CapturedObjectName, NULL, 0);
@@ -70,10 +72,7 @@ LpcpCreatePort(OUT PHANDLE PortHandle, ProbeForRead(ObjectAttributes, sizeof(*ObjectAttributes), sizeof(ULONG)); ObjectName = ((volatile OBJECT_ATTRIBUTES*)ObjectAttributes)->ObjectName; if (ObjectName) - { - ProbeForRead(ObjectName, sizeof(*ObjectName), 1); - CapturedObjectName = *(volatile UNICODE_STRING*)ObjectName; - } + CapturedObjectName = ProbeForReadUnicodeString(ObjectName); } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) { @@ -88,10 +87,20 @@ LpcpCreatePort(OUT PHANDLE PortHandle, CapturedObjectName = *(ObjectAttributes->ObjectName); }
- /* Normalize the buffer pointer in case we don't have a name */ + /* Normalize the buffer pointer in case we don't have + * a name, for initializing an unconnected port. */ if (CapturedObjectName.Length == 0) CapturedObjectName.Buffer = NULL;
+#if DBG + /* Capture the port name for DPRINT only - ObCreateObject does its + * own capture. As it is used only for debugging, ignore any failure; + * the string is zeroed out in such case. */ + ProbeAndCaptureUnicodeString(&CapturedPortName, PreviousMode, &CapturedObjectName); + LPCTRACE(LPC_CREATE_DEBUG, "Name: %wZ\n", &CapturedPortName); + ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode); +#endif + /* Create the Object */ Status = ObCreateObject(PreviousMode, LpcPortObjectType, diff --git a/ntoskrnl/lpc/send.c b/ntoskrnl/lpc/send.c index d6b22063014..317ad1e4563 100644 --- a/ntoskrnl/lpc/send.c +++ b/ntoskrnl/lpc/send.c @@ -449,11 +449,6 @@ NtRequestPort(IN HANDLE PortHandle, PLPCP_MESSAGE Message;
PAGED_CODE(); - LPCTRACE(LPC_SEND_DEBUG, - "Handle: %p. Message: %p. Type: %lx\n", - PortHandle, - LpcRequest, - LpcpGetMessageType(LpcRequest));
/* Check if the call comes from user mode */ if (PreviousMode != KernelMode) @@ -476,6 +471,12 @@ NtRequestPort(IN HANDLE PortHandle, CapturedLpcRequest = *LpcRequest; }
+ LPCTRACE(LPC_SEND_DEBUG, + "Handle: %p. Message: %p. Type: %lx\n", + PortHandle, + LpcRequest, + LpcpGetMessageType(&CapturedLpcRequest)); + /* Get the message type */ MessageType = CapturedLpcRequest.u2.s2.Type | LPC_DATAGRAM;
@@ -709,15 +710,10 @@ NtRequestWaitReplyPort(IN HANDLE PortHandle, PLPCP_DATA_INFO DataInfo;
PAGED_CODE(); - LPCTRACE(LPC_SEND_DEBUG, - "Handle: %p. Messages: %p/%p. Type: %lx\n", - PortHandle, - LpcRequest, - LpcReply, - LpcpGetMessageType(LpcRequest));
/* Check if the thread is dying */ - if (Thread->LpcExitThreadCalled) return STATUS_THREAD_IS_TERMINATING; + if (Thread->LpcExitThreadCalled) + return STATUS_THREAD_IS_TERMINATING;
/* Check for user mode access */ if (PreviousMode != KernelMode) @@ -757,6 +753,13 @@ NtRequestWaitReplyPort(IN HANDLE PortHandle, } }
+ LPCTRACE(LPC_SEND_DEBUG, + "Handle: %p. Messages: %p/%p. Type: %lx\n", + PortHandle, + LpcRequest, + LpcReply, + LpcpGetMessageType(&CapturedLpcRequest)); + /* This flag is undocumented. Remove it before continuing */ CapturedLpcRequest.u2.s2.Type &= ~0x4000;