https://git.reactos.org/?p=reactos.git;a=commitdiff;h=65dbfc286818e18095b7cc...
commit 65dbfc286818e18095b7cc0c85f63cde6b3e629d Author: Timo Kreuzer timo.kreuzer@reactos.org AuthorDate: Fri Jan 18 22:11:43 2019 +0100 Commit: GitHub noreply@github.com CommitDate: Fri Jan 18 22:11:43 2019 +0100
[NTOS:Mm] Rewrite MiWriteProtectSystemImage (#749)
* The previous version was overcomplicated and broken and therefore disabled. * The new version also enforces NX protection on x64. * Now that protecting works, also protect the boot loaded images. --- ntoskrnl/include/internal/amd64/mm.h | 3 +- ntoskrnl/include/internal/arm/mm.h | 1 + ntoskrnl/include/internal/i386/mm.h | 2 + ntoskrnl/include/ntoskrnl.h | 8 + ntoskrnl/mm/ARM3/miarm.h | 42 ++++- ntoskrnl/mm/ARM3/sysldr.c | 294 +++++++++++------------------------ ntoskrnl/mm/mminit.c | 14 ++ 7 files changed, 162 insertions(+), 202 deletions(-)
diff --git a/ntoskrnl/include/internal/amd64/mm.h b/ntoskrnl/include/internal/amd64/mm.h index 8f881a12c2..fbdd92eb3d 100644 --- a/ntoskrnl/include/internal/amd64/mm.h +++ b/ntoskrnl/include/internal/amd64/mm.h @@ -3,7 +3,8 @@ */ #pragma once
-#define _MI_PAGING_LEVELS 4 +#define _MI_PAGING_LEVELS 4 +#define _MI_HAS_NO_EXECUTE 1
/* Memory layout base addresses (This is based on Vista!) */ #define MI_USER_PROBE_ADDRESS (PVOID)0x000007FFFFFF0000ULL diff --git a/ntoskrnl/include/internal/arm/mm.h b/ntoskrnl/include/internal/arm/mm.h index 0e2e79d15c..d5bd2c5066 100644 --- a/ntoskrnl/include/internal/arm/mm.h +++ b/ntoskrnl/include/internal/arm/mm.h @@ -4,6 +4,7 @@ #pragma once
#define _MI_PAGING_LEVELS 2 +#define _MI_HAS_NO_EXECUTE 1
/* Memory layout base addresses */ #define MI_USER_PROBE_ADDRESS (PVOID)0x7FFF0000 diff --git a/ntoskrnl/include/internal/i386/mm.h b/ntoskrnl/include/internal/i386/mm.h index a77b92b9e3..63bce80582 100644 --- a/ntoskrnl/include/internal/i386/mm.h +++ b/ntoskrnl/include/internal/i386/mm.h @@ -5,8 +5,10 @@
#ifdef _PAE_ #define _MI_PAGING_LEVELS 3 +#define _MI_HAS_NO_EXECUTE 1 #else #define _MI_PAGING_LEVELS 2 +#define _MI_NO_EXECUTE 0 #endif
/* Memory layout base addresses */ diff --git a/ntoskrnl/include/ntoskrnl.h b/ntoskrnl/include/ntoskrnl.h index 51516c6bd7..35a243e2a2 100644 --- a/ntoskrnl/include/ntoskrnl.h +++ b/ntoskrnl/include/ntoskrnl.h @@ -94,6 +94,14 @@
#define ExRaiseStatus RtlRaiseStatus
+/* Also defined in fltkernel.h, but we don't want the entire header */ +#ifndef Add2Ptr +#define Add2Ptr(P,I) ((PVOID)((PUCHAR)(P) + (I))) +#endif +#ifndef PtrOffset +#define PtrOffset(B,O) ((ULONG)((ULONG_PTR)(O) - (ULONG_PTR)(B))) +#endif + // // Switch for enabling global page support // diff --git a/ntoskrnl/mm/ARM3/miarm.h b/ntoskrnl/mm/ARM3/miarm.h index 27ff0718ba..b0fa87c4de 100644 --- a/ntoskrnl/mm/ARM3/miarm.h +++ b/ntoskrnl/mm/ARM3/miarm.h @@ -83,7 +83,7 @@ C_ASSERT(SYSTEM_PD_SIZE == PAGE_SIZE); // while on certain architectures such as ARM, it is enabling the cache which // requires a flag. // -#if defined(_M_IX86) || defined(_M_AMD64) +#if defined(_M_IX86) // // Access Flags // @@ -109,6 +109,34 @@ C_ASSERT(SYSTEM_PD_SIZE == PAGE_SIZE); #define PTE_ENABLE_CACHE 0 #define PTE_DISABLE_CACHE 0x10 #define PTE_WRITECOMBINED_CACHE 0x10 +#define PTE_PROTECT_MASK 0x612 +#elif defined(_M_AMD64) +// +// Access Flags +// +#define PTE_READONLY 0x8000000000000000ULL +#define PTE_EXECUTE 0x0000000000000000ULL +#define PTE_EXECUTE_READ PTE_EXECUTE /* EXECUTE implies READ on x64 */ +#define PTE_READWRITE 0x8000000000000002ULL +#define PTE_WRITECOPY 0x8000000000000200ULL +#define PTE_EXECUTE_READWRITE 0x0000000000000002ULL +#define PTE_EXECUTE_WRITECOPY 0x0000000000000200ULL +#define PTE_PROTOTYPE 0x0000000000000400ULL + +// +// State Flags +// +#define PTE_VALID 0x0000000000000001ULL +#define PTE_ACCESSED 0x0000000000000020ULL +#define PTE_DIRTY 0x0000000000000040ULL + +// +// Cache flags +// +#define PTE_ENABLE_CACHE 0x0000000000000000ULL +#define PTE_DISABLE_CACHE 0x0000000000000010ULL +#define PTE_WRITECOMBINED_CACHE 0x0000000000000010ULL +#define PTE_PROTECT_MASK 0x8000000000000612ULL #elif defined(_M_ARM) #define PTE_READONLY 0x200 #define PTE_EXECUTE 0 // Not worrying about NX yet @@ -118,16 +146,23 @@ C_ASSERT(SYSTEM_PD_SIZE == PAGE_SIZE); #define PTE_EXECUTE_READWRITE 0 // Not worrying about NX yet #define PTE_EXECUTE_WRITECOPY 0 // Not worrying about NX yet #define PTE_PROTOTYPE 0x400 // Using the Shared bit + // // Cache flags // #define PTE_ENABLE_CACHE 0 #define PTE_DISABLE_CACHE 0x10 #define PTE_WRITECOMBINED_CACHE 0x10 +#define PTE_PROTECT_MASK 0x610 #else #error Define these please! #endif
+// +// Mask for image section page protection +// +#define IMAGE_SCN_PROTECTION_MASK (IMAGE_SCN_MEM_WRITE | IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_EXECUTE) + extern const ULONG_PTR MmProtectToPteMask[32]; extern const ULONG MmProtectToValue[32];
@@ -2263,6 +2298,11 @@ MiMakePdeExistAndMakeValid( IN KIRQL OldIrql );
+VOID +NTAPI +MiWriteProtectSystemImage( + _In_ PVOID ImageBase); + // // MiRemoveZeroPage will use inline code to zero out the page manually if only // free pages are available. In some scenarios, we don't/can't run that piece of diff --git a/ntoskrnl/mm/ARM3/sysldr.c b/ntoskrnl/mm/ARM3/sysldr.c index 15b267d146..107f3fcf40 100644 --- a/ntoskrnl/mm/ARM3/sysldr.c +++ b/ntoskrnl/mm/ARM3/sysldr.c @@ -2341,252 +2341,146 @@ MiUseLargeDriverPage(IN ULONG NumberOfPtes, return FALSE; }
-ULONG +VOID NTAPI -MiComputeDriverProtection(IN BOOLEAN SessionSpace, - IN ULONG SectionProtection) +MiSetSystemCodeProtection( + _In_ PMMPTE FirstPte, + _In_ PMMPTE LastPte, + _In_ ULONG Protection) { - ULONG Protection = MM_ZERO_ACCESS; + PMMPTE PointerPte; + MMPTE TempPte;
- /* Check if the caller gave anything */ - if (SectionProtection) + /* Loop the PTEs */ + for (PointerPte = FirstPte; PointerPte <= LastPte; PointerPte++) { - /* Always turn on execute access */ - SectionProtection |= IMAGE_SCN_MEM_EXECUTE; + /* Read the PTE */ + TempPte = *PointerPte;
- /* Check if the registry setting is on or not */ - if (!MmEnforceWriteProtection) - { - /* Turn on write access too */ - SectionProtection |= (IMAGE_SCN_MEM_WRITE | IMAGE_SCN_MEM_EXECUTE); - } - } + /* Make sure it's valid */ + ASSERT(TempPte.u.Hard.Valid == 1);
- /* Convert to internal PTE flags */ - if (SectionProtection & IMAGE_SCN_MEM_EXECUTE) Protection |= MM_EXECUTE; - if (SectionProtection & IMAGE_SCN_MEM_READ) Protection |= MM_READONLY; + /* Update the protection */ + TempPte.u.Hard.Write = BooleanFlagOn(Protection, IMAGE_SCN_MEM_WRITE); +#if _MI_HAS_NO_EXECUTE + TempPte.u.Hard.NoExecute = !BooleanFlagOn(Protection, IMAGE_SCN_MEM_EXECUTE); +#endif
- /* Check for write access */ - if (SectionProtection & IMAGE_SCN_MEM_WRITE) - { - /* Session space is not supported */ - if (SessionSpace) - { - DPRINT1("Session drivers not supported\n"); - ASSERT(SessionSpace == FALSE); - } - else - { - /* Convert to internal PTE flag */ - Protection = (Protection & MM_EXECUTE) ? MM_EXECUTE_READWRITE : MM_READWRITE; - } + MI_UPDATE_VALID_PTE(PointerPte, TempPte); }
- /* If there's no access at all by now, convert to internal no access flag */ - if (Protection == MM_ZERO_ACCESS) Protection = MM_NOACCESS; - - /* Return the computed PTE protection */ - return Protection; -} + /* Flush it all */ + KeFlushEntireTb(TRUE, TRUE);
-VOID -NTAPI -MiSetSystemCodeProtection(IN PMMPTE FirstPte, - IN PMMPTE LastPte, - IN ULONG ProtectionMask) -{ - /* I'm afraid to introduce regressions at the moment... */ return; }
VOID NTAPI -MiWriteProtectSystemImage(IN PVOID ImageBase) +MiWriteProtectSystemImage( + _In_ PVOID ImageBase) { PIMAGE_NT_HEADERS NtHeaders; - PIMAGE_SECTION_HEADER Section; - PFN_NUMBER DriverPages; - ULONG CurrentProtection, SectionProtection, CombinedProtection = 0, ProtectionMask; - ULONG Sections, Size; - ULONG_PTR BaseAddress, CurrentAddress; - PMMPTE PointerPte, StartPte, LastPte, CurrentPte, ComboPte = NULL; - ULONG CurrentMask, CombinedMask = 0; - PAGED_CODE(); - - /* No need to write protect physical memory-backed drivers (large pages) */ - if (MI_IS_PHYSICAL_ADDRESS(ImageBase)) return; - - /* Get the image headers */ - NtHeaders = RtlImageNtHeader(ImageBase); - if (!NtHeaders) return; + PIMAGE_SECTION_HEADER SectionHeaders, Section; + ULONG i; + PVOID SectionBase, SectionEnd; + ULONG SectionSize; + ULONG Protection; + PMMPTE FirstPte, LastPte;
- /* Check if this is a session driver or not */ - if (!MI_IS_SESSION_ADDRESS(ImageBase)) - { - /* Don't touch NT4 drivers */ - if (NtHeaders->OptionalHeader.MajorOperatingSystemVersion < 5) return; - if (NtHeaders->OptionalHeader.MajorImageVersion < 5) return; - } - else + /* Check if the registry setting is on or not */ + if (!MmEnforceWriteProtection) { - /* Not supported */ - UNIMPLEMENTED_DBGBREAK("Session drivers not supported\n"); + /* Ignore section protection */ + return; }
- /* These are the only protection masks we care about */ - ProtectionMask = IMAGE_SCN_MEM_WRITE | IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_EXECUTE; + /* Large page mapped images are not supported */ + NT_ASSERT(!MI_IS_PHYSICAL_ADDRESS(ImageBase));
- /* Calculate the number of pages this driver is occupying */ - DriverPages = BYTES_TO_PAGES(NtHeaders->OptionalHeader.SizeOfImage); + /* Session images are not yet supported */ + NT_ASSERT(!MI_IS_SESSION_ADDRESS(ImageBase));
- /* Get the number of sections and the first section header */ - Sections = NtHeaders->FileHeader.NumberOfSections; - ASSERT(Sections != 0); - Section = IMAGE_FIRST_SECTION(NtHeaders); + /* Get the NT headers */ + NtHeaders = RtlImageNtHeader(ImageBase); + if (NtHeaders == NULL) + { + DPRINT1("Failed to get NT headers for image @ %p\n", ImageBase); + return; + }
- /* Loop all the sections */ - CurrentAddress = (ULONG_PTR)ImageBase; - while (Sections) + /* Don't touch NT4 drivers */ + if ((NtHeaders->OptionalHeader.MajorOperatingSystemVersion < 5) || + (NtHeaders->OptionalHeader.MajorSubsystemVersion < 5)) { - /* Get the section size */ - Size = max(Section->SizeOfRawData, Section->Misc.VirtualSize); + DPRINT1("Skipping NT 4 driver @ %p\n", ImageBase); + return; + }
- /* Get its virtual address */ - BaseAddress = (ULONG_PTR)ImageBase + Section->VirtualAddress; - if (BaseAddress < CurrentAddress) - { - /* Windows doesn't like these */ - DPRINT1("Badly linked image!\n"); - return; - } + /* Get the section headers */ + SectionHeaders = IMAGE_FIRST_SECTION(NtHeaders);
- /* Remember the current address */ - CurrentAddress = BaseAddress + Size - 1; + /* Get the base address of the first section */ + SectionBase = Add2Ptr(ImageBase, SectionHeaders[0].VirtualAddress);
- /* Next */ - Sections--; - Section++; + /* Start protecting the image header as R/O */ + FirstPte = MiAddressToPte(ImageBase); + LastPte = MiAddressToPte(SectionBase) - 1; + Protection = IMAGE_SCN_MEM_READ; + if (LastPte >= FirstPte) + { + MiSetSystemCodeProtection(FirstPte, LastPte, IMAGE_SCN_MEM_READ); }
- /* Get the number of sections and the first section header */ - Sections = NtHeaders->FileHeader.NumberOfSections; - ASSERT(Sections != 0); - Section = IMAGE_FIRST_SECTION(NtHeaders); - - /* Set the address at the end to initialize the loop */ - CurrentAddress = (ULONG_PTR)Section + Sections - 1; - CurrentProtection = IMAGE_SCN_MEM_WRITE | IMAGE_SCN_MEM_READ; - - /* Set the PTE points for the image, and loop its sections */ - StartPte = MiAddressToPte(ImageBase); - LastPte = StartPte + DriverPages; - while (Sections) + /* Loop the sections */ + for (i = 0; i < NtHeaders->FileHeader.NumberOfSections; i++) { - /* Get the section size */ - Size = max(Section->SizeOfRawData, Section->Misc.VirtualSize); + /* Get the section base address and size */ + Section = &SectionHeaders[i]; + SectionBase = Add2Ptr(ImageBase, Section->VirtualAddress); + SectionSize = max(Section->SizeOfRawData, Section->Misc.VirtualSize);
- /* Get its virtual address and PTE */ - BaseAddress = (ULONG_PTR)ImageBase + Section->VirtualAddress; - PointerPte = MiAddressToPte(BaseAddress); + /* Get the first PTE of this section */ + FirstPte = MiAddressToPte(SectionBase);
- /* Check if we were already protecting a run, and found a new run */ - if ((ComboPte) && (PointerPte > ComboPte)) + /* Check for overlap with the previous range */ + if (FirstPte == LastPte) { - /* Compute protection */ - CombinedMask = MiComputeDriverProtection(FALSE, CombinedProtection); - - /* Set it */ - MiSetSystemCodeProtection(ComboPte, ComboPte, CombinedMask); - - /* Check for overlap */ - if (ComboPte == StartPte) StartPte++; + /* Combine the old and new protection by ORing them */ + Protection |= (Section->Characteristics & IMAGE_SCN_PROTECTION_MASK);
- /* One done, reset variables */ - ComboPte = NULL; - CombinedProtection = 0; - } - - /* Break out when needed */ - if (PointerPte >= LastPte) break; + /* Update the protection for this PTE */ + MiSetSystemCodeProtection(FirstPte, FirstPte, Protection);
- /* Get the requested protection from the image header */ - SectionProtection = Section->Characteristics & ProtectionMask; - if (SectionProtection == CurrentProtection) - { - /* Same protection, so merge the request */ - CurrentAddress = BaseAddress + Size - 1; - - /* Next */ - Sections--; - Section++; - continue; + /* Skip this PTE */ + FirstPte++; }
- /* This is now a new section, so close up the old one */ - CurrentPte = MiAddressToPte(CurrentAddress); - - /* Check for overlap */ - if (CurrentPte == PointerPte) - { - /* Skip the last PTE, since it overlaps with us */ - CurrentPte--; - - /* And set the PTE we will merge with */ - ASSERT((ComboPte == NULL) || (ComboPte == PointerPte)); - ComboPte = PointerPte; + /* There can not be gaps! */ + NT_ASSERT(FirstPte == (LastPte + 1));
- /* Get the most flexible protection by merging both */ - CombinedMask |= (SectionProtection | CurrentProtection); - } + /* Get the end of the section and the last PTE */ + SectionEnd = Add2Ptr(SectionBase, SectionSize - 1); + NT_ASSERT(SectionEnd < Add2Ptr(ImageBase, NtHeaders->OptionalHeader.SizeOfImage)); + LastPte = MiAddressToPte(SectionEnd);
- /* Loop any PTEs left */ - if (CurrentPte >= StartPte) + /* If there are no more pages (after an overlap), skip this section */ + if (LastPte < FirstPte) { - /* Sanity check */ - ASSERT(StartPte < LastPte); - - /* Make sure we don't overflow past the last PTE in the driver */ - if (CurrentPte >= LastPte) CurrentPte = LastPte - 1; - ASSERT(CurrentPte >= StartPte); - - /* Compute the protection and set it */ - CurrentMask = MiComputeDriverProtection(FALSE, CurrentProtection); - MiSetSystemCodeProtection(StartPte, CurrentPte, CurrentMask); + NT_ASSERT(FirstPte == (LastPte + 1)); + continue; }
- /* Set new state */ - StartPte = PointerPte; - CurrentAddress = BaseAddress + Size - 1; - CurrentProtection = SectionProtection; + /* Get the section protection */ + Protection = (Section->Characteristics & IMAGE_SCN_PROTECTION_MASK);
- /* Next */ - Sections--; - Section++; - } - - /* Is there a leftover section to merge? */ - if (ComboPte) - { - /* Compute and set the protection */ - CombinedMask = MiComputeDriverProtection(FALSE, CombinedProtection); - MiSetSystemCodeProtection(ComboPte, ComboPte, CombinedMask); - - /* Handle overlap */ - if (ComboPte == StartPte) StartPte++; + /* Update the protection for this section */ + MiSetSystemCodeProtection(FirstPte, LastPte, Protection); }
- /* Finally, handle the last section */ - CurrentPte = MiAddressToPte(CurrentAddress); - if ((StartPte < LastPte) && (CurrentPte >= StartPte)) - { - /* Handle overlap */ - if (CurrentPte >= LastPte) CurrentPte = LastPte - 1; - ASSERT(CurrentPte >= StartPte); - - /* Compute and set the protection */ - CurrentMask = MiComputeDriverProtection(FALSE, CurrentProtection); - MiSetSystemCodeProtection(StartPte, CurrentPte, CurrentMask); - } + /* Image should end with the last section */ + NT_ASSERT(ALIGN_UP_POINTER_BY(SectionEnd, PAGE_SIZE) == + Add2Ptr(ImageBase, NtHeaders->OptionalHeader.SizeOfImage)); }
VOID diff --git a/ntoskrnl/mm/mminit.c b/ntoskrnl/mm/mminit.c index 4f328ab65c..7c0bbd4efd 100644 --- a/ntoskrnl/mm/mminit.c +++ b/ntoskrnl/mm/mminit.c @@ -203,6 +203,8 @@ MmInitSystem(IN ULONG Phase, PMMPTE PointerPte; MMPTE TempPte = ValidKernelPte; PFN_NUMBER PageFrameNumber; + PLIST_ENTRY ListEntry; + PLDR_DATA_TABLE_ENTRY DataTableEntry;
/* Initialize the kernel address space */ ASSERT(Phase == 1); @@ -271,6 +273,18 @@ MmInitSystem(IN ULONG Phase, /* Initialize the balance set manager */ MmInitBsmThread();
+ /* Loop the boot loaded images */ + for (ListEntry = PsLoadedModuleList.Flink; + ListEntry != &PsLoadedModuleList; + ListEntry = ListEntry->Flink) + { + /* Get the data table entry */ + DataTableEntry = CONTAINING_RECORD(ListEntry, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks); + + /* Set up the image protection */ + MiWriteProtectSystemImage(DataTableEntry->DllBase); + } + return TRUE; }
-
Timo Kreuzer