From 187400d0ae35bb0c2e2c7bcbdc8751224de305de Mon Sep 17 00:00:00 2001 From: jief Date: Sat, 11 Nov 2023 15:57:02 +0100 Subject: [PATCH] Why did I put the FreePool parameter as const. That's stupid ! You must never free a const pointer, by definition. --- MdePkg/Include/Library/MemoryAllocationLib.h | 5 ++++- rEFIt_UEFI/entry_scan/loader.cpp | 7 ++++--- rEFIt_UEFI/libeg/XTheme.cpp | 7 ++++--- rEFIt_UEFI/refit/main.cpp | 4 +++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/MdePkg/Include/Library/MemoryAllocationLib.h b/MdePkg/Include/Library/MemoryAllocationLib.h index c684bb3d4..20d1f4cb5 100644 --- a/MdePkg/Include/Library/MemoryAllocationLib.h +++ b/MdePkg/Include/Library/MemoryAllocationLib.h @@ -478,10 +478,13 @@ ReallocateReservedPool ( @param Buffer Pointer to the buffer to free. **/ +/* + * Jief : One day, I put JCONST. This is wrong !!! You must never free a ptr that's const. + */ VOID EFIAPI FreePool( - IN JCONST VOID *Buffer + IN VOID *Buffer ); #endif diff --git a/rEFIt_UEFI/entry_scan/loader.cpp b/rEFIt_UEFI/entry_scan/loader.cpp index c63e49592..d994c43b8 100644 --- a/rEFIt_UEFI/entry_scan/loader.cpp +++ b/rEFIt_UEFI/entry_scan/loader.cpp @@ -681,7 +681,8 @@ MacOsVersion GetOSVersion(int LoaderType, const EFI_GUID& APFSTargetUUID, const // Check for ia.log - InstallESD/createinstallmedia/startosinstall // Implemented by Sherlocks if (OSVersion.isEmpty()) { - CONST CHAR8 *s, *fileBuffer; + CONST CHAR8 *s; + UINT8* fileBuffer; // CHAR8 *Res5 = (__typeof__(Res5))AllocateZeroPool(5); // CHAR8 *Res6 = (__typeof__(Res6))AllocateZeroPool(6); // CHAR8 *Res7 = (__typeof__(Res7))AllocateZeroPool(7); @@ -696,10 +697,10 @@ MacOsVersion GetOSVersion(int LoaderType, const EFI_GUID& APFSTargetUUID, const } } if (FileExists (Volume->RootDir, InstallerLog)) { - Status = egLoadFile(Volume->RootDir, InstallerLog.wc_str(), (UINT8 **)&fileBuffer, &fileLen); + Status = egLoadFile(Volume->RootDir, InstallerLog.wc_str(), &fileBuffer, &fileLen); if (!EFI_ERROR(Status)) { XString8 targetString; - targetString.strncpy(fileBuffer, fileLen); + targetString.strncpy((CHAR8*)fileBuffer, fileLen); // s = SearchString(targetString, fileLen, "Running OS Build: Mac OS X ", 27); s = AsciiStrStr(targetString.c_str(), "Running OS Build: Mac OS X "); if (s[31] == ' ') { diff --git a/rEFIt_UEFI/libeg/XTheme.cpp b/rEFIt_UEFI/libeg/XTheme.cpp index 896d23ed0..e834691c7 100644 --- a/rEFIt_UEFI/libeg/XTheme.cpp +++ b/rEFIt_UEFI/libeg/XTheme.cpp @@ -125,8 +125,8 @@ EFI_STATUS Status = EFI_NOT_FOUND; } TestTheme.setEmpty(); } - FreePool(ChosenTheme); - ChosenTheme = NULL; + //FreePool(ChosenTheme); // ChosenTheme is an argument passed here, so the callee is "own" that memory. + //ChosenTheme = NULL; // Why is this bad pratice : it's assuming that ChosenTheme was dynamically allocated and freeable. What is this the content of an XString ? } // Try to get theme from settings if (ThemeDict == NULL) { @@ -204,7 +204,8 @@ finish: } } if (ChosenTheme != NULL) { - FreePool(ChosenTheme); + //FreePool(ChosenTheme); // ChosenTheme is an argument passed here, so the callee is "own" that memory. + // Why is this bad pratice : it's assuming that ChosenTheme was dynamically allocated and freeable. What is this the content of an XString ? } if (!ThemeX->TypeSVG) { ThemeX->PrepareFont(); diff --git a/rEFIt_UEFI/refit/main.cpp b/rEFIt_UEFI/refit/main.cpp index cc86f9e1c..9de5046e5 100644 --- a/rEFIt_UEFI/refit/main.cpp +++ b/rEFIt_UEFI/refit/main.cpp @@ -3030,7 +3030,9 @@ RefitMain (IN EFI_HANDLE ImageHandle, if (!GlobalConfig.isFastBoot()) { if (gThemeNeedInit) { UINTN Size = 0; - InitTheme((CHAR8*)GetNvramVariable(L"Clover.Theme", gEfiAppleBootGuid, NULL, &Size)); + CHAR8* ChoosenTheme = (CHAR8*)GetNvramVariable(L"Clover.Theme", gEfiAppleBootGuid, NULL, &Size); + InitTheme(ChoosenTheme); + FreePool(ChoosenTheme); gThemeNeedInit = false; } else if (GlobalConfig.gThemeChanged) { DBG("change theme\n");