[tkreuzer] 61095: [NTOSKRNL] Windows / ReactOS uses a software protection field called protection mask, which is stored inside invalid (Software) PTEs to provide information about the desired prote...
Author: tkreuzer Date: Mon Nov 25 00:18:33 2013 New Revision: 61095
URL: http://svn.reactos.org/svn/reactos?rev=61095&view=rev Log: [NTOSKRNL] Windows / ReactOS uses a software protection field called protection mask, which is stored inside invalid (Software) PTEs to provide information about the desired protection, when a page is made valid by the page fault handler. The mask consists of the values 0-7 specifying the read/write/execute rights, 0 being inaccessible aka MM_ZERO_ACCESS, plus 2 flag-like bits, for uncached and writecombine memory respectively. Both flags together don't make sense, so this combination is used to mark guard pages. Since all these flags only make sense when used together with a proper access (i.e. not MM_ZERO_ACCESS), the combination of these flags together with MM_ZERO_ACCESS was given special meaning: MM_DECOMMIT, which equals MM_GUARDPAGE | MM_ZERO_ACCESS is for decommitted pages, that are not yet erased to zero, MM_NOACCESS, which is the mask for pages that are mapped with PAGE_NOACCESS (this is to make sure that a software PTE of a committed page is never completely 0, which it could be, when MM_ZERO_ACCESS was used), and finally MM_OUTSWAPPED_KSTACK for outswapped kernel stacks. See also https://www.reactos.org/wiki/Techwiki:Memory_Protection_constants. The next thing to know is that the number of PTEs that are not null is counted for each PDE. So once a page gets committed, a software PTE is written and the reference count is incremented. When the page is made valid by the fault handler, the count is not changed, when the page is decommitted, the MM_DECOMMIT software PTE is written and again the PTE stays non-null and nothing is changed. Only when the range is cleaned up totally, the PTEs get erased and the reference count is decremented. Now it happened that our page fault handler missed to validate the access rights of protection constants. The problem that came up with this is a major one: since a decommitted page is a software PTE with MM_DECOMMIT as the protection mask (which we remember has the MM_GUARDPAGE bit set), the fault handler considered faults on decommitted PTEs as faults on guard pages and simply removed the guard page flag, leaving a completely empty PTE behind! So the decommitted page got erased without decrementing the reference count. This lead to CORE-7445. - Add protection flags (MM_GUARDPAGE, MM_WRITECOMBINE, MM_OUTSWAPPED_KSTACK) - Instead of writing 0 to a PTE, use MI_WRITE_INVALID_PTE with MmZeroPte - Implement MiIsAccessAllowed that checks for read/write/execute access and use it in MiAccessCheck - Add some more ASSERTs CORE-7445 #resolve
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c trunk/reactos/ntoskrnl/mm/ARM3/section.c trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/miarm.h?re... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] Mon Nov 25 00:18:33 2013 @@ -96,6 +96,7 @@ // // Protection Bits part of the internal memory manager Protection Mask, from: // http://reactos.org/wiki/Techwiki:Memory_management_in_the_Windows_XP_kernel +// https://www.reactos.org/wiki/Techwiki:Memory_Protection_constants // and public assertions. // #define MM_ZERO_ACCESS 0 @@ -106,9 +107,20 @@ #define MM_WRITECOPY 5 #define MM_EXECUTE_READWRITE 6 #define MM_EXECUTE_WRITECOPY 7 -#define MM_NOCACHE 8 -#define MM_DECOMMIT 0x10 -#define MM_NOACCESS (MM_DECOMMIT | MM_NOCACHE) + +// +// These are flags on top of the actual protection mask +// +#define MM_NOCACHE 0x08 +#define MM_GUARDPAGE 0x10 +#define MM_WRITECOMBINE 0x18 + +// +// These are special cases +// +#define MM_DECOMMIT (MM_ZERO_ACCESS | MM_GUARDPAGE) +#define MM_NOACCESS (MM_ZERO_ACCESS | MM_WRITECOMBINE) +#define MM_OUTSWAPPED_KSTACK (MM_EXECUTE_WRITECOPY | MM_WRITECOMBINE) #define MM_INVALID_PROTECTION 0xFFFFFFFF
// @@ -576,6 +588,7 @@ LONG ImageLoadingCount; } MM_SESSION_SPACE, *PMM_SESSION_SPACE;
+static const MMPTE MmZeroPte = {{0}}; extern PMM_SESSION_SPACE MmSessionSpace; extern MMPTE HyperTemplatePte; extern MMPDE ValidKernelPde;
Modified: trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] Mon Nov 25 00:18:33 2013 @@ -112,12 +112,41 @@ return STATUS_STACK_OVERFLOW; }
+FORCEINLINE +BOOLEAN +MiIsAccessAllowed( + _In_ ULONG ProtectionMask, + _In_ BOOLEAN Write, + _In_ BOOLEAN Execute) +{ + #define _BYTE_MASK(Bit0, Bit1, Bit2, Bit3, Bit4, Bit5, Bit6, Bit7) \ + (Bit0) | ((Bit1) << 1) | ((Bit2) << 2) | ((Bit3) << 3) | \ + ((Bit4) << 4) | ((Bit5) << 5) | ((Bit6) << 6) | ((Bit7) << 7) + static const UCHAR MiAccessAllowedMask[2][2] = + { + { // Protect 0 1 2 3 4 5 6 7 + _BYTE_MASK(0, 1, 1, 1, 1, 1, 1, 1), // READ + _BYTE_MASK(0, 0, 1, 1, 0, 0, 1, 1), // EXECUTE READ + }, + { + _BYTE_MASK(0, 0, 0, 0, 1, 1, 1, 1), // WRITE + _BYTE_MASK(0, 0, 0, 0, 0, 0, 1, 1), // EXECUTE WRITE + } + }; + + /* We want only the low 3 bits */ + ProtectionMask &= 7; + + /* Look it up in the table */ + return (MiAccessAllowedMask[Write != 0][Execute != 0] >> ProtectionMask) & 1; +} + NTSTATUS NTAPI MiAccessCheck(IN PMMPTE PointerPte, IN BOOLEAN StoreInstruction, IN KPROCESSOR_MODE PreviousMode, - IN ULONG_PTR ProtectionCode, + IN ULONG_PTR ProtectionMask, IN PVOID TrapFrame, IN BOOLEAN LockHeld) { @@ -151,20 +180,17 @@ return STATUS_SUCCESS; }
- /* Convert any fault flag to 1 only */ - if (StoreInstruction) StoreInstruction = 1; - -#if 0 /* Check if the protection on the page allows what is being attempted */ - if ((MmReadWrite[Protection] - StoreInstruction) < 10) + if (!MiIsAccessAllowed(ProtectionMask, StoreInstruction, FALSE)) { return STATUS_ACCESS_VIOLATION; } -#endif
/* Check if this is a guard page */ - if (ProtectionCode & MM_DECOMMIT) - { + if (ProtectionMask & MM_GUARDPAGE) + { + NT_ASSERT(ProtectionMask != MM_DECOMMIT); + /* Attached processes can't expand their stack */ if (KeIsAttachedProcess()) return STATUS_ACCESS_VIOLATION;
@@ -173,7 +199,9 @@ (TempPte.u.Soft.Prototype == 0)) == FALSE);
/* Remove the guard page bit, and return a guard page violation */ - PointerPte->u.Soft.Protection = ProtectionCode & ~MM_DECOMMIT; + TempPte.u.Soft.Protection = ProtectionMask & ~MM_GUARDPAGE; + NT_ASSERT(TempPte.u.Long != 0); + MI_WRITE_INVALID_PTE(PointerPte, TempPte); return STATUS_GUARD_PAGE_VIOLATION; }
@@ -950,6 +978,9 @@ KeReleaseQueuedSpinLock(LockQueuePfnLock, OldIrql); return STATUS_ACCESS_VIOLATION; } + + /* There is no such thing as a decommitted prototype PTE */ + NT_ASSERT(TempPte.u.Long != MmDecommittedPte.u.Long);
/* Check for access rights on the PTE proper */ PteContents = *PointerPte; @@ -1863,10 +1894,14 @@ }
/* Is this a guard page? */ - if (ProtectionCode & MM_DECOMMIT) - { + if (ProtectionCode & MM_GUARDPAGE) + { + /* The VAD protection cannot be MM_DECOMMIT! */ + NT_ASSERT(ProtectionCode != MM_DECOMMIT); + /* Remove the bit */ - PointerPte->u.Soft.Protection = ProtectionCode & ~MM_DECOMMIT; + TempPte.u.Soft.Protection = ProtectionCode & ~MM_GUARDPAGE; + MI_WRITE_INVALID_PTE(PointerPte, TempPte);
/* Not supported */ ASSERT(ProtoPte == NULL); @@ -1892,7 +1927,8 @@ else { /* No, create a new PTE. First, write the protection */ - PointerPte->u.Soft.Protection = ProtectionCode; + TempPte.u.Soft.Protection = ProtectionCode; + MI_WRITE_INVALID_PTE(PointerPte, TempPte); }
/* Lock the PFN database since we're going to grab a page */ @@ -1972,6 +2008,7 @@ /* Write the prototype PTE */ TempPte = PrototypePte; TempPte.u.Soft.Protection = ProtectionCode; + NT_ASSERT(TempPte.u.Long != 0); MI_WRITE_INVALID_PTE(PointerPte, TempPte); } else
Modified: trunk/reactos/ntoskrnl/mm/ARM3/section.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/section.c?... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/section.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/section.c [iso-8859-1] Mon Nov 25 00:18:33 2013 @@ -198,7 +198,7 @@ }
/* This actually turns on guard page in this scenario! */ - ProtectMask |= MM_DECOMMIT; + ProtectMask |= MM_GUARDPAGE; }
/* Check for nocache option */
Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c?... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] Mon Nov 25 00:18:33 2013 @@ -350,7 +350,7 @@ KeReleaseQueuedSpinLock(LockQueuePfnLock, OldIrql);
/* Destroy the PTE */ - PointerPte->u.Long = 0; + MI_WRITE_INVALID_PTE(PointerPte, MmZeroPte); }
/* Actual legitimate pages */ @@ -368,7 +368,7 @@ ASSERT(PointerPte->u.Soft.PageFileHigh == 0);
/* Destroy the PTE */ - PointerPte->u.Long = 0; + MI_WRITE_INVALID_PTE(PointerPte, MmZeroPte); }
/* Keep going */ @@ -486,7 +486,7 @@ }
/* Destroy the PTE and flush the TLB */ - PointerPte->u.Long = 0; + MI_WRITE_INVALID_PTE(PointerPte, MmZeroPte); KeFlushCurrentTb(); }
@@ -618,7 +618,7 @@ (TempPte.u.Soft.Prototype == 1)) { /* Just nuke it */ - PointerPte->u.Long = 0; + MI_WRITE_INVALID_PTE(PointerPte, MmZeroPte); } else { @@ -632,7 +632,7 @@ else { /* The PTE was never mapped, just nuke it here */ - PointerPte->u.Long = 0; + MI_WRITE_INVALID_PTE(PointerPte, MmZeroPte); } }
@@ -2113,7 +2113,8 @@ ASSERT(PteContents.u.Soft.Transition == 0);
/* The PTE is already demand-zero, just update the protection mask */ - PointerPte->u.Soft.Protection = ProtectionMask; + PteContents.u.Soft.Protection = ProtectionMask; + MI_WRITE_INVALID_PTE(PointerPte, PteContents); ASSERT(PointerPte->u.Long != 0); }
-
tkreuzer＠svn.reactos.org