Author: sir_richard
Date: Sun Aug 29 19:32:25 2010
New Revision: 48651
URL: http://svn.reactos.org/svn/reactos?rev=48651&view=rev
Log:
[NTOS]: Add an extra layer of protection for freed nonpaged pool: write a 4-byte signature on freed blocks, and assert its valid on checked builds. Use a slightly less egocentric ASCII value than on Windows (name of the developer who wrote the first memory manager).
Modified:
trunk/reactos/ntoskrnl/mm/ARM3/pool.c
Modified: trunk/reactos/ntoskrnl/mm/ARM3/pool.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pool.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] Sun Aug 29 19:32:25 2010
@@ -298,6 +298,7 @@
FreeEntry = MmNonPagedPoolStart;
FirstEntry = FreeEntry;
FreeEntry->Size = PoolPages;
+ FreeEntry->Signature = MM_FREE_POOL_SIGNATURE;
FreeEntry->Owner = FirstEntry;
//
@@ -316,6 +317,7 @@
//
FreeEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)FreeEntry + PAGE_SIZE);
FreeEntry->Owner = FirstEntry;
+ FreeEntry->Signature = MM_FREE_POOL_SIGNATURE;
}
//
@@ -626,6 +628,7 @@
// Grab the entry and see if it can handle our allocation
//
FreeEntry = CONTAINING_RECORD(NextEntry, MMFREE_POOL_ENTRY, List);
+ ASSERT(FreeEntry->Signature == MM_FREE_POOL_SIGNATURE);
if (FreeEntry->Size >= SizeInPages)
{
//
@@ -964,6 +967,7 @@
//
FreeEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)StartingVa +
(NumberOfPages << PAGE_SHIFT));
+ ASSERT(FreeEntry->Signature == MM_FREE_POOL_SIGNATURE);
ASSERT(FreeEntry->Owner == FreeEntry);
/* Consume this entry's pages */
@@ -1032,6 +1036,7 @@
// Get the free entry descriptor for that given page range
//
FreeEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)StartingVa - PAGE_SIZE);
+ ASSERT(FreeEntry->Signature == MM_FREE_POOL_SIGNATURE);
FreeEntry = FreeEntry->Owner;
/* Check if protected pool is enabled */
@@ -1118,6 +1123,7 @@
// Link back to the parent free entry, and keep going
//
NextEntry->Owner = FreeEntry;
+ NextEntry->Signature = MM_FREE_POOL_SIGNATURE;
NextEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)NextEntry + PAGE_SIZE);
} while (NextEntry != LastEntry);
Author: sir_richard
Date: Sun Aug 29 19:27:58 2010
New Revision: 48650
URL: http://svn.reactos.org/svn/reactos?rev=48650&view=rev
Log:
[NTOS]: Missed a bunch of codepaths, protected pool "should" work now.
Modified:
trunk/reactos/ntoskrnl/include/internal/mm.h
trunk/reactos/ntoskrnl/mm/ARM3/pool.c
Modified: trunk/reactos/ntoskrnl/include/internal/mm.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/mm.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/mm.h [iso-8859-1] Sun Aug 29 19:27:58 2010
@@ -435,6 +435,9 @@
struct _MMFREE_POOL_ENTRY *Owner;
} MMFREE_POOL_ENTRY, *PMMFREE_POOL_ENTRY;
+/* Signature of a freed block */
+#define MM_FREE_POOL_SIGNATURE 'ARM3'
+
/* Paged pool information */
typedef struct _MM_PAGED_POOL_INFO
{
Modified: trunk/reactos/ntoskrnl/mm/ARM3/pool.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pool.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] Sun Aug 29 19:27:58 2010
@@ -615,6 +615,13 @@
NextEntry = NextHead->Flink;
while (NextEntry != NextHead)
{
+ /* Is freed non paged pool enabled */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* We need to be able to touch this page, unprotect it */
+ MiUnProtectFreeNonPagedPool(NextEntry, 0);
+ }
+
//
// Grab the entry and see if it can handle our allocation
//
@@ -632,23 +639,31 @@
BaseVa = (PVOID)((ULONG_PTR)FreeEntry +
(FreeEntry->Size << PAGE_SHIFT));
- //
- // This is not a free page segment anymore
- //
- RemoveEntryList(&FreeEntry->List);
+ /* Remove the item from the list, depending if pool is protected */
+ MmProtectFreedNonPagedPool ?
+ MiProtectedPoolRemoveEntryList(&FreeEntry->List) :
+ RemoveEntryList(&FreeEntry->List);
//
// However, check if its' still got space left
//
if (FreeEntry->Size != 0)
{
- //
- // Insert it back into a different list, based on its pages
- //
+ /* Check which list to insert this entry into */
i = FreeEntry->Size - 1;
if (i >= MI_MAX_FREE_PAGE_LISTS) i = MI_MAX_FREE_PAGE_LISTS - 1;
- InsertTailList (&MmNonPagedPoolFreeListHead[i],
- &FreeEntry->List);
+
+ /* Insert the entry into the free list head, check for prot. pool */
+ MmProtectFreedNonPagedPool ?
+ MiProtectedPoolInsertList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List, TRUE) :
+ InsertTailList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List);
+
+ /* Is freed non paged pool protected? */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* Protect the freed pool! */
+ MiProtectFreeNonPagedPool(FreeEntry, FreeEntry->Size);
+ }
}
//
@@ -698,6 +713,13 @@
// Try the next free page entry
//
NextEntry = FreeEntry->List.Flink;
+
+ /* Is freed non paged pool protected? */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* Protect the freed pool! */
+ MiProtectFreeNonPagedPool(FreeEntry, FreeEntry->Size);
+ }
}
} while (++NextHead < LastHead);
@@ -1095,7 +1117,7 @@
//
// Link back to the parent free entry, and keep going
//
- NextEntry->Owner = FreeEntry;
+ NextEntry->Owner = FreeEntry;
NextEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)NextEntry + PAGE_SIZE);
} while (NextEntry != LastEntry);
Author: sir_richard
Date: Sun Aug 29 19:13:08 2010
New Revision: 48649
URL: http://svn.reactos.org/svn/reactos?rev=48649&view=rev
Log:
[NTOS]: Add DRIVER_CAUGHT_MODIFYING_FREED_POOL bugcheck code.
[NTOS]: Add support for protected freed nonpaged pool. This is controlled through MmProtectFreedNonPagedPool, which is initialized based on a registry value (see cmdata.c). This is not "Special Pool", but a useful debugging feature Windows implements that we now have too, since I noticed a lot of mj's work was with freed pool access.
NB. It's 3AM and I have not tested this, it should be off in trunk by default, you'll need to try turning it on and testing it. Hope it helps.
--This line, and those low, will be ignored--
M ntoskrnl/mm/ARM3/pagfault.c
M ntoskrnl/mm/ARM3/pool.c
M include/reactos/mc/bugcodes.mc
Modified:
trunk/reactos/include/reactos/mc/bugcodes.mc
trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c
trunk/reactos/ntoskrnl/mm/ARM3/pool.c
Modified: trunk/reactos/include/reactos/mc/bugcodes.mc
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/reactos/mc/bugcode…
==============================================================================
--- trunk/reactos/include/reactos/mc/bugcodes.mc [iso-8859-1] (original)
+++ trunk/reactos/include/reactos/mc/bugcodes.mc [iso-8859-1] Sun Aug 29 19:13:08 2010
@@ -1311,6 +1311,16 @@
and then select Safe Mode.
.
+MessageId=0xC6
+Severity=Success
+Facility=System
+SymbolicName=DRIVER_CAUGHT_MODIFYING_FREED_POOL
+Language=English
+A device driver attempting to corrupt the system has been caught.
+The faulty driver currently on the kernel stack must be replaced
+with a working version.
+.
+
MessageId=0xC8
Severity=Success
Facility=System
Modified: trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pagfault.…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] Sun Aug 29 19:13:08 2010
@@ -620,10 +620,28 @@
return STATUS_SUCCESS;
}
- //
- // We don't implement prototype PTEs
- //
- ASSERT(TempPte.u.Soft.Prototype == 0);
+ /* Check one kind of prototype PTE */
+ if (TempPte.u.Soft.Prototype)
+ {
+ /* The one used for protected pool... */
+ ASSERT(MmProtectFreedNonPagedPool == TRUE);
+
+ /* Make sure protected pool is on, and that this is a pool address */
+ if ((MmProtectFreedNonPagedPool) &&
+ (((Address >= MmNonPagedPoolStart) &&
+ (Address < (PVOID)((ULONG_PTR)MmNonPagedPoolStart +
+ MmSizeOfNonPagedPoolInBytes))) ||
+ ((Address >= MmNonPagedPoolExpansionStart) &&
+ (Address < MmNonPagedPoolEnd))))
+ {
+ /* Bad boy, bad boy, whatcha gonna do, whatcha gonna do when ARM3 comes for you! */
+ KeBugCheckEx(DRIVER_CAUGHT_MODIFYING_FREED_POOL,
+ (ULONG_PTR)Address,
+ StoreInstruction,
+ Mode,
+ 4);
+ }
+ }
//
// We don't implement transition PTEs
Modified: trunk/reactos/ntoskrnl/mm/ARM3/pool.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pool.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] Sun Aug 29 19:13:08 2010
@@ -33,6 +33,150 @@
VOID
NTAPI
+MiProtectFreeNonPagedPool(IN PVOID VirtualAddress,
+ IN ULONG PageCount)
+{
+ PMMPTE PointerPte, LastPte;
+ MMPTE TempPte;
+
+ /* If pool is physical, can't protect PTEs */
+ if (MI_IS_PHYSICAL_ADDRESS(VirtualAddress)) return;
+
+ /* Get PTE pointers and loop */
+ PointerPte = MiAddressToPte(VirtualAddress);
+ LastPte = PointerPte + PageCount;
+ do
+ {
+ /* Capture the PTE for safety */
+ TempPte = *PointerPte;
+
+ /* Mark it as an invalid PTE, set proto bit to recognize it as pool */
+ TempPte.u.Hard.Valid = 0;
+ TempPte.u.Soft.Prototype = 1;
+ MI_WRITE_INVALID_PTE(PointerPte, TempPte);
+ } while (++PointerPte < LastPte);
+
+ /* Flush the TLB */
+ KeFlushEntireTb(TRUE, TRUE);
+}
+
+BOOLEAN
+NTAPI
+MiUnProtectFreeNonPagedPool(IN PVOID VirtualAddress,
+ IN ULONG PageCount)
+{
+ PMMPTE PointerPte;
+ MMPTE TempPte;
+ PFN_NUMBER UnprotectedPages = 0;
+
+ /* If pool is physical, can't protect PTEs */
+ if (MI_IS_PHYSICAL_ADDRESS(VirtualAddress)) return FALSE;
+
+ /* Get, and capture the PTE */
+ PointerPte = MiAddressToPte(VirtualAddress);
+ TempPte = *PointerPte;
+
+ /* Loop protected PTEs */
+ while ((TempPte.u.Hard.Valid == 0) && (TempPte.u.Soft.Prototype == 1))
+ {
+ /* Unprotect the PTE */
+ TempPte.u.Hard.Valid = 1;
+ TempPte.u.Soft.Prototype = 0;
+ MI_WRITE_VALID_PTE(PointerPte, TempPte);
+
+ /* One more page */
+ if (++UnprotectedPages == PageCount) break;
+
+ /* Capture next PTE */
+ TempPte = *(++PointerPte);
+ }
+
+ /* Return if any pages were unprotected */
+ return UnprotectedPages ? TRUE : FALSE;
+}
+
+VOID
+FORCEINLINE
+MiProtectedPoolUnProtectLinks(IN PLIST_ENTRY Links,
+ OUT PVOID* PoolFlink,
+ OUT PVOID* PoolBlink)
+{
+ BOOLEAN Safe;
+ PVOID PoolVa;
+
+ /* Initialize variables */
+ *PoolFlink = *PoolBlink = NULL;
+
+ /* Check if the list has entries */
+ if (IsListEmpty(Links) == FALSE)
+ {
+ /* We are going to need to forward link to do an insert */
+ PoolVa = Links->Flink;
+
+ /* So make it safe to access */
+ Safe = MiUnProtectFreeNonPagedPool(PoolVa, 1);
+ if (Safe) PoolFlink = PoolVa;
+ }
+
+ /* Are we going to need a backward link too? */
+ if (Links != Links->Blink)
+ {
+ /* Get the head's backward link for the insert */
+ PoolVa = Links->Blink;
+
+ /* Make it safe to access */
+ Safe = MiUnProtectFreeNonPagedPool(PoolVa, 1);
+ if (Safe) PoolBlink = PoolVa;
+ }
+}
+
+VOID
+FORCEINLINE
+MiProtectedPoolProtectLinks(IN PVOID PoolFlink,
+ IN PVOID PoolBlink)
+{
+ /* Reprotect the pages, if they got unprotected earlier */
+ if (PoolFlink) MiProtectFreeNonPagedPool(PoolFlink, 1);
+ if (PoolBlink) MiProtectFreeNonPagedPool(PoolBlink, 1);
+}
+
+VOID
+NTAPI
+MiProtectedPoolInsertList(IN PLIST_ENTRY ListHead,
+ IN PLIST_ENTRY Entry,
+ IN BOOLEAN Critical)
+{
+ PVOID PoolFlink, PoolBlink;
+
+ /* Make the list accessible */
+ MiProtectedPoolUnProtectLinks(ListHead, &PoolFlink, &PoolBlink);
+
+ /* Now insert in the right position */
+ Critical ? InsertHeadList(ListHead, Entry) : InsertTailList(ListHead, Entry);
+
+ /* And reprotect the pages containing the free links */
+ MiProtectedPoolProtectLinks(PoolFlink, PoolBlink);
+}
+
+VOID
+NTAPI
+MiProtectedPoolRemoveEntryList(IN PLIST_ENTRY Entry)
+{
+ PVOID PoolFlink, PoolBlink;
+
+ /* Make the list accessible */
+ MiProtectedPoolUnProtectLinks(Entry, &PoolFlink, &PoolBlink);
+
+ /* Now remove */
+ RemoveEntryList(Entry);
+
+ /* And reprotect the pages containing the free links */
+ if (PoolFlink) MiProtectFreeNonPagedPool(PoolFlink, 1);
+ if (PoolBlink) MiProtectFreeNonPagedPool(PoolBlink, 1);
+}
+
+VOID
+NTAPI
MiInitializeNonPagedPoolThresholds(VOID)
{
PFN_NUMBER Size = MmMaximumNonPagedPoolInPages;
@@ -245,7 +389,7 @@
//
// Handle paged pool
//
- if (PoolType == PagedPool)
+ if ((PoolType & BASE_POOL_TYPE_MASK) == PagedPool)
{
//
// Lock the paged pool mutex
@@ -755,12 +899,21 @@
}
else
{
+ /* Sanity check */
+ ASSERT((ULONG_PTR)StartingVa + NumberOfPages <= (ULONG_PTR)MmNonPagedPoolEnd);
+
+ /* Check if protected pool is enabled */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* The freed block will be merged, it must be made accessible */
+ MiUnProtectFreeNonPagedPool(MiPteToAddress(PointerPte), 0);
+ }
+
//
// Otherwise, our entire allocation must've fit within the initial non
// paged pool, or the expansion nonpaged pool, so get the PFN entry of
// the next allocation
//
- ASSERT((ULONG_PTR)StartingVa + NumberOfPages <= (ULONG_PTR)MmNonPagedPoolEnd);
if (PointerPte->u.Hard.Valid == 1)
{
//
@@ -791,11 +944,13 @@
(NumberOfPages << PAGE_SHIFT));
ASSERT(FreeEntry->Owner == FreeEntry);
- //
- // Consume this entry's pages, and remove it from its free list
- //
+ /* Consume this entry's pages */
FreePages += FreeEntry->Size;
- RemoveEntryList (&FreeEntry->List);
+
+ /* Remove the item from the list, depending if pool is protected */
+ MmProtectFreedNonPagedPool ?
+ MiProtectedPoolRemoveEntryList(&FreeEntry->List) :
+ RemoveEntryList(&FreeEntry->List);
}
//
@@ -819,6 +974,15 @@
// Otherwise, get the PTE for the page right before our allocation
//
PointerPte -= NumberOfPages + 1;
+
+ /* Check if protected pool is enabled */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* The freed block will be merged, it must be made accessible */
+ MiUnProtectFreeNonPagedPool(MiPteToAddress(PointerPte), 0);
+ }
+
+ /* Check if this is valid pool, or a guard page */
if (PointerPte->u.Hard.Valid == 1)
{
//
@@ -848,6 +1012,13 @@
FreeEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)StartingVa - PAGE_SIZE);
FreeEntry = FreeEntry->Owner;
+ /* Check if protected pool is enabled */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* The freed block will be merged, it must be made accessible */
+ MiUnProtectFreeNonPagedPool(FreeEntry, 0);
+ }
+
//
// Check if the entry is small enough to be indexed on a free list
// If it is, we'll want to re-insert it, since we're about to
@@ -855,10 +1026,10 @@
//
if (FreeEntry->Size < (MI_MAX_FREE_PAGE_LISTS - 1))
{
- //
- // Remove the list from where it is now
- //
- RemoveEntryList(&FreeEntry->List);
+ /* Remove the item from the list, depending if pool is protected */
+ MmProtectFreedNonPagedPool ?
+ MiProtectedPoolRemoveEntryList(&FreeEntry->List) :
+ RemoveEntryList(&FreeEntry->List);
//
// Update its size
@@ -871,10 +1042,10 @@
i = (ULONG)(FreeEntry->Size - 1);
if (i >= MI_MAX_FREE_PAGE_LISTS) i = MI_MAX_FREE_PAGE_LISTS - 1;
- //
- // Do it
- //
- InsertTailList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List);
+ /* Insert the entry into the free list head, check for prot. pool */
+ MmProtectFreedNonPagedPool ?
+ MiProtectedPoolInsertList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List, TRUE) :
+ InsertTailList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List);
}
else
{
@@ -902,10 +1073,10 @@
i = FreeEntry->Size - 1;
if (i >= MI_MAX_FREE_PAGE_LISTS) i = MI_MAX_FREE_PAGE_LISTS - 1;
- //
- // And insert us
- //
- InsertTailList (&MmNonPagedPoolFreeListHead[i], &FreeEntry->List);
+ /* Insert the entry into the free list head, check for prot. pool */
+ MmProtectFreedNonPagedPool ?
+ MiProtectedPoolInsertList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List, TRUE) :
+ InsertTailList(&MmNonPagedPoolFreeListHead[i], &FreeEntry->List);
}
//
@@ -927,6 +1098,13 @@
NextEntry->Owner = FreeEntry;
NextEntry = (PMMFREE_POOL_ENTRY)((ULONG_PTR)NextEntry + PAGE_SIZE);
} while (NextEntry != LastEntry);
+
+ /* Is freed non paged pool protected? */
+ if (MmProtectFreedNonPagedPool)
+ {
+ /* Protect the freed pool! */
+ MiProtectFreeNonPagedPool(FreeEntry, FreeEntry->Size);
+ }
//
// We're done, release the lock and let the caller know how much we freed
Author: mjmartin
Date: Sun Aug 29 17:46:18 2010
New Revision: 48646
URL: http://svn.reactos.org/svn/reactos?rev=48646&view=rev
Log:
[cdfs]
- Working with Pierre Schweitzer for yet another NonPaged Pool corruption fix. When copying VolumeLabel the VolumeLabelLength is in Unicode, so theres no need to mulitply it by size of WCHAR.
Modified:
trunk/reactos/drivers/filesystems/cdfs/fsctl.c
Modified: trunk/reactos/drivers/filesystems/cdfs/fsctl.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/cdfs/f…
==============================================================================
--- trunk/reactos/drivers/filesystems/cdfs/fsctl.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/cdfs/fsctl.c [iso-8859-1] Sun Aug 29 17:46:18 2010
@@ -359,7 +359,7 @@
Vpb->SerialNumber = CdInfo.SerialNumber;
Vpb->VolumeLabelLength = CdInfo.VolumeLabelLength;
- RtlCopyMemory(Vpb->VolumeLabel, CdInfo.VolumeLabel, CdInfo.VolumeLabelLength * sizeof(WCHAR));
+ RtlCopyMemory(Vpb->VolumeLabel, CdInfo.VolumeLabel, CdInfo.VolumeLabelLength);
RtlCopyMemory(&DeviceExt->CdInfo, &CdInfo, sizeof(CDINFO));
NewDeviceObject->Vpb = DeviceToMount->Vpb;