18 Jan
2019
18 Jan
'19
9:11 p.m.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=65dbfc286818e18095b7c…
commit 65dbfc286818e18095b7cc0c85f63cde6b3e629d
Author: Timo Kreuzer <timo.kreuzer(a)reactos.org>
AuthorDate: Fri Jan 18 22:11:43 2019 +0100
Commit: GitHub <noreply(a)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;
}
2072
days inactive
2072
days old
0 comments
1 participants
participants (1)
-
Timo Kreuzer