Skip to content

Commit d5181e4

Browse files
authored
[NTOS] Fix MiFindInitializationCode (reactos#751)
Short: The code was suffering from an off-by-one bug (inconsistency between inclusive end exclusive end address), which could lead to freeing one page above the initialization code. This led to freeing part of the kernel import section on x64. Now it is consistently using the aligned/exclusive end address. Long: * Initialization sections are freed both for the boot loaded images as well as for drivers that are loaded later. Obviously the second mechanism needs to be able to run at any time, so it is not initialization code itself. For some reason someone decided though, it would be a smart idea to implement the code twice, once for the boot loaded images, once for drivers and concluding that the former was initialization code itself and had to be freed. * Since freeing the code that frees the initialization sections, while it is doing that is not possible, it uses a "smart trick", initially skipping that range, returning its start and end to the caller and have the caller free it afterwards. * The code was using the end address in an inconsistent way, partly aligning it to the start of the following section, sometimes pointing to the last byte that should be freed. The function that freed each chunk was assuming the latter (i.e. that the end was included in the range) and thus freed the page that contained the end address. The end address for the range that was returned to the caller was aligned to the start of the next section, and the caller used it to free the range including the following page. On x64 this was the start of the import section of ntoskrnl. How that worked on x86 I don't even want to know.
1 parent b5aa79a commit d5181e4

File tree

1 file changed

+23
-11
lines changed

1 file changed

+23
-11
lines changed

ntoskrnl/mm/ARM3/sysldr.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,7 +1435,7 @@ MiFreeInitializationCode(IN PVOID InitStart,
14351435
ASSERT(MI_IS_PHYSICAL_ADDRESS(InitStart) == FALSE);
14361436

14371437
/* Compute the number of pages we expect to free */
1438-
PagesFreed = (PFN_NUMBER)(MiAddressToPte(InitEnd) - PointerPte + 1);
1438+
PagesFreed = (PFN_NUMBER)(MiAddressToPte(InitEnd) - PointerPte);
14391439

14401440
/* Try to actually free them */
14411441
PagesFreed = MiDeleteSystemPageableVm(PointerPte,
@@ -1455,8 +1455,9 @@ MiFindInitializationCode(OUT PVOID *StartVa,
14551455
ULONG_PTR DllBase, InitStart, InitEnd, ImageEnd, InitCode;
14561456
PLIST_ENTRY NextEntry;
14571457
PIMAGE_NT_HEADERS NtHeader;
1458-
PIMAGE_SECTION_HEADER Section, LastSection;
1458+
PIMAGE_SECTION_HEADER Section, LastSection, InitSection;
14591459
BOOLEAN InitFound;
1460+
DBG_UNREFERENCED_LOCAL_VARIABLE(InitSection);
14601461

14611462
/* So we don't free our own code yet */
14621463
InitCode = (ULONG_PTR)&MiFindInitializationCode;
@@ -1503,11 +1504,12 @@ MiFindInitializationCode(OUT PVOID *StartVa,
15031504
InitFound = FALSE;
15041505

15051506
/* Is this the INIT section or a discardable section? */
1506-
if ((*(PULONG)Section->Name == 'TINI') ||
1507+
if ((strncmp((PCCH)Section->Name, "INIT", 5) == 0) ||
15071508
((Section->Characteristics & IMAGE_SCN_MEM_DISCARDABLE)))
15081509
{
15091510
/* Remember this */
15101511
InitFound = TRUE;
1512+
InitSection = Section;
15111513
}
15121514

15131515
if (InitFound)
@@ -1518,10 +1520,13 @@ MiFindInitializationCode(OUT PVOID *StartVa,
15181520
/* Read the section alignment */
15191521
Alignment = NtHeader->OptionalHeader.SectionAlignment;
15201522

1521-
/* Align the start and end addresses appropriately */
1523+
/* Get the start and end addresses */
15221524
InitStart = DllBase + Section->VirtualAddress;
1523-
InitEnd = ((Alignment + InitStart + Size - 2) & 0xFFFFF000) - 1;
1524-
InitStart = (InitStart + (PAGE_SIZE - 1)) & 0xFFFFF000;
1525+
InitEnd = ALIGN_UP_BY(InitStart + Size, Alignment);
1526+
1527+
/* Align the addresses to PAGE_SIZE */
1528+
InitStart = ALIGN_UP_BY(InitStart, PAGE_SIZE);
1529+
InitEnd = ALIGN_DOWN_BY(InitEnd, PAGE_SIZE);
15251530

15261531
/* Have we reached the last section? */
15271532
if (SectionCount == 1)
@@ -1559,25 +1564,26 @@ MiFindInitializationCode(OUT PVOID *StartVa,
15591564
Size = max(LastSection->SizeOfRawData, LastSection->Misc.VirtualSize);
15601565

15611566
/* Use this as the end of the section address */
1562-
InitEnd = DllBase + LastSection->VirtualAddress + Size - 1;
1567+
InitEnd = DllBase + LastSection->VirtualAddress + Size;
15631568

15641569
/* Have we reached the last section yet? */
15651570
if (SectionCount != 1)
15661571
{
15671572
/* Then align this accross the session boundary */
1568-
InitEnd = ((Alignment + InitEnd - 1) & 0XFFFFF000) - 1;
1573+
InitEnd = ALIGN_UP_BY(InitEnd, Alignment);
1574+
InitEnd = ALIGN_DOWN_BY(InitEnd, PAGE_SIZE);
15691575
}
15701576
}
15711577

15721578
/* Make sure we don't let the init section go past the image */
15731579
ImageEnd = DllBase + LdrEntry->SizeOfImage;
1574-
if (InitEnd > ImageEnd) InitEnd = (ImageEnd - 1) | (PAGE_SIZE - 1);
1580+
if (InitEnd > ImageEnd) InitEnd = ALIGN_UP_BY(ImageEnd, PAGE_SIZE);
15751581

15761582
/* Make sure we have a valid, non-zero init section */
1577-
if (InitStart <= InitEnd)
1583+
if (InitStart < InitEnd)
15781584
{
15791585
/* Make sure we are not within this code itself */
1580-
if ((InitCode >= InitStart) && (InitCode <= InitEnd))
1586+
if ((InitCode >= InitStart) && (InitCode < InitEnd))
15811587
{
15821588
/* Return it, we can't free ourselves now */
15831589
ASSERT(*StartVa == 0);
@@ -1588,6 +1594,12 @@ MiFindInitializationCode(OUT PVOID *StartVa,
15881594
{
15891595
/* This isn't us -- go ahead and free it */
15901596
ASSERT(MI_IS_PHYSICAL_ADDRESS((PVOID)InitStart) == FALSE);
1597+
DPRINT("Freeing init code: %p-%p ('%wZ' @%p : '%s')\n",
1598+
(PVOID)InitStart,
1599+
(PVOID)InitEnd,
1600+
&LdrEntry->BaseDllName,
1601+
LdrEntry->DllBase,
1602+
InitSection->Name);
15911603
MiFreeInitializationCode((PVOID)InitStart, (PVOID)InitEnd);
15921604
}
15931605
}

0 commit comments

Comments
 (0)