https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2221df213f40ec1b6d62a0...
commit 2221df213f40ec1b6d62a0747863c9e0c891a97c Author: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org AuthorDate: Fri Feb 18 03:20:24 2022 +0100 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org CommitDate: Fri Feb 18 20:24:49 2022 +0100
[FREELDR] Add cleanup in PE-image-handling-related failure code paths. --- boot/freeldr/freeldr/disk/scsiport.c | 25 ++++++--- boot/freeldr/freeldr/include/peloader.h | 5 ++ boot/freeldr/freeldr/lib/peloader.c | 82 ++++++++++++++++++----------- boot/freeldr/freeldr/ntldr/winldr.c | 93 ++++++++++++++++++++++++++------- 4 files changed, 148 insertions(+), 57 deletions(-)
diff --git a/boot/freeldr/freeldr/disk/scsiport.c b/boot/freeldr/freeldr/disk/scsiport.c index d23fe58a76e..0699f69f6a6 100644 --- a/boot/freeldr/freeldr/disk/scsiport.c +++ b/boot/freeldr/freeldr/disk/scsiport.c @@ -1653,7 +1653,7 @@ LoadBootDeviceDriver(VOID) Success = PeLdrLoadImage(NtBootDdPath, LoaderBootDriver, &ImageBase); if (!Success) { - /* That's OK. File simply doesn't exist */ + /* That's OK, file simply doesn't exist */ return ESUCCESS; }
@@ -1661,7 +1661,11 @@ LoadBootDeviceDriver(VOID) Success = PeLdrAllocateDataTableEntry(&ModuleListHead, "ntbootdd.sys", "NTBOOTDD.SYS", ImageBase, &BootDdDTE); if (!Success) + { + /* Cleanup and bail out */ + MmFreeMemory(ImageBase); return EIO; + }
/* Add the PE part of freeldr.sys to the list of loaded executables, it contains ScsiPort* exports, imported by ntbootdd.sys */ @@ -1669,20 +1673,27 @@ LoadBootDeviceDriver(VOID) "FREELDR.SYS", &__ImageBase, &FreeldrDTE); if (!Success) { - RemoveEntryList(&BootDdDTE->InLoadOrderLinks); + /* Cleanup and bail out */ + PeLdrFreeDataTableEntry(BootDdDTE); + MmFreeMemory(ImageBase); return EIO; }
/* Fix imports */ Success = PeLdrScanImportDescriptorTable(&ModuleListHead, "", BootDdDTE); + if (!Success) + { + /* Cleanup and bail out */ + PeLdrFreeDataTableEntry(FreeldrDTE); + PeLdrFreeDataTableEntry(BootDdDTE); + MmFreeMemory(ImageBase); + return EIO; + }
- /* Now unlinkt the DTEs, they won't be valid later */ + /* Now unlink the DTEs, they won't be valid later */ RemoveEntryList(&BootDdDTE->InLoadOrderLinks); RemoveEntryList(&FreeldrDTE->InLoadOrderLinks);
- if (!Success) - return EIO; - /* Change imports to PA */ ImportTable = (PIMAGE_IMPORT_DESCRIPTOR)RtlImageDirectoryEntryToData(VaToPa(BootDdDTE->DllBase), TRUE, IMAGE_DIRECTORY_ENTRY_IMPORT, &ImportTableSize); @@ -1705,7 +1716,7 @@ LoadBootDeviceDriver(VOID) NtHeaders->OptionalHeader.ImageBase - (ULONG_PTR)BootDdDTE->DllBase, "FreeLdr", TRUE, - TRUE, /* in case of conflict still return success */ + TRUE, /* In case of conflict still return success */ FALSE); if (!Success) return EIO; diff --git a/boot/freeldr/freeldr/include/peloader.h b/boot/freeldr/freeldr/include/peloader.h index 63ffa80471a..1f67ed3bbac 100644 --- a/boot/freeldr/freeldr/include/peloader.h +++ b/boot/freeldr/freeldr/include/peloader.h @@ -40,6 +40,11 @@ PeLdrAllocateDataTableEntry( IN PVOID BasePA, OUT PLDR_DATA_TABLE_ENTRY *NewEntry);
+VOID +PeLdrFreeDataTableEntry( + // _In_ PLIST_ENTRY ModuleListHead, + _In_ PLDR_DATA_TABLE_ENTRY Entry); + BOOLEAN PeLdrScanImportDescriptorTable( IN OUT PLIST_ENTRY ModuleListHead, diff --git a/boot/freeldr/freeldr/lib/peloader.c b/boot/freeldr/freeldr/lib/peloader.c index b768aa9dea8..f97639e4cec 100644 --- a/boot/freeldr/freeldr/lib/peloader.c +++ b/boot/freeldr/freeldr/lib/peloader.c @@ -115,7 +115,7 @@ PeLdrpBindImportName( // DllBase, ImageBase, ThunkData, ExportDirectory, ExportSize, ProcessForwards);
/* Check passed DllBase param */ - if(DllBase == NULL) + if (DllBase == NULL) { WARN("DllBase == NULL!\n"); return FALSE; @@ -368,7 +368,7 @@ PeLdrpLoadAndScanReferencedDll( Success = PeLdrLoadImage(FullDllName, LoaderBootDriver, &BasePA); if (!Success) { - ERR("PeLdrLoadImage() failed\n"); + ERR("PeLdrLoadImage('%s') failed\n", FullDllName); return Success; }
@@ -380,7 +380,9 @@ PeLdrpLoadAndScanReferencedDll( DataTableEntry); if (!Success) { - ERR("PeLdrAllocateDataTableEntry() failed\n"); + /* Cleanup and bail out */ + ERR("PeLdrAllocateDataTableEntry('%s') failed\n", FullDllName); + MmFreeMemory(BasePA); return Success; }
@@ -392,7 +394,10 @@ PeLdrpLoadAndScanReferencedDll( Success = PeLdrScanImportDescriptorTable(ModuleListHead, DirectoryPath, *DataTableEntry); if (!Success) { + /* Cleanup and bail out */ ERR("PeLdrScanImportDescriptorTable() failed\n"); + PeLdrFreeDataTableEntry(*DataTableEntry); + MmFreeMemory(BasePA); return Success; }
@@ -603,7 +608,7 @@ PeLdrAllocateDataTableEntry( PIMAGE_NT_HEADERS NtHeaders; USHORT Length;
- TRACE("PeLdrAllocateDataTableEntry(, '%s', '%s', %p)\n", + TRACE("PeLdrAllocateDataTableEntry('%s', '%s', %p)\n", BaseDllName, FullDllName, BasePA);
/* Allocate memory for a data table entry, zero-initialize it */ @@ -706,6 +711,20 @@ PeLdrAllocateDataTableEntry( return TRUE; }
+VOID +PeLdrFreeDataTableEntry( + // _In_ PLIST_ENTRY ModuleListHead, + _In_ PLDR_DATA_TABLE_ENTRY Entry) +{ + // ASSERT(ModuleListHead); + ASSERT(Entry); + + RemoveEntryList(&Entry->InLoadOrderLinks); + FrLdrHeapFree(VaToPa(Entry->FullDllName.Buffer), TAG_WLDR_NAME); + FrLdrHeapFree(VaToPa(Entry->BaseDllName.Buffer), TAG_WLDR_NAME); + FrLdrHeapFree(Entry, TAG_WLDR_DTE); +} + /* * PeLdrLoadImage loads the specified image from the file (it doesn't * perform any additional operations on the filename, just directly @@ -730,13 +749,13 @@ PeLdrLoadImage( LARGE_INTEGER Position; ULONG i, BytesRead;
- TRACE("PeLdrLoadImage(%s, %ld)\n", FileName, MemoryType); + TRACE("PeLdrLoadImage('%s', %ld)\n", FileName, MemoryType);
/* Open the image file */ Status = ArcOpen((PSTR)FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { - WARN("ArcOpen(FileName: '%s') failed. Status: %u\n", FileName, Status); + WARN("ArcOpen('%s') failed. Status: %u\n", FileName, Status); return FALSE; }
@@ -744,8 +763,7 @@ PeLdrLoadImage( Status = ArcRead(FileId, HeadersBuffer, SECTOR_SIZE * 2, &BytesRead); if (Status != ESUCCESS) { - ERR("ArcRead(File: '%s') failed. Status: %u\n", FileName, Status); - UiMessageBox("Error reading from file."); + ERR("ArcRead('%s') failed. Status: %u\n", FileName, Status); ArcClose(FileId); return FALSE; } @@ -755,7 +773,6 @@ PeLdrLoadImage( if (!NtHeaders) { ERR("No NT header found in "%s"\n", FileName); - UiMessageBox("Error: No NT header found."); ArcClose(FileId); return FALSE; } @@ -764,7 +781,6 @@ PeLdrLoadImage( if (((NtHeaders->FileHeader.Characteristics & IMAGE_FILE_EXECUTABLE_IMAGE) == 0)) { ERR("Not an executable image "%s"\n", FileName); - UiMessageBox("Not an executable image."); ArcClose(FileId); return FALSE; } @@ -773,26 +789,25 @@ PeLdrLoadImage( NumberOfSections = NtHeaders->FileHeader.NumberOfSections; SectionHeader = IMAGE_FIRST_SECTION(NtHeaders);
- /* Try to allocate this memory, if fails - allocate somewhere else */ + /* Try to allocate this memory; if it fails, allocate somewhere else */ PhysicalBase = MmAllocateMemoryAtAddress(NtHeaders->OptionalHeader.SizeOfImage, (PVOID)((ULONG)NtHeaders->OptionalHeader.ImageBase & (KSEG0_BASE - 1)), MemoryType);
if (PhysicalBase == NULL) { - /* It's ok, we don't panic - let's allocate again at any other "low" place */ + /* Don't fail, allocate again at any other "low" place */ PhysicalBase = MmAllocateMemoryWithType(NtHeaders->OptionalHeader.SizeOfImage, MemoryType);
if (PhysicalBase == NULL) { ERR("Failed to alloc %lu bytes for image %s\n", NtHeaders->OptionalHeader.SizeOfImage, FileName); - UiMessageBox("Failed to alloc pages for image."); ArcClose(FileId); return FALSE; } }
- /* This is the real image base - in form of a virtual address */ + /* This is the real image base, in form of a virtual address */ VirtualBase = PaToVa(PhysicalBase);
TRACE("Base PA: 0x%X, VA: 0x%X\n", PhysicalBase, VirtualBase); @@ -805,10 +820,10 @@ PeLdrLoadImage( Status = ArcRead(FileId, (PUCHAR)PhysicalBase + sizeof(HeadersBuffer), NtHeaders->OptionalHeader.SizeOfHeaders - sizeof(HeadersBuffer), &BytesRead); if (Status != ESUCCESS) { - ERR("ArcRead(File: '%s') failed. Status: %u\n", FileName, Status); - UiMessageBox("Error reading headers."); + ERR("ArcRead('%s') failed. Status: %u\n", FileName, Status); + // UiMessageBox("Error reading headers."); ArcClose(FileId); - return FALSE; + goto Failure; } }
@@ -824,9 +839,6 @@ PeLdrLoadImage( /* Load the first section */ SectionHeader = IMAGE_FIRST_SECTION(NtHeaders);
- /* Fill output parameters */ - *ImageBasePA = PhysicalBase; - /* Walk through each section and read it (check/fix any possible bad situations, if they arise) */ for (i = 0; i < NumberOfSections; i++) @@ -868,7 +880,7 @@ PeLdrLoadImage( } }
- /* Size of data is less than the virtual size - fill up the remainder with zeroes */ + /* Size of data is less than the virtual size: fill up the remainder with zeroes */ if (SizeOfRawData < VirtualSize) { TRACE("PeLdrLoadImage(): SORD %d < VS %d\n", SizeOfRawData, VirtualSize); @@ -878,25 +890,35 @@ PeLdrLoadImage( SectionHeader++; }
- /* We are done with the file - close it */ + /* We are done with the file, close it */ ArcClose(FileId);
- /* If loading failed - return right now */ + /* If loading failed, return right now */ if (Status != ESUCCESS) - return FALSE; + goto Failure;
/* Relocate the image, if it needs it */ if (NtHeaders->OptionalHeader.ImageBase != (ULONG_PTR)VirtualBase) { WARN("Relocating %p -> %p\n", NtHeaders->OptionalHeader.ImageBase, VirtualBase); - return (BOOLEAN)LdrRelocateImageWithBias(PhysicalBase, - (ULONG_PTR)VirtualBase - (ULONG_PTR)PhysicalBase, - "FreeLdr", - TRUE, - TRUE, /* in case of conflict still return success */ - FALSE); + Status = LdrRelocateImageWithBias(PhysicalBase, + (ULONG_PTR)VirtualBase - (ULONG_PTR)PhysicalBase, + "FreeLdr", + ESUCCESS, + ESUCCESS, /* In case of conflict still return success */ + ENOEXEC); + if (Status != ESUCCESS) + goto Failure; }
+ /* Fill output parameters */ + *ImageBasePA = PhysicalBase; + TRACE("PeLdrLoadImage() done, PA = %p\n", *ImageBasePA); return TRUE; + +Failure: + /* Cleanup and bail out */ + MmFreeMemory(PhysicalBase); + return FALSE; } diff --git a/boot/freeldr/freeldr/ntldr/winldr.c b/boot/freeldr/freeldr/ntldr/winldr.c index 2721ce4f0ed..3e7abaa51f4 100644 --- a/boot/freeldr/freeldr/ntldr/winldr.c +++ b/boot/freeldr/freeldr/ntldr/winldr.c @@ -333,7 +333,9 @@ WinLdrLoadDeviceDriver(PLIST_ENTRY LoadOrderListHead, Success = PeLdrAllocateDataTableEntry(LoadOrderListHead, DllName, DllName, DriverBase, DriverDTE); if (!Success) { + /* Cleanup and bail out */ ERR("PeLdrAllocateDataTableEntry('%s') failed\n", DllName); + MmFreeMemory(DriverBase); return FALSE; }
@@ -345,7 +347,10 @@ WinLdrLoadDeviceDriver(PLIST_ENTRY LoadOrderListHead, Success = PeLdrScanImportDescriptorTable(LoadOrderListHead, FullPath, *DriverDTE); if (!Success) { + /* Cleanup and bail out */ ERR("PeLdrScanImportDescriptorTable('%s') failed\n", FullPath); + PeLdrFreeDataTableEntry(*DriverDTE); + MmFreeMemory(DriverBase); return FALSE; }
@@ -482,7 +487,7 @@ WinLdrDetectVersion(VOID) }
static -BOOLEAN +PVOID LoadModule( IN OUT PLOADER_PARAMETER_BLOCK LoaderBlock, IN PCCH Path, @@ -495,7 +500,7 @@ LoadModule( BOOLEAN Success; CHAR FullFileName[MAX_PATH]; CHAR ProgressString[256]; - PVOID BaseAddress = NULL; + PVOID BaseAddress;
RtlStringCbPrintfA(ProgressString, sizeof(ProgressString), "Loading %s...", File); if (!SosEnabled) UiDrawProgressBarCenter(Percentage, 100, ProgressString); @@ -508,15 +513,10 @@ LoadModule( if (!Success) { ERR("PeLdrLoadImage('%s') failed\n", File); - return FALSE; + return NULL; } TRACE("%s loaded successfully at %p\n", File, BaseAddress);
- /* - * Cheat about the base DLL name if we are loading - * the Kernel Debugger Transport DLL, to make the - * PE loader happy. - */ Success = PeLdrAllocateDataTableEntry(&LoaderBlock->LoadOrderListHead, ImportName, FullFileName, @@ -524,10 +524,13 @@ LoadModule( Dte); if (!Success) { + /* Cleanup and bail out */ ERR("PeLdrAllocateDataTableEntry('%s') failed\n", FullFileName); + MmFreeMemory(BaseAddress); + BaseAddress = NULL; }
- return Success; + return BaseAddress; }
static @@ -541,7 +544,8 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion, BOOLEAN Success; PCSTR Option; ULONG OptionLength; - PLDR_DATA_TABLE_ENTRY HalDTE, KdComDTE = NULL; + PVOID KernelBase, HalBase, KdDllBase = NULL; + PLDR_DATA_TABLE_ENTRY HalDTE, KdDllDTE = NULL; CHAR DirPath[MAX_PATH]; CHAR HalFileName[MAX_PATH]; CHAR KernelFileName[MAX_PATH]; @@ -585,17 +589,33 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion,
TRACE("HAL file = '%s' ; Kernel file = '%s'\n", HalFileName, KernelFileName);
+ /* + * Load the core NT files: Kernel, HAL and KD transport DLL. + * Cheat about their base DLL name so as to satisfy the imports/exports, + * even if the corresponding underlying files do not have the same names + * -- this happens e.g. with UP vs. MP kernel, standard vs. ACPI hal, or + * different KD transport DLLs. + */ + /* Load the Kernel */ - if (!LoadModule(LoaderBlock, DirPath, KernelFileName, "ntoskrnl.exe", LoaderSystemCode, KernelDTE, 30)) + KernelBase = LoadModule(LoaderBlock, DirPath, KernelFileName, + "ntoskrnl.exe", LoaderSystemCode, KernelDTE, 30); + if (!KernelBase) { - ERR("LoadModule() failed for %s\n", KernelFileName); + ERR("LoadModule('%s') failed\n", KernelFileName); + UiMessageBox("Could not load %s", KernelFileName); return FALSE; }
/* Load the HAL */ - if (!LoadModule(LoaderBlock, DirPath, HalFileName, "hal.dll", LoaderHalCode, &HalDTE, 45)) + HalBase = LoadModule(LoaderBlock, DirPath, HalFileName, + "hal.dll", LoaderHalCode, &HalDTE, 45); + if (!HalBase) { - ERR("LoadModule() failed for %s\n", HalFileName); + ERR("LoadModule('%s') failed\n", HalFileName); + UiMessageBox("Could not load %s", HalFileName); + PeLdrFreeDataTableEntry(*KernelDTE); + MmFreeMemory(KernelBase); return FALSE; }
@@ -651,10 +671,12 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion, * Load the transport DLL. Override the base DLL name of the * loaded transport DLL to the default "KDCOM.DLL" name. */ - if (!LoadModule(LoaderBlock, DirPath, KdDllName, "kdcom.dll", LoaderSystemCode, &KdComDTE, 60)) + KdDllBase = LoadModule(LoaderBlock, DirPath, KdDllName, + "kdcom.dll", LoaderSystemCode, &KdDllDTE, 60); + if (!KdDllBase) { /* The transport DLL being optional, just ignore the failure */ - WARN("LoadModule() failed for %s\n", KdDllName); + WARN("LoadModule('%s') failed\n", KdDllName); } } } @@ -730,11 +752,42 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion, }
/* Load all referenced DLLs for Kernel, HAL and Kernel Debugger Transport DLL */ - Success = PeLdrScanImportDescriptorTable(&LoaderBlock->LoadOrderListHead, DirPath, *KernelDTE); - Success &= PeLdrScanImportDescriptorTable(&LoaderBlock->LoadOrderListHead, DirPath, HalDTE); - if (KdComDTE) + Success = PeLdrScanImportDescriptorTable(&LoaderBlock->LoadOrderListHead, DirPath, *KernelDTE); + if (!Success) + { + UiMessageBox("Could not load %s", KernelFileName); + goto Quit; + } + Success = PeLdrScanImportDescriptorTable(&LoaderBlock->LoadOrderListHead, DirPath, HalDTE); + if (!Success) + { + UiMessageBox("Could not load %s", HalFileName); + goto Quit; + } + if (KdDllDTE) + { + Success = PeLdrScanImportDescriptorTable(&LoaderBlock->LoadOrderListHead, DirPath, KdDllDTE); + if (!Success) + { + UiMessageBox("Could not load %s", KdDllName); + goto Quit; + } + } + +Quit: + if (!Success) { - Success &= PeLdrScanImportDescriptorTable(&LoaderBlock->LoadOrderListHead, DirPath, KdComDTE); + /* Cleanup and bail out */ + if (KdDllDTE) + PeLdrFreeDataTableEntry(KdDllDTE); + if (KdDllBase) // Optional + MmFreeMemory(KdDllBase); + + PeLdrFreeDataTableEntry(HalDTE); + MmFreeMemory(HalBase); + + PeLdrFreeDataTableEntry(*KernelDTE); + MmFreeMemory(KernelBase); }
return Success;
-
Hermès Bélusca-Maïto