From f694d12f0c499819cba854c51017f9c9242a03bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Tue, 25 Jun 2019 20:45:23 +0200 Subject: [PATCH] [NTOS:KE/MM] Some bug-fixes in the bugcheck code. - Introduce the MmMakeKernelResourceSectionWritable() helper for making the kernel resource memory section writable, and use it in KeGetBugMessageText(). Indeed, this latter function patches in place the bugcheck resource message to trim any trailing newlines before displaying the message on screen. See also https://github.com/osresearch/uxen/blob/83bad53/dm/introspection-win7.c#L286 that mentions it too. This fixes bugcheck text display (e.g. the MANUALLY_INITIATED_CRASH one) when using (at least) MSVC-built ReactOS, avoiding a Page-Fault exception during the bugcheck. - Cover KeGetBugMessageText() in SEH since we are accessing kernel resources that could also be corrupted in bugcheck scenarii, and we don't want to further bugcheck. - Fix newline trimming loop. - KiDoBugCheckCallbacks(): * Wrap the bugcheck CallbackRoutine call in SEH. * Add a FIXME concerning the need of further memory validation of CurrentRecord. - Add a FIXME concerning the need to run the bugcheck-reason callbacks with the KbCallbackReserved1 reason, in KeBugCheckWithTf(). Mentioned in http://blog.ptsecurity.com/2012/06/customizing-blue-screen-of-death.html --- ntoskrnl/include/internal/mm.h | 6 ++- ntoskrnl/ke/bug.c | 88 ++++++++++++++++++++++++---------- ntoskrnl/mm/ARM3/sysldr.c | 40 ++++++++++++++++ 3 files changed, 108 insertions(+), 26 deletions(-) diff --git a/ntoskrnl/include/internal/mm.h b/ntoskrnl/include/internal/mm.h index b4d7ad85bcc..da3a720d4dc 100644 --- a/ntoskrnl/include/internal/mm.h +++ b/ntoskrnl/include/internal/mm.h @@ -794,7 +794,7 @@ NTAPI MmDeleteKernelStack(PVOID Stack, BOOLEAN GuiStack); -/* balace.c ******************************************************************/ +/* balance.c *****************************************************************/ INIT_FUNCTION VOID @@ -1370,6 +1370,10 @@ MiInitializeLoadedModuleList( IN PLOADER_PARAMETER_BLOCK LoaderBlock ); +VOID +NTAPI +MmMakeKernelResourceSectionWritable(VOID); + NTSTATUS NTAPI MmLoadSystemImage( diff --git a/ntoskrnl/ke/bug.c b/ntoskrnl/ke/bug.c index f1e7a50376f..a92eb9297b9 100644 --- a/ntoskrnl/ke/bug.c +++ b/ntoskrnl/ke/bug.c @@ -406,47 +406,65 @@ NTAPI KeGetBugMessageText(IN ULONG BugCheckCode, OUT PANSI_STRING OutputString OPTIONAL) { - ULONG i, j; + ULONG i; ULONG IdOffset; - ULONG_PTR MessageEntry; + PMESSAGE_RESOURCE_ENTRY MessageEntry; PCHAR BugCode; - BOOLEAN Result = FALSE; USHORT Length; + BOOLEAN Result = FALSE; /* Make sure we're not bugchecking too early */ if (!KiBugCodeMessages) return Result; + /* + * Globally protect in SEH as we are trying to access data in + * dire situations, and potentially going to patch it (see below). + */ + _SEH2_TRY + { + + /* + * Make the kernel resource section writable, as we are going to manually + * trim the trailing newlines in the bugcheck resource message in place, + * when OutputString is NULL and before displaying it on screen. + */ + MmMakeKernelResourceSectionWritable(); + /* Find the message. This code is based on RtlFindMesssage */ for (i = 0; i < KiBugCodeMessages->NumberOfBlocks; i++) { - /* Check if the ID Matches */ + /* Check if the ID matches */ if ((BugCheckCode >= KiBugCodeMessages->Blocks[i].LowId) && (BugCheckCode <= KiBugCodeMessages->Blocks[i].HighId)) { - /* Get Offset to Entry */ - MessageEntry = KiBugCodeMessages->Blocks[i].OffsetToEntries + - (ULONG_PTR)KiBugCodeMessages; + /* Get offset to entry */ + MessageEntry = (PMESSAGE_RESOURCE_ENTRY) + ((ULONG_PTR)KiBugCodeMessages + KiBugCodeMessages->Blocks[i].OffsetToEntries); IdOffset = BugCheckCode - KiBugCodeMessages->Blocks[i].LowId; - /* Get offset to ID */ - for (j = 0; j < IdOffset; j++) + /* Advance in the entries until finding it */ + while (IdOffset--) { - /* Advance in the Entries */ - MessageEntry += ((PMESSAGE_RESOURCE_ENTRY)MessageEntry)-> - Length; + MessageEntry = (PMESSAGE_RESOURCE_ENTRY) + ((ULONG_PTR)MessageEntry + MessageEntry->Length); } - /* Get the final Code */ - BugCode = (PCHAR)((PMESSAGE_RESOURCE_ENTRY)MessageEntry)->Text; + /* Make sure it's not Unicode */ + ASSERT(!(MessageEntry->Flags & MESSAGE_RESOURCE_UNICODE)); + + /* Get the final code */ + BugCode = (PCHAR)MessageEntry->Text; Length = (USHORT)strlen(BugCode); /* Handle trailing newlines */ - while ((Length > 0) && ((BugCode[Length] == '\n') || - (BugCode[Length] == '\r') || - (BugCode[Length] == ANSI_NULL))) + while ((Length > 0) && ((BugCode[Length - 1] == '\n') || + (BugCode[Length - 1] == '\r') || + (BugCode[Length - 1] == ANSI_NULL))) { - /* Check if we have a string to return */ - if (!OutputString) BugCode[Length] = ANSI_NULL; + /* Directly trim the newline in place if we don't return the string */ + if (!OutputString) BugCode[Length - 1] = ANSI_NULL; + + /* Skip the trailing newline */ Length--; } @@ -455,12 +473,12 @@ KeGetBugMessageText(IN ULONG BugCheckCode, { /* Return it in the OutputString */ OutputString->Buffer = BugCode; - OutputString->Length = Length + 1; - OutputString->MaximumLength = Length + 1; + OutputString->Length = Length; + OutputString->MaximumLength = Length; } else { - /* Direct Output to Screen */ + /* Direct output to screen */ InbvDisplayString(BugCode); InbvDisplayString("\r"); } @@ -471,6 +489,12 @@ KeGetBugMessageText(IN ULONG BugCheckCode, } } + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + } + _SEH2_END; + /* Return the result */ return Result; } @@ -499,6 +523,8 @@ KiDoBugCheckCallbacks(VOID) Entry); /* Validate it */ + // TODO/FIXME: Check whether the memory CurrentRecord points to + // is still accessible and valid! if (CurrentRecord->Entry.Blink != LastEntry) return; Checksum = (ULONG_PTR)CurrentRecord->CallbackRoutine; Checksum += (ULONG_PTR)CurrentRecord->Buffer; @@ -511,9 +537,17 @@ KiDoBugCheckCallbacks(VOID) { /* Call the routine */ CurrentRecord->State = BufferStarted; - (CurrentRecord->CallbackRoutine)(CurrentRecord->Buffer, - CurrentRecord->Length); - CurrentRecord->State = BufferFinished; + _SEH2_TRY + { + (CurrentRecord->CallbackRoutine)(CurrentRecord->Buffer, + CurrentRecord->Length); + CurrentRecord->State = BufferFinished; + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + CurrentRecord->State = BufferIncomplete; + } + _SEH2_END; } /* Go to the next entry */ @@ -1145,6 +1179,10 @@ KeBugCheckWithTf(IN ULONG BugCheckCode, HardErrMessage, AnsiName); + // TODO/FIXME: Run the registered reason-callbacks from + // the KeBugcheckReasonCallbackListHead list with the + // KbCallbackReserved1 reason. + /* Check if the debugger is disabled but we can enable it */ if (!(KdDebuggerEnabled) && !(KdPitchDebugger)) { diff --git a/ntoskrnl/mm/ARM3/sysldr.c b/ntoskrnl/mm/ARM3/sysldr.c index 640856620a7..8b34fea69fb 100644 --- a/ntoskrnl/mm/ARM3/sysldr.c +++ b/ntoskrnl/mm/ARM3/sysldr.c @@ -2284,6 +2284,46 @@ MiInitializeLoadedModuleList(IN PLOADER_PARAMETER_BLOCK LoaderBlock) return TRUE; } +VOID +NTAPI +MmMakeKernelResourceSectionWritable(VOID) +{ + PMMPTE PointerPte; + MMPTE TempPte; + + /* Don't do anything if the resource section is already writable */ + if (MiKernelResourceStartPte == NULL || MiKernelResourceEndPte == NULL) + return; + + /* If the resource section is physical, we cannot change its protection */ + if (MI_IS_PHYSICAL_ADDRESS(MiPteToAddress(MiKernelResourceStartPte))) + return; + + /* Loop the PTEs */ + for (PointerPte = MiKernelResourceStartPte; PointerPte <= MiKernelResourceEndPte; PointerPte++) + { + /* Read the PTE */ + TempPte = *PointerPte; + + /* Make sure it's valid */ + ASSERT(TempPte.u.Hard.Valid == 1); + + /* Update the protection */ + MI_MAKE_WRITE_PAGE(&TempPte); + MI_UPDATE_VALID_PTE(PointerPte, TempPte); + } + + /* + * Invalidate the cached resource section PTEs + * so as to not change its protection again later. + */ + MiKernelResourceStartPte = NULL; + MiKernelResourceEndPte = NULL; + + /* Only flush the current processor's TLB */ + KeFlushCurrentTb(); +} + LOGICAL NTAPI MiUseLargeDriverPage(IN ULONG NumberOfPtes,