https://git.reactos.org/?p=reactos.git;a=commitdiff;h=c5e5e3117c77e4caa8bbd1...
commit c5e5e3117c77e4caa8bbd1b48df0bcd56155da1a Author: Serge Gautherie reactos-git_serge_171003@gautherie.fr AuthorDate: Wed Sep 9 03:46:56 2020 +0200 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org CommitDate: Fri Nov 19 01:49:55 2021 +0100
[FREELDR] Diverse improvements and fixes for CORE-12802. (#3466)
- Move a few lines around. - Switch to RtlZeroMemory() from memset(). - Make while() more explicit.
For CORE-12802: - Add/Fix FrLdrHeapAlloc() failure handling and related. Especially, add/fix FrLdrHeapFree() calls.
- Add/Improve ERR() to some FrLdrHeapAlloc() failure cases.
Co-authored-by: Hermès BÉLUSCA - MAÏTO hermes.belusca-maito@reactos.org --- boot/freeldr/freeldr/arch/i386/hwacpi.c | 1 - boot/freeldr/freeldr/arch/i386/hwpci.c | 6 ++-- boot/freeldr/freeldr/arch/i386/pc/machpc.c | 17 ++++++------ boot/freeldr/freeldr/arch/i386/pc/pchw.c | 5 ++-- boot/freeldr/freeldr/arch/i386/pc98/pc98hw.c | 5 ++-- boot/freeldr/freeldr/arch/i386/xbox/machxbox.c | 6 ++-- boot/freeldr/freeldr/lib/peloader.c | 24 ++++++++++------ boot/freeldr/freeldr/ntldr/winldr.c | 6 ++++ boot/freeldr/freeldr/ntldr/wlregistry.c | 38 ++++++++++++++++++++------ 9 files changed, 73 insertions(+), 35 deletions(-)
diff --git a/boot/freeldr/freeldr/arch/i386/hwacpi.c b/boot/freeldr/freeldr/arch/i386/hwacpi.c index 345e2c607aa..9b471116629 100644 --- a/boot/freeldr/freeldr/arch/i386/hwacpi.c +++ b/boot/freeldr/freeldr/arch/i386/hwacpi.c @@ -75,7 +75,6 @@ DetectAcpiBios(PCONFIGURATION_COMPONENT_DATA SystemKey, ULONG *BusNumber) PartialResourceList = FrLdrHeapAlloc(sizeof(CM_PARTIAL_RESOURCE_LIST) + TableSize, TAG_HW_RESOURCE_LIST); - if (PartialResourceList == NULL) { ERR("Failed to allocate resource descriptor\n"); diff --git a/boot/freeldr/freeldr/arch/i386/hwpci.c b/boot/freeldr/freeldr/arch/i386/hwpci.c index c4f0e97488c..035572b4996 100644 --- a/boot/freeldr/freeldr/arch/i386/hwpci.c +++ b/boot/freeldr/freeldr/arch/i386/hwpci.c @@ -229,7 +229,8 @@ DetectPciBios(PCONFIGURATION_COMPONENT_DATA SystemKey, ULONG *BusNumber) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (!PartialResourceList) { - ERR("Failed to allocate resource descriptor\n"); + ERR("Failed to allocate resource descriptor! Ignoring remaining PCI buses. (i = %lu, NoBuses = %lu)\n", + i, (ULONG)BusData.NoBuses); return; }
@@ -254,7 +255,8 @@ DetectPciBios(PCONFIGURATION_COMPONENT_DATA SystemKey, ULONG *BusNumber) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (!PartialResourceList) { - ERR("Failed to allocate resource descriptor\n"); + ERR("Failed to allocate resource descriptor! Ignoring remaining PCI buses. (i = %lu, NoBuses = %lu)\n", + i, (ULONG)BusData.NoBuses); return; }
diff --git a/boot/freeldr/freeldr/arch/i386/pc/machpc.c b/boot/freeldr/freeldr/arch/i386/pc/machpc.c index 6b2ee3d7508..2f0b3563b87 100644 --- a/boot/freeldr/freeldr/arch/i386/pc/machpc.c +++ b/boot/freeldr/freeldr/arch/i386/pc/machpc.c @@ -236,9 +236,9 @@ DetectPnpBios(PCONFIGURATION_COMPONENT_DATA SystemKey, ULONG *BusNumber) ERR("Failed to allocate resource descriptor\n"); return; } - memset(PartialResourceList, 0, Size);
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, Size); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = 1; @@ -689,12 +689,13 @@ DetectSerialPorts(PCONFIGURATION_COMPONENT_DATA BusKey, GET_SERIAL_PORT MachGetS PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (PartialResourceList == NULL) { - ERR("Failed to allocate resource descriptor\n"); - continue; + ERR("Failed to allocate resource descriptor! Ignoring remaining serial ports. (i = %lu, Count = %lu)\n", + i, Count); + break; } - memset(PartialResourceList, 0, Size);
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, Size); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = 3; @@ -792,12 +793,12 @@ DetectParallelPorts(PCONFIGURATION_COMPONENT_DATA BusKey) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (PartialResourceList == NULL) { - ERR("Failed to allocate resource descriptor\n"); - continue; + ERR("Failed to allocate resource descriptor! Ignoring remaining parallel ports. (i = %lu)\n", i); + break; } - memset(PartialResourceList, 0, Size);
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, Size); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = (Irq[i] != (ULONG) - 1) ? 2 : 1; @@ -1173,9 +1174,9 @@ DetectPS2Mouse(PCONFIGURATION_COMPONENT_DATA BusKey) ERR("Failed to allocate resource descriptor\n"); return; } - memset(PartialResourceList, 0, sizeof(CM_PARTIAL_RESOURCE_LIST));
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, sizeof(CM_PARTIAL_RESOURCE_LIST)); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = 1; diff --git a/boot/freeldr/freeldr/arch/i386/pc/pchw.c b/boot/freeldr/freeldr/arch/i386/pc/pchw.c index bed2a11f62b..f69de0f3c90 100644 --- a/boot/freeldr/freeldr/arch/i386/pc/pchw.c +++ b/boot/freeldr/freeldr/arch/i386/pc/pchw.c @@ -230,7 +230,8 @@ DetectBiosFloppyPeripheral(PCONFIGURATION_COMPONENT_DATA ControllerKey) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (PartialResourceList == NULL) { - ERR("Failed to allocate resource descriptor\n"); + ERR("Failed to allocate resource descriptor! Ignoring remaining floppy peripherals. (FloppyNumber = %u)\n", + FloppyNumber); return; }
@@ -288,9 +289,9 @@ DetectBiosFloppyController(PCONFIGURATION_COMPONENT_DATA BusKey) ERR("Failed to allocate resource descriptor\n"); return NULL; } - memset(PartialResourceList, 0, Size);
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, Size); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = 3; diff --git a/boot/freeldr/freeldr/arch/i386/pc98/pc98hw.c b/boot/freeldr/freeldr/arch/i386/pc98/pc98hw.c index f0e2c1c0d43..8e4f2355e57 100644 --- a/boot/freeldr/freeldr/arch/i386/pc98/pc98hw.c +++ b/boot/freeldr/freeldr/arch/i386/pc98/pc98hw.c @@ -176,7 +176,8 @@ DetectBiosFloppyPeripheral(PCONFIGURATION_COMPONENT_DATA ControllerKey) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (PartialResourceList == NULL) { - ERR("Failed to allocate resource descriptor\n"); + ERR("Failed to allocate resource descriptor! Ignoring remaining floppy peripherals. (FloppyNumber = %u, FloppyCount = %u)\n", + FloppyNumber, Pc98GetFloppyCount()); return; } RtlZeroMemory(PartialResourceList, Size); @@ -1113,9 +1114,9 @@ DetectPnpBios(PCONFIGURATION_COMPONENT_DATA SystemKey, ULONG *BusNumber) ERR("Failed to allocate resource descriptor\n"); return; } - RtlZeroMemory(PartialResourceList, Size);
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, Size); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = 1; diff --git a/boot/freeldr/freeldr/arch/i386/xbox/machxbox.c b/boot/freeldr/freeldr/arch/i386/xbox/machxbox.c index eb0f2df4bc8..e2f615593cd 100644 --- a/boot/freeldr/freeldr/arch/i386/xbox/machxbox.c +++ b/boot/freeldr/freeldr/arch/i386/xbox/machxbox.c @@ -106,7 +106,7 @@ XboxGetHarddiskConfigurationData(UCHAR DriveNumber, ULONG* pSize) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (PartialResourceList == NULL) { - ERR("Failed to allocate a full resource descriptor\n"); + ERR("Failed to allocate resource descriptor\n"); return NULL; }
@@ -175,9 +175,9 @@ DetectDisplayController(PCONFIGURATION_COMPONENT_DATA BusKey) ERR("Failed to allocate resource descriptor\n"); return; } - memset(PartialResourceList, 0, Size);
/* Initialize resource descriptor */ + RtlZeroMemory(PartialResourceList, Size); PartialResourceList->Version = 1; PartialResourceList->Revision = 1; PartialResourceList->Count = 1; @@ -218,7 +218,7 @@ DetectIsaBios(PCONFIGURATION_COMPONENT_DATA SystemKey, ULONG *BusNumber) PartialResourceList = FrLdrHeapAlloc(Size, TAG_HW_RESOURCE_LIST); if (PartialResourceList == NULL) { - TRACE("Failed to allocate resource descriptor\n"); + ERR("Failed to allocate resource descriptor\n"); return; }
diff --git a/boot/freeldr/freeldr/lib/peloader.c b/boot/freeldr/freeldr/lib/peloader.c index 64ef1103562..0b5ca0f87b2 100644 --- a/boot/freeldr/freeldr/lib/peloader.c +++ b/boot/freeldr/freeldr/lib/peloader.c @@ -19,8 +19,8 @@ /* INCLUDES ******************************************************************/
#include <freeldr.h> -#include <debug.h>
+#include <debug.h> DBG_DEFAULT_CHANNEL(PELOADER);
/* PRIVATE FUNCTIONS *********************************************************/ @@ -590,7 +590,7 @@ PeLdrAllocateDataTableEntry( OUT PLDR_DATA_TABLE_ENTRY *NewEntry) { PVOID BaseVA = PaToVa(BasePA); - PWSTR Buffer; + PWSTR BaseDllNameBuffer, Buffer; PLDR_DATA_TABLE_ENTRY DataTableEntry; PIMAGE_NT_HEADERS NtHeaders; USHORT Length; @@ -603,12 +603,12 @@ PeLdrAllocateDataTableEntry( TAG_WLDR_DTE); if (DataTableEntry == NULL) return FALSE; - RtlZeroMemory(DataTableEntry, sizeof(LDR_DATA_TABLE_ENTRY));
/* Get NT headers from the image */ NtHeaders = RtlImageNtHeader(BasePA);
/* Initialize corresponding fields of DTE based on NT headers value */ + RtlZeroMemory(DataTableEntry, sizeof(LDR_DATA_TABLE_ENTRY)); DataTableEntry->DllBase = BaseVA; DataTableEntry->SizeOfImage = NtHeaders->OptionalHeader.SizeOfImage; DataTableEntry->EntryPoint = RVA(BaseVA, NtHeaders->OptionalHeader.AddressOfEntryPoint); @@ -624,12 +624,17 @@ PeLdrAllocateDataTableEntry( FrLdrHeapFree(DataTableEntry, TAG_WLDR_DTE); return FALSE; } - RtlZeroMemory(Buffer, Length); + + /* Save Buffer, in case of later failure */ + BaseDllNameBuffer = Buffer;
DataTableEntry->BaseDllName.Length = Length; DataTableEntry->BaseDllName.MaximumLength = Length; DataTableEntry->BaseDllName.Buffer = PaToVa(Buffer); - while (*BaseDllName != 0) + + RtlZeroMemory(Buffer, Length); + Length /= sizeof(WCHAR); + while (Length--) { *Buffer++ = *BaseDllName++; } @@ -640,15 +645,18 @@ PeLdrAllocateDataTableEntry( Buffer = (PWSTR)FrLdrHeapAlloc(Length, TAG_WLDR_NAME); if (Buffer == NULL) { + FrLdrHeapFree(BaseDllNameBuffer, TAG_WLDR_NAME); FrLdrHeapFree(DataTableEntry, TAG_WLDR_DTE); return FALSE; } - RtlZeroMemory(Buffer, Length);
DataTableEntry->FullDllName.Length = Length; DataTableEntry->FullDllName.MaximumLength = Length; DataTableEntry->FullDllName.Buffer = PaToVa(Buffer); - while (*FullDllName != 0) + + RtlZeroMemory(Buffer, Length); + Length /= sizeof(WCHAR); + while (Length--) { *Buffer++ = *FullDllName++; } @@ -679,7 +687,7 @@ PeLdrAllocateDataTableEntry( /* Insert this DTE to a list in the LPB */ InsertTailList(ModuleListHead, &DataTableEntry->InLoadOrderLinks); TRACE("Inserting DTE %p, name='%.*S' DllBase=%p\n", DataTableEntry, - DataTableEntry->BaseDllName.Length / 2, + DataTableEntry->BaseDllName.Length / sizeof(WCHAR), VaToPa(DataTableEntry->BaseDllName.Buffer), DataTableEntry->DllBase);
diff --git a/boot/freeldr/freeldr/ntldr/winldr.c b/boot/freeldr/freeldr/ntldr/winldr.c index 68036794941..a4238a4555a 100644 --- a/boot/freeldr/freeldr/ntldr/winldr.c +++ b/boot/freeldr/freeldr/ntldr/winldr.c @@ -170,6 +170,12 @@ WinLdrInitializePhase1(PLOADER_PARAMETER_BLOCK LoaderBlock,
/* Allocate the ARC structure */ ArcDiskSig = FrLdrHeapAlloc(sizeof(ARC_DISK_SIGNATURE_EX), 'giSD'); + if (!ArcDiskSig) + { + ERR("Failed to allocate ARC structure! Ignoring remaining ARC disks. (i = %lu, DiskCount = %lu)\n", + i, reactos_disk_count); + break; + }
/* Copy the data over */ RtlCopyMemory(ArcDiskSig, &reactos_arc_disk_info[i], sizeof(ARC_DISK_SIGNATURE_EX)); diff --git a/boot/freeldr/freeldr/ntldr/wlregistry.c b/boot/freeldr/freeldr/ntldr/wlregistry.c index 5fbe864d3df..7d9d9219183 100644 --- a/boot/freeldr/freeldr/ntldr/wlregistry.c +++ b/boot/freeldr/freeldr/ntldr/wlregistry.c @@ -536,10 +536,19 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead, /* Get the Name Group */ BufferSize = 4096; GroupNameBuffer = FrLdrHeapAlloc(BufferSize, TAG_WLDR_NAME); + if (!GroupNameBuffer) + { + TRACE_CH(REACTOS, "Failed to allocate buffer\n"); + return; + } rc = RegQueryValue(hGroupKey, L"List", NULL, (PUCHAR)GroupNameBuffer, &BufferSize); TRACE_CH(REACTOS, "RegQueryValue(): rc %d\n", (int)rc); if (rc != ERROR_SUCCESS) + { + TRACE_CH(REACTOS, "Failed to query the 'List' value (rc %d)\n", (int)rc); + FrLdrHeapFree(GroupNameBuffer, TAG_WLDR_NAME); return; + } TRACE_CH(REACTOS, "BufferSize: %d\n", (int)BufferSize); TRACE_CH(REACTOS, "GroupNameBuffer: '%S'\n", GroupNameBuffer);
@@ -775,7 +784,6 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, USHORT PathLength;
BootDriverEntry = FrLdrHeapAlloc(sizeof(BOOT_DRIVER_LIST_ENTRY), TAG_WLDR_BDE); - if (!BootDriverEntry) return FALSE;
@@ -792,7 +800,6 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, BootDriverEntry->FilePath.Length = 0; BootDriverEntry->FilePath.MaximumLength = PathLength; BootDriverEntry->FilePath.Buffer = FrLdrHeapAlloc(PathLength, TAG_WLDR_NAME); - if (!BootDriverEntry->FilePath.Buffer) { FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); @@ -814,10 +821,9 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, BootDriverEntry->FilePath.Length = 0; BootDriverEntry->FilePath.MaximumLength = PathLength; BootDriverEntry->FilePath.Buffer = FrLdrHeapAlloc(PathLength, TAG_WLDR_NAME); - if (!BootDriverEntry->FilePath.Buffer) { - FrLdrHeapFree(BootDriverEntry, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; }
@@ -825,7 +831,7 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, if (!NT_SUCCESS(Status)) { FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); - FrLdrHeapFree(BootDriverEntry, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; }
@@ -833,7 +839,7 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, if (!NT_SUCCESS(Status)) { FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); - FrLdrHeapFree(BootDriverEntry, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; }
@@ -841,7 +847,7 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, if (!NT_SUCCESS(Status)) { FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); - FrLdrHeapFree(BootDriverEntry, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; } } @@ -852,22 +858,36 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, BootDriverEntry->RegistryPath.MaximumLength = PathLength; BootDriverEntry->RegistryPath.Buffer = FrLdrHeapAlloc(PathLength, TAG_WLDR_NAME); if (!BootDriverEntry->RegistryPath.Buffer) + { + FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; + }
Status = RtlAppendUnicodeToString(&BootDriverEntry->RegistryPath, RegistryPath); if (!NT_SUCCESS(Status)) + { + FrLdrHeapFree(BootDriverEntry->RegistryPath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; + }
Status = RtlAppendUnicodeToString(&BootDriverEntry->RegistryPath, ServiceName); if (!NT_SUCCESS(Status)) + { + FrLdrHeapFree(BootDriverEntry->RegistryPath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); return FALSE; + }
// Insert entry into the list if (!InsertInBootDriverList(BootDriverListHead, BootDriverEntry)) { // It was already there, so delete our entry - if (BootDriverEntry->FilePath.Buffer) FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); - if (BootDriverEntry->RegistryPath.Buffer) FrLdrHeapFree(BootDriverEntry->RegistryPath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry->RegistryPath.Buffer, TAG_WLDR_NAME); + FrLdrHeapFree(BootDriverEntry->FilePath.Buffer, TAG_WLDR_NAME); FrLdrHeapFree(BootDriverEntry, TAG_WLDR_BDE); }
-
Serge Gautherie