From 125d8e8df36bd7ca12085d9670dca9060cf49c76 Mon Sep 17 00:00:00 2001 From: Mike Beaton Date: Sat, 18 Nov 2023 19:42:20 +0000 Subject: [PATCH] OcBootManagementLib: Store ImageLoaderCaps in protocol (#504) --- Changelog.md | 1 + .../OcBootManagementLib/BootEntryManagement.c | 4 + Library/OcBootManagementLib/ImageLoader.c | 161 +++++++++++------- 3 files changed, 109 insertions(+), 57 deletions(-) diff --git a/Changelog.md b/Changelog.md index f4dee41d..a4e6768d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ OpenCore Changelog - Updated `FixupAppleEfiImages` quirk to fix `W^X` errors in all non-Secure Boot Apple signed binaries - Updated builtin firmware versions for SMBIOS and the rest - Updated `AppleEfiSignTool` to work with new PE COFF loader +- Fixed recovery failing to boot on some systems #### v0.9.6 - Updated builtin firmware versions for SMBIOS and the rest diff --git a/Library/OcBootManagementLib/BootEntryManagement.c b/Library/OcBootManagementLib/BootEntryManagement.c index 6f6d62ee..57cfb518 100644 --- a/Library/OcBootManagementLib/BootEntryManagement.c +++ b/Library/OcBootManagementLib/BootEntryManagement.c @@ -2548,6 +2548,10 @@ OcLoadBootEntry ( // Unload dmg if any. // InternalUnloadDmg (&DmgLoadContext); + // + // Unload image. + // + gBS->UnloadImage (EntryHandle); } } else { DEBUG ((DEBUG_WARN, "OCB: LoadImage failed - %r\n", Status)); diff --git a/Library/OcBootManagementLib/ImageLoader.c b/Library/OcBootManagementLib/ImageLoader.c index adfe0efe..aa7f60c5 100644 --- a/Library/OcBootManagementLib/ImageLoader.c +++ b/Library/OcBootManagementLib/ImageLoader.c @@ -56,6 +56,10 @@ STATIC EFI_GUID mOcLoadedImageProtocolGuid = { 0x1f3c963d, 0xf9dc, 0x4537, { 0xbb, 0x06, 0xd8, 0x08, 0x46, 0x4a, 0x85, 0x2e } }; +STATIC EFI_GUID mOcImageLoaderCapsProtocolGuid = { + 0xf5bbca36, 0x0f99, 0x4e7b, { 0x86, 0x0f, 0x92, 0x06, 0xa9, 0x3b, 0x52, 0xd0 } +}; + typedef struct { EFI_IMAGE_ENTRY_POINT EntryPoint; EFI_PHYSICAL_ADDRESS ImageArea; @@ -70,6 +74,10 @@ typedef struct { EFI_LOADED_IMAGE_PROTOCOL LoadedImage; } OC_LOADED_IMAGE_PROTOCOL; +typedef struct { + UINT32 ImageCaps; +} OC_IMAGE_LOADER_CAPS_PROTOCOL; + STATIC EFI_IMAGE_LOAD mOriginalEfiLoadImage; STATIC EFI_IMAGE_START mOriginalEfiStartImage; STATIC EFI_IMAGE_UNLOAD mOriginalEfiUnloadImage; @@ -78,8 +86,6 @@ STATIC EFI_HANDLE mCurrentImageHandle; STATIC OC_IMAGE_LOADER_PATCH mImageLoaderPatch; STATIC OC_IMAGE_LOADER_CONFIGURE mImageLoaderConfigure; -STATIC UINT32 mImageLoaderCaps; -STATIC EFI_HANDLE mImageLoaderCapsHandle; STATIC BOOLEAN mImageLoaderEnabled; STATIC BOOLEAN mProtectUefiServices; @@ -272,17 +278,6 @@ OcImageLoaderLoad ( OC_LOADED_IMAGE_PROTOCOL *OcLoadedImage; EFI_LOADED_IMAGE_PROTOCOL *LoadedImage; - // - // For OcImageLoaderLoad always assume target default. - // - #ifdef MDE_CPU_IA32 - mImageLoaderCaps = OC_KERN_CAPABILITY_K32_U32 | OC_KERN_CAPABILITY_K32_U64; - #else - mImageLoaderCaps = OC_KERN_CAPABILITY_K64_U64; - #endif - - mImageLoaderCapsHandle = NULL; - ASSERT (SourceBuffer != NULL); // @@ -424,8 +419,6 @@ OcImageLoaderLoad ( return Status; } - mImageLoaderCapsHandle = *ImageHandle; - DEBUG ((DEBUG_VERBOSE, "OCB: Loaded image at %p\n", *ImageHandle)); return EFI_SUCCESS; @@ -784,15 +777,14 @@ InternalEfiLoadImage ( OUT EFI_HANDLE *ImageHandle ) { - EFI_STATUS SecureBootStatus; - EFI_STATUS FilterStatus; - EFI_STATUS Status; - VOID *AllocatedBuffer; - UINT32 RealSize; - UINT32 SignedFileSize; - BOOLEAN IsFat; - - mImageLoaderCapsHandle = NULL; + EFI_STATUS SecureBootStatus; + EFI_STATUS FilterStatus; + EFI_STATUS Status; + VOID *AllocatedBuffer; + OC_IMAGE_LOADER_CAPS_PROTOCOL *OcImageLoaderCaps; + UINT32 RealSize; + UINT32 SignedFileSize; + BOOLEAN IsFatSlice; if ((ParentImageHandle == NULL) || (ImageHandle == NULL)) { return EFI_INVALID_PARAMETER; @@ -806,7 +798,8 @@ InternalEfiLoadImage ( return EFI_UNSUPPORTED; } - AllocatedBuffer = NULL; + OcImageLoaderCaps = NULL; + AllocatedBuffer = NULL; if (SourceBuffer == NULL) { Status = InternalEfiLoadImageFile ( DevicePath, @@ -848,15 +841,6 @@ InternalEfiLoadImage ( return EFI_SECURITY_VIOLATION; } - // - // By default assume target default. - // - #ifdef MDE_CPU_IA32 - mImageLoaderCaps = OC_KERN_CAPABILITY_K32_U32 | OC_KERN_CAPABILITY_K32_U64; - #else - mImageLoaderCaps = OC_KERN_CAPABILITY_K64_U64; - #endif - if (SourceBuffer != NULL) { RealSize = (UINT32)SourceSize; #ifdef MDE_CPU_IA32 @@ -865,10 +849,19 @@ InternalEfiLoadImage ( FilterStatus = FatFilterArchitecture64 ((UINT8 **)&SourceBuffer, &RealSize); #endif - IsFat = !EFI_ERROR (FilterStatus) && (RealSize != SourceSize) && (RealSize >= EFI_PAGE_SIZE); + IsFatSlice = !EFI_ERROR (FilterStatus) && (RealSize != SourceSize) && (RealSize >= EFI_PAGE_SIZE); - if (IsFat) { - mImageLoaderCaps = DetectCapabilities (SourceBuffer, RealSize); + if (IsFatSlice) { + OcImageLoaderCaps = AllocateZeroPool (sizeof (*OcImageLoaderCaps)); + if (OcImageLoaderCaps == NULL) { + if (AllocatedBuffer != NULL) { + FreePool (AllocatedBuffer); + } + + return EFI_OUT_OF_RESOURCES; + } + + OcImageLoaderCaps->ImageCaps = DetectCapabilities (SourceBuffer, RealSize); } // @@ -879,7 +872,7 @@ InternalEfiLoadImage ( if (SecureBootStatus == EFI_SUCCESS) { DEBUG ((DEBUG_INFO, "OCB: Secure boot, fixup efi ignored\n")); Status = EFI_SUCCESS; - } else if (IsFat) { + } else if (IsFatSlice) { DEBUG ((DEBUG_INFO, "OCB: Fat binary, fixup efi...\n")); Status = OcPatchLegacyEfi (SourceBuffer, RealSize); } else { @@ -924,7 +917,7 @@ InternalEfiLoadImage ( (UINT32)SourceSize, SourceBuffer, RealSize, - mImageLoaderCaps, + OcImageLoaderCaps == NULL ? 0 : OcImageLoaderCaps->ImageCaps, FilterStatus )); @@ -948,7 +941,7 @@ InternalEfiLoadImage ( // Load the image ourselves in secure boot mode. // if (SecureBootStatus == EFI_SUCCESS) { - if (SourceBuffer != NULL) { + if ((SourceBuffer != NULL) && (OcImageLoaderCaps == NULL)) { Status = OcImageLoaderLoad ( FALSE, ParentImageHandle, @@ -959,9 +952,10 @@ InternalEfiLoadImage ( ); } else { // - // We verified the image, but contained garbage. - // This should not happen, just abort. + // We verified the image, but contained garbage, or we are trying to secure boot a Fat slice. + // This should not happen. // + ASSERT (FALSE); Status = EFI_UNSUPPORTED; } } else { @@ -975,6 +969,25 @@ InternalEfiLoadImage ( ImageHandle ); RestoreGrubShimHooks ("LoadImage"); + + if (!EFI_ERROR (Status) && (OcImageLoaderCaps != NULL)) { + Status = gBS->InstallMultipleProtocolInterfaces ( + ImageHandle, + &mOcImageLoaderCapsProtocolGuid, + OcImageLoaderCaps, + NULL + ); + if (!EFI_ERROR (Status)) { + OcImageLoaderCaps = NULL; + } else { + DEBUG ((DEBUG_INFO, "OCB: LoaderCaps proto install error - %r\n", Status)); + mOriginalEfiUnloadImage (ImageHandle); + } + } + } + + if (OcImageLoaderCaps != NULL) { + FreePool (OcImageLoaderCaps); } if (AllocatedBuffer != NULL) { @@ -989,8 +1002,6 @@ InternalEfiLoadImage ( InternalUpdateLoadedImage (*ImageHandle, DevicePath); } - mImageLoaderCapsHandle = *ImageHandle; - return Status; } @@ -1003,16 +1014,20 @@ InternalEfiStartImage ( OUT CHAR16 **ExitData OPTIONAL ) { - EFI_STATUS Status; - OC_LOADED_IMAGE_PROTOCOL *OcLoadedImage; - EFI_LOADED_IMAGE_PROTOCOL *LoadedImage; + EFI_STATUS Status; + OC_LOADED_IMAGE_PROTOCOL *OcLoadedImage; + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage; + OC_IMAGE_LOADER_CAPS_PROTOCOL *OcImageLoaderCaps; + UINT32 Caps; - if ( (mImageLoaderConfigure != NULL) - && (ImageHandle != mImageLoaderCapsHandle)) - { - DEBUG ((DEBUG_ERROR, "OCB: load/start unsupported ordering, %p != %p\n", ImageHandle, mImageLoaderCapsHandle)); - return EFI_INVALID_PARAMETER; - } + // + // Use target default except where we detected and saved other caps on load. + // + #ifdef MDE_CPU_IA32 + Caps = OC_KERN_CAPABILITY_K32_U32 | OC_KERN_CAPABILITY_K32_U64; + #else + Caps = OC_KERN_CAPABILITY_K64_U64; + #endif // // If we loaded the image, invoke the entry point manually. @@ -1029,7 +1044,7 @@ InternalEfiStartImage ( if (mImageLoaderConfigure != NULL) { mImageLoaderConfigure ( &OcLoadedImage->LoadedImage, - mImageLoaderCaps + Caps ); } @@ -1051,9 +1066,18 @@ InternalEfiStartImage ( (VOID **)&LoadedImage ); if (!EFI_ERROR (Status)) { + Status = gBS->HandleProtocol ( + ImageHandle, + &mOcImageLoaderCapsProtocolGuid, + (VOID **)&OcImageLoaderCaps + ); + if (!EFI_ERROR (Status)) { + Caps = OcImageLoaderCaps->ImageCaps; + } + mImageLoaderConfigure ( LoadedImage, - mImageLoaderCaps + Caps ); } } @@ -1072,8 +1096,9 @@ InternalEfiUnloadImage ( IN EFI_HANDLE ImageHandle ) { - EFI_STATUS Status; - OC_LOADED_IMAGE_PROTOCOL *OcLoadedImage; + EFI_STATUS Status; + OC_LOADED_IMAGE_PROTOCOL *OcLoadedImage; + OC_IMAGE_LOADER_CAPS_PROTOCOL *OcImageLoaderCaps; // // If we loaded the image, do the unloading manually. @@ -1090,6 +1115,28 @@ InternalEfiUnloadImage ( ); } + // + // If we saved image caps during load, free them now. + // + Status = gBS->HandleProtocol ( + ImageHandle, + &mOcImageLoaderCapsProtocolGuid, + (VOID **)&OcImageLoaderCaps + ); + if (!EFI_ERROR (Status)) { + Status = gBS->UninstallMultipleProtocolInterfaces ( + ImageHandle, + &mOcImageLoaderCapsProtocolGuid, + OcImageLoaderCaps, + NULL + ); + if (EFI_ERROR (Status)) { + return Status; + } + + FreePool (OcImageLoaderCaps); + } + return mOriginalEfiUnloadImage (ImageHandle); }