From 423d1e2e091ba08b08925bf35985d7440a4a8fb3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Wed, 14 Aug 2019 15:59:01 +0200 Subject: [PATCH] [FREELDR] Minor fixes. - Add some forgotten, or move some misplaced ArcClose() calls so as to avoid leakages in failure paths. (Thanks to Serge Gautherie for having caught some of them.) - Simplify some code; remove unneeded casts; use string-safe functions. --- boot/freeldr/freeldr/lib/inifile/ini_init.c | 1 + boot/freeldr/freeldr/miscboot.c | 19 +-- boot/freeldr/freeldr/ntldr/wlregistry.c | 129 +++++++++----------- 3 files changed, 74 insertions(+), 75 deletions(-) diff --git a/boot/freeldr/freeldr/lib/inifile/ini_init.c b/boot/freeldr/freeldr/lib/inifile/ini_init.c index 52a24bcdfdc..f9c7e1c26ad 100644 --- a/boot/freeldr/freeldr/lib/inifile/ini_init.c +++ b/boot/freeldr/freeldr/lib/inifile/ini_init.c @@ -63,6 +63,7 @@ BOOLEAN IniFileInitialize(VOID) if (Status != ESUCCESS || FileInformation.EndingAddress.HighPart != 0) { UiMessageBoxCritical("Error while getting informations about freeldr.ini.\nYou need to re-install FreeLoader."); + ArcClose(FileId); return FALSE; } FreeLoaderIniFileSize = FileInformation.EndingAddress.LowPart; diff --git a/boot/freeldr/freeldr/miscboot.c b/boot/freeldr/freeldr/miscboot.c index 535f9d6efab..f7a0e2a912e 100644 --- a/boot/freeldr/freeldr/miscboot.c +++ b/boot/freeldr/freeldr/miscboot.c @@ -31,6 +31,7 @@ LoadAndBootBootSector( IN PCHAR Argv[], IN PCHAR Envp[]) { + ARC_STATUS Status; PCSTR FileName; ULONG FileId; ULONG BytesRead; @@ -54,14 +55,14 @@ LoadAndBootBootSector( } /* Read boot sector */ - if ((ArcRead(FileId, (PVOID)0x7c00, 512, &BytesRead) != ESUCCESS) || (BytesRead != 512)) + Status = ArcRead(FileId, (PVOID)0x7c00, 512, &BytesRead); + ArcClose(FileId); + if ((Status != ESUCCESS) || (BytesRead != 512)) { UiMessageBox("Unable to read boot sector."); return EIO; } - ArcClose(FileId); - /* Check for validity */ if (*((USHORT*)(0x7c00 + 0x1fe)) != 0xaa55) { @@ -93,6 +94,7 @@ LoadAndBootPartitionOrDrive( IN UCHAR DriveNumber, IN ULONG PartitionNumber OPTIONAL) { + ARC_STATUS Status; ULONG FileId; ULONG BytesRead; CHAR ArcPath[MAX_PATH]; @@ -100,7 +102,10 @@ LoadAndBootPartitionOrDrive( /* Construct the corresponding ARC path */ ConstructArcPath(ArcPath, "", DriveNumber, PartitionNumber); *strrchr(ArcPath, '\\') = ANSI_NULL; // Trim the trailing path separator. - if (ArcOpen(ArcPath, OpenReadOnly, &FileId) != ESUCCESS) + + /* Open the volume */ + Status = ArcOpen(ArcPath, OpenReadOnly, &FileId); + if (Status != ESUCCESS) { UiMessageBox("Unable to open %s", ArcPath); return ENOENT; @@ -110,7 +115,9 @@ LoadAndBootPartitionOrDrive( * Now try to read the partition boot sector or the MBR (when PartitionNumber == 0). * If this fails then abort. */ - if ((ArcRead(FileId, (PVOID)0x7c00, 512, &BytesRead) != ESUCCESS) || (BytesRead != 512)) + Status = ArcRead(FileId, (PVOID)0x7c00, 512, &BytesRead); + ArcClose(FileId); + if ((Status != ESUCCESS) || (BytesRead != 512)) { if (PartitionNumber != 0) UiMessageBox("Unable to read partition's boot sector."); @@ -119,8 +126,6 @@ LoadAndBootPartitionOrDrive( return EIO; } - ArcClose(FileId); - /* Check for validity */ if (*((USHORT*)(0x7c00 + 0x1fe)) != 0xaa55) { diff --git a/boot/freeldr/freeldr/ntldr/wlregistry.c b/boot/freeldr/freeldr/ntldr/wlregistry.c index 1e39ec78961..c4a3e0ba9c9 100644 --- a/boot/freeldr/freeldr/ntldr/wlregistry.c +++ b/boot/freeldr/freeldr/ntldr/wlregistry.c @@ -41,14 +41,14 @@ WinLdrLoadSystemHive( ARC_STATUS Status; FILEINFORMATION FileInfo; ULONG HiveFileSize; - ULONG_PTR HiveDataPhysical; + PVOID HiveDataPhysical; PVOID HiveDataVirtual; ULONG BytesRead; PCWSTR FsService; /* Concatenate path and filename to get the full name */ - strcpy(FullHiveName, DirectoryPath); - strcat(FullHiveName, HiveName); + RtlStringCbCopyA(FullHiveName, sizeof(FullHiveName), DirectoryPath); + RtlStringCbCatA(FullHiveName, sizeof(FullHiveName), HiveName); Status = ArcOpen(FullHiveName, OpenReadOnly, &FileId); if (Status != ESUCCESS) @@ -69,11 +69,11 @@ WinLdrLoadSystemHive( HiveFileSize = FileInfo.EndingAddress.LowPart; /* Round up the size to page boundary and alloc memory */ - HiveDataPhysical = (ULONG_PTR)MmAllocateMemoryWithType( + HiveDataPhysical = MmAllocateMemoryWithType( MM_SIZE_TO_PAGES(HiveFileSize + MM_PAGE_SIZE - 1) << MM_PAGE_SHIFT, LoaderRegistryData); - if (HiveDataPhysical == 0) + if (HiveDataPhysical == NULL) { ArcClose(FileId); UiMessageBox("Unable to alloc memory for a hive!"); @@ -81,14 +81,14 @@ WinLdrLoadSystemHive( } /* Convert address to virtual */ - HiveDataVirtual = PaToVa((PVOID)HiveDataPhysical); + HiveDataVirtual = PaToVa(HiveDataPhysical); /* Fill LoaderBlock's entries */ LoaderBlock->RegistryLength = HiveFileSize; LoaderBlock->RegistryBase = HiveDataVirtual; /* Finally read from file to the memory */ - Status = ArcRead(FileId, (PVOID)HiveDataPhysical, HiveFileSize, &BytesRead); + Status = ArcRead(FileId, HiveDataPhysical, HiveFileSize, &BytesRead); if (Status != ESUCCESS) { ArcClose(FileId); @@ -131,7 +131,7 @@ WinLdrInitSystemHive( if (Setup) { - strcpy(SearchPath, SystemRoot); + RtlStringCbCopyA(SearchPath, sizeof(SearchPath), SystemRoot); HiveName = "SETUPREG.HIV"; } else @@ -140,8 +140,8 @@ WinLdrInitSystemHive( // fails, then give system.alt a try, and finally try a system.sav // FIXME: For now we only try system - strcpy(SearchPath, SystemRoot); - strcat(SearchPath, "system32\\config\\"); + RtlStringCbCopyA(SearchPath, sizeof(SearchPath), SystemRoot); + RtlStringCbCatA(SearchPath, sizeof(SearchPath), "system32\\config\\"); HiveName = "SYSTEM"; } @@ -191,8 +191,8 @@ BOOLEAN WinLdrScanSystemHive(IN OUT PLOADER_PARAMETER_BLOCK LoaderBlock, TRACE("NLS data %s %s %s\n", AnsiName, OemName, LangName); /* Load NLS data */ - strcpy(SearchPath, SystemRoot); - strcat(SearchPath, "system32\\"); + RtlStringCbCopyA(SearchPath, sizeof(SearchPath), SystemRoot); + RtlStringCbCatA(SearchPath, sizeof(SearchPath), "system32\\"); Success = WinLdrLoadNLSData(LoaderBlock, SearchPath, AnsiName, OemName, LangName); TRACE("NLS data loading %s\n", Success ? "successful" : "failed"); @@ -224,7 +224,7 @@ WinLdrGetNLSNames(PSTR AnsiName, &hKey); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "Couldn't open CodePage registry key"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't open CodePage registry key"); return FALSE; } @@ -233,7 +233,7 @@ WinLdrGetNLSNames(PSTR AnsiName, rc = RegQueryValue(hKey, L"ACP", NULL, (PUCHAR)szIdBuffer, &BufferSize); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "Couldn't get ACP NLS setting"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't get ACP NLS setting"); return FALSE; } @@ -241,7 +241,7 @@ WinLdrGetNLSNames(PSTR AnsiName, rc = RegQueryValue(hKey, szIdBuffer, NULL, (PUCHAR)NameBuffer, &BufferSize); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "ACP NLS Setting exists, but isn't readable"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "ACP NLS Setting exists, but isn't readable"); //return FALSE; wcscpy(NameBuffer, L"c_1252.nls"); // HACK: ReactOS bug CORE-6105 } @@ -252,7 +252,7 @@ WinLdrGetNLSNames(PSTR AnsiName, rc = RegQueryValue(hKey, L"OEMCP", NULL, (PUCHAR)szIdBuffer, &BufferSize); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "Couldn't get OEMCP NLS setting"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't get OEMCP NLS setting"); return FALSE; } @@ -260,7 +260,7 @@ WinLdrGetNLSNames(PSTR AnsiName, rc = RegQueryValue(hKey, szIdBuffer, NULL, (PUCHAR)NameBuffer, &BufferSize); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "OEMCP NLS setting exists, but isn't readable"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "OEMCP NLS setting exists, but isn't readable"); //return FALSE; wcscpy(NameBuffer, L"c_437.nls"); // HACK: ReactOS bug CORE-6105 } @@ -272,7 +272,7 @@ WinLdrGetNLSNames(PSTR AnsiName, &hKey); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "Couldn't open Language registry key"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't open Language registry key"); return FALSE; } @@ -281,7 +281,7 @@ WinLdrGetNLSNames(PSTR AnsiName, rc = RegQueryValue(hKey, L"Default", NULL, (PUCHAR)szIdBuffer, &BufferSize); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "Couldn't get Language Default setting"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't get Language Default setting"); return FALSE; } @@ -289,7 +289,7 @@ WinLdrGetNLSNames(PSTR AnsiName, rc = RegQueryValue(hKey, szIdBuffer, NULL, (PUCHAR)NameBuffer, &BufferSize); if (rc != ERROR_SUCCESS) { - //strcpy(szErrorOut, "Language Default setting exists, but isn't readable"); + //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Language Default setting exists, but isn't readable"); return FALSE; } sprintf(LangName, "%S", NameBuffer); @@ -305,12 +305,10 @@ WinLdrLoadNLSData(IN OUT PLOADER_PARAMETER_BLOCK LoaderBlock, IN PCSTR LanguageFileName) { CHAR FileName[255]; - ULONG AnsiFileId; - ULONG OemFileId; - ULONG LanguageFileId; + ULONG FileId; ULONG AnsiFileSize, OemFileSize, LanguageFileSize; ULONG TotalSize; - ULONG_PTR NlsDataBase; + PVOID NlsDataBase; PVOID NlsVirtual; BOOLEAN AnsiEqualsOem = FALSE; FILEINFORMATION FileInfo; @@ -322,21 +320,21 @@ WinLdrLoadNLSData(IN OUT PLOADER_PARAMETER_BLOCK LoaderBlock, AnsiEqualsOem = TRUE; /* Open file with ANSI and store its size */ - strcpy(FileName, DirectoryPath); - strcat(FileName, AnsiFileName); - Status = ArcOpen(FileName, OpenReadOnly, &AnsiFileId); + RtlStringCbCopyA(FileName, sizeof(FileName), DirectoryPath); + RtlStringCbCatA(FileName, sizeof(FileName), AnsiFileName); + Status = ArcOpen(FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { WARN("Error while opening '%s', Status: %u\n", FileName, Status); goto Failure; } - Status = ArcGetFileInformation(AnsiFileId, &FileInfo); + Status = ArcGetFileInformation(FileId, &FileInfo); + ArcClose(FileId); if (Status != ESUCCESS) goto Failure; AnsiFileSize = FileInfo.EndingAddress.LowPart; TRACE("AnsiFileSize: %d\n", AnsiFileSize); - ArcClose(AnsiFileId); /* Open OEM file and store its length */ if (AnsiEqualsOem) @@ -346,128 +344,123 @@ WinLdrLoadNLSData(IN OUT PLOADER_PARAMETER_BLOCK LoaderBlock, else { //Print(L"Loading %s...\n", Filename); - strcpy(FileName, DirectoryPath); - strcat(FileName, OemFileName); - Status = ArcOpen(FileName, OpenReadOnly, &OemFileId); + RtlStringCbCopyA(FileName, sizeof(FileName), DirectoryPath); + RtlStringCbCatA(FileName, sizeof(FileName), OemFileName); + Status = ArcOpen(FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { WARN("Error while opening '%s', Status: %u\n", FileName, Status); goto Failure; } - Status = ArcGetFileInformation(OemFileId, &FileInfo); + Status = ArcGetFileInformation(FileId, &FileInfo); + ArcClose(FileId); if (Status != ESUCCESS) goto Failure; OemFileSize = FileInfo.EndingAddress.LowPart; - ArcClose(OemFileId); } TRACE("OemFileSize: %d\n", OemFileSize); /* And finally open the language codepage file and store its length */ //Print(L"Loading %s...\n", Filename); - strcpy(FileName, DirectoryPath); - strcat(FileName, LanguageFileName); - Status = ArcOpen(FileName, OpenReadOnly, &LanguageFileId); + RtlStringCbCopyA(FileName, sizeof(FileName), DirectoryPath); + RtlStringCbCatA(FileName, sizeof(FileName), LanguageFileName); + Status = ArcOpen(FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { WARN("Error while opening '%s', Status: %u\n", FileName, Status); goto Failure; } - Status = ArcGetFileInformation(LanguageFileId, &FileInfo); + Status = ArcGetFileInformation(FileId, &FileInfo); + ArcClose(FileId); if (Status != ESUCCESS) goto Failure; LanguageFileSize = FileInfo.EndingAddress.LowPart; - ArcClose(LanguageFileId); TRACE("LanguageFileSize: %d\n", LanguageFileSize); /* Sum up all three length, having in mind that every one of them must start at a page boundary => thus round up each file to a page */ TotalSize = MM_SIZE_TO_PAGES(AnsiFileSize) + - MM_SIZE_TO_PAGES(OemFileSize) + - MM_SIZE_TO_PAGES(LanguageFileSize); + MM_SIZE_TO_PAGES(OemFileSize) + + MM_SIZE_TO_PAGES(LanguageFileSize); /* Store it for later marking the pages as NlsData type */ TotalNLSSize = TotalSize; - NlsDataBase = (ULONG_PTR)MmAllocateMemoryWithType(TotalSize*MM_PAGE_SIZE, LoaderNlsData); - - if (NlsDataBase == 0) + NlsDataBase = MmAllocateMemoryWithType(TotalSize*MM_PAGE_SIZE, LoaderNlsData); + if (NlsDataBase == NULL) goto Failure; - NlsVirtual = PaToVa((PVOID)NlsDataBase); + NlsVirtual = PaToVa(NlsDataBase); LoaderBlock->NlsData->AnsiCodePageData = NlsVirtual; - LoaderBlock->NlsData->OemCodePageData = (PVOID)((PUCHAR)NlsVirtual + + LoaderBlock->NlsData->OemCodePageData = (PVOID)((ULONG_PTR)NlsVirtual + (MM_SIZE_TO_PAGES(AnsiFileSize) << MM_PAGE_SHIFT)); - LoaderBlock->NlsData->UnicodeCodePageData = (PVOID)((PUCHAR)NlsVirtual + + LoaderBlock->NlsData->UnicodeCodePageData = (PVOID)((ULONG_PTR)NlsVirtual + (MM_SIZE_TO_PAGES(AnsiFileSize) << MM_PAGE_SHIFT) + - (MM_SIZE_TO_PAGES(OemFileSize) << MM_PAGE_SHIFT)); + (MM_SIZE_TO_PAGES(OemFileSize) << MM_PAGE_SHIFT)); /* Ansi and OEM data are the same - just set pointers to the same area */ if (AnsiEqualsOem) LoaderBlock->NlsData->OemCodePageData = LoaderBlock->NlsData->AnsiCodePageData; - /* Now actually read the data into memory, starting with Ansi file */ - strcpy(FileName, DirectoryPath); - strcat(FileName, AnsiFileName); - Status = ArcOpen(FileName, OpenReadOnly, &AnsiFileId); + RtlStringCbCopyA(FileName, sizeof(FileName), DirectoryPath); + RtlStringCbCatA(FileName, sizeof(FileName), AnsiFileName); + Status = ArcOpen(FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { WARN("Error while opening '%s', Status: %u\n", FileName, Status); goto Failure; } - Status = ArcRead(AnsiFileId, VaToPa(LoaderBlock->NlsData->AnsiCodePageData), AnsiFileSize, &BytesRead); + Status = ArcRead(FileId, VaToPa(LoaderBlock->NlsData->AnsiCodePageData), AnsiFileSize, &BytesRead); + ArcClose(FileId); if (Status != ESUCCESS) { WARN("Error while reading '%s', Status: %u\n", FileName, Status); goto Failure; } - ArcClose(AnsiFileId); - /* OEM now, if it doesn't equal Ansi of course */ if (!AnsiEqualsOem) { - strcpy(FileName, DirectoryPath); - strcat(FileName, OemFileName); - Status = ArcOpen(FileName, OpenReadOnly, &OemFileId); + RtlStringCbCopyA(FileName, sizeof(FileName), DirectoryPath); + RtlStringCbCatA(FileName, sizeof(FileName), OemFileName); + Status = ArcOpen(FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { WARN("Error while opening '%s', Status: %u\n", FileName, Status); goto Failure; } - Status = ArcRead(OemFileId, VaToPa(LoaderBlock->NlsData->OemCodePageData), OemFileSize, &BytesRead); + Status = ArcRead(FileId, VaToPa(LoaderBlock->NlsData->OemCodePageData), OemFileSize, &BytesRead); + ArcClose(FileId); if (Status != ESUCCESS) { WARN("Error while reading '%s', Status: %u\n", FileName, Status); goto Failure; } - - ArcClose(OemFileId); } - /* finally the language file */ - strcpy(FileName, DirectoryPath); - strcat(FileName, LanguageFileName); - Status = ArcOpen(FileName, OpenReadOnly, &LanguageFileId); + /* Finally the language file */ + RtlStringCbCopyA(FileName, sizeof(FileName), DirectoryPath); + RtlStringCbCatA(FileName, sizeof(FileName), LanguageFileName); + Status = ArcOpen(FileName, OpenReadOnly, &FileId); if (Status != ESUCCESS) { WARN("Error while opening '%s', Status: %u\n", FileName, Status); goto Failure; } - Status = ArcRead(LanguageFileId, VaToPa(LoaderBlock->NlsData->UnicodeCodePageData), LanguageFileSize, &BytesRead); + Status = ArcRead(FileId, VaToPa(LoaderBlock->NlsData->UnicodeCodePageData), LanguageFileSize, &BytesRead); + ArcClose(FileId); if (Status != ESUCCESS) { WARN("Error while reading '%s', Status: %u\n", FileName, Status); goto Failure; } - ArcClose(LanguageFileId); - // // THIS IS HAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACK // Should go to WinLdrLoadOemHalFont(), when it will be implemented -- 2.17.1