From 814b3d81b80a3c1aeac43cd21224dcaafdff8479 Mon Sep 17 00:00:00 2001 From: vit9696 Date: Sat, 5 Dec 2020 04:41:45 +0300 Subject: [PATCH] OpenRuntime: Store Boot options with different prefix --- Changelog.md | 1 + Platform/OpenRuntime/OpenRuntimePrivate.h | 2 +- Platform/OpenRuntime/UefiRuntimeServices.c | 116 +++++++++++++++++++-- 3 files changed, 108 insertions(+), 11 deletions(-) diff --git a/Changelog.md b/Changelog.md index 93a7564f..d3a62ab1 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ OpenCore Changelog - Fixed `DisableSingleUser` quirk when Apple Secure Boot is enabled - Added `BootstrapShort` to workaround buggy Insyde firmwares - Changed `Bootstrap(Short)` to choose dynamic entry (requires NVRAM reset) +- Avoided `Boot` prefix in `RequestBootVarRouting` to workaround AMI issues #### v0.6.3 - Added support for xml comments in plist files diff --git a/Platform/OpenRuntime/OpenRuntimePrivate.h b/Platform/OpenRuntime/OpenRuntimePrivate.h index 0b0a8842..05fdd39c 100644 --- a/Platform/OpenRuntime/OpenRuntimePrivate.h +++ b/Platform/OpenRuntime/OpenRuntimePrivate.h @@ -33,7 +33,7 @@ extern OC_FWRT_CONFIG gOverrideConfig; **/ extern OC_FWRT_CONFIG *gCurrentConfig; -#define OC_GL_BOOT_OPTION_START 0xF000 +#define OC_VARIABLE_NAME_SIZE 256 VOID RedirectRuntimeServices ( diff --git a/Platform/OpenRuntime/UefiRuntimeServices.c b/Platform/OpenRuntime/UefiRuntimeServices.c index a9f928da..9c3cc6e9 100644 --- a/Platform/OpenRuntime/UefiRuntimeServices.c +++ b/Platform/OpenRuntime/UefiRuntimeServices.c @@ -56,6 +56,9 @@ OC_FWRT_CONFIG *gCurrentConfig; STATIC EFI_EVENT mTranslateEvent; STATIC EFI_GET_VARIABLE mCustomGetVariable; STATIC BOOLEAN mKernelStarted; +#ifdef OC_DEBUG_VAR_SERVICE ///< For file logging disable TPL checking in OcLogAddEntry. +STATIC BOOLEAN mInsideVarService; +#endif STATIC VOID @@ -108,9 +111,12 @@ STATIC BOOLEAN IsEfiBootVar ( IN CHAR16 *VariableName, - IN EFI_GUID *VendorGuid + IN EFI_GUID *VendorGuid, + OUT CHAR16 *NewVariableName OPTIONAL ) { + UINTN Size; + if (!CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) { return FALSE; } @@ -119,24 +125,85 @@ IsEfiBootVar ( return FALSE; } + // + // Return OC option path if requested. + // + // Many modern AMI BIOSes (e.g. ASUS ROG STRIX Z370-F GAMING) + // have bugs in AMI Post Manager protocol implementation in AMI TSE. + // + // The handshake function, responsible for obtaining firmware + // boot option list, calls gRT->GetNextVariableName and then matches + // every Boot#### variable as a potential boot option regardless + // of the GUID namespace it is in. + // + // The correct way to do it is to only check Boot#### variables in + // gEfiGlobalVariableGuid VendorGuid. However, since they made a mistake, + // most ASUS firmwares (and perhaps other vendors) get corrupted boot option + // list (with e.g. duplicated options) if Boot#### variables are present + // outside of gEfiGlobalVariableGuid. + // + // This is the case with OpenCore as we store macOS-specific boot options + // in a separate GUID (gOcVendorVariableGuid). To workaround the issue + // we store Boot options with a different prefix - OCBt. This way + // Boot0001 becomes OCBt0001 and Bootorder becomes OCBtOrder. + // + // One of the ways to reproduce the issue is to enable Fast Boot + // and let an ExitBootServices handler try to update BootOrder. + // It can be easily noticed by BootFlow GetVariable with an uninitialised + // size argument. + // + if (NewVariableName != NULL) { + Size = StrSize (VariableName); + if (Size > OC_VARIABLE_NAME_SIZE * sizeof (CHAR16)) { + return FALSE; + } + + CopyMem (&NewVariableName[0], L"OCBt", L_STR_SIZE_NT (L"OCBt")); + CopyMem ( + &NewVariableName[L_STR_LEN (L"OCBt")], + &VariableName[L_STR_LEN (L"OCBt")], + Size - L_STR_SIZE_NT (L"OCBt") + ); + } + return TRUE; } STATIC BOOLEAN IsOcBootVar ( - IN CHAR16 *VariableName, - IN EFI_GUID *VendorGuid + IN CHAR16 *VariableName, + IN EFI_GUID *VendorGuid, + OUT CHAR16 *NewVariableName OPTIONAL ) { + UINTN Size; + if (!CompareGuid (VendorGuid, &gOcVendorVariableGuid)) { return FALSE; } - if (StrnCmp (L"Boot", VariableName, L_STR_LEN (L"Boot")) != 0) { + if (StrnCmp (L"OCBt", VariableName, L_STR_LEN (L"OCBt")) != 0) { return FALSE; } + // + // Return boot option if requested. + // + if (NewVariableName != NULL) { + Size = StrSize (VariableName); + if (Size > OC_VARIABLE_NAME_SIZE * sizeof (CHAR16)) { + return FALSE; + } + + CopyMem (NewVariableName, L"Boot", L_STR_SIZE_NT (L"Boot")); + CopyMem ( + &NewVariableName[L_STR_LEN (L"Boot")], + &VariableName[L_STR_LEN (L"Boot")], + Size - L_STR_SIZE_NT (L"Boot") + ); + } + return TRUE; } @@ -261,6 +328,7 @@ WrapGetVariable ( ) { EFI_STATUS Status; + CHAR16 TempName[OC_VARIABLE_NAME_SIZE]; BOOLEAN Ints; BOOLEAN Wp; @@ -274,6 +342,14 @@ WrapGetVariable ( return EFI_INVALID_PARAMETER; } +#ifdef OC_DEBUG_VAR_SERVICE + if (!mInsideVarService && StrCmp (VariableName, L"EfiTime") != 0) { + mInsideVarService = TRUE; + DEBUG ((DEBUG_INFO, "GETVAR %g:%s (%u)\n", VendorGuid, VariableName, (UINT32) *DataSize)); + mInsideVarService = FALSE; + } +#endif + // // Abort access to write-only variables. // @@ -286,7 +362,8 @@ WrapGetVariable ( // // Redirect Boot-prefixed variables to our own GUID. // - if (gCurrentConfig->BootVariableRedirect && IsEfiBootVar (VariableName, VendorGuid)) { + if (gCurrentConfig->BootVariableRedirect && IsEfiBootVar (VariableName, VendorGuid, TempName)) { + VariableName = TempName; VendorGuid = &gOcVendorVariableGuid; } @@ -317,12 +394,20 @@ WrapGetNextVariableName ( EFI_STATUS Status; UINTN Index; UINTN Size; - CHAR16 TempName[256]; + CHAR16 TempName[OC_VARIABLE_NAME_SIZE]; EFI_GUID TempGuid; BOOLEAN StartBootVar; BOOLEAN Ints; BOOLEAN Wp; +#ifdef OC_DEBUG_VAR_SERVICE + if (!mInsideVarService) { + mInsideVarService = TRUE; + DEBUG ((DEBUG_INFO, "NEXVAR %g:%s (%u/%u)\n", VendorGuid, VariableName, (UINT32) *VariableNameSize)); + mInsideVarService = FALSE; + } +#endif + // // Perform initial checks as per spec. Last check is part of: // Null-terminator is not found in the first VariableNameSize @@ -374,7 +459,7 @@ WrapGetNextVariableName ( // then go through the whole variable list and return // variables except EfiBoot. // - if (!IsEfiBootVar (TempName, &TempGuid)) { + if (!IsEfiBootVar (TempName, &TempGuid, TempName)) { while (TRUE) { // // Request for variables. @@ -383,7 +468,7 @@ WrapGetNextVariableName ( Status = mStoredGetNextVariableName (&Size, TempName, &TempGuid); if (!EFI_ERROR (Status)) { - if (!IsEfiBootVar (TempName, &TempGuid)) { + if (!IsEfiBootVar (TempName, &TempGuid, NULL)) { Size = StrSize (TempName); ///< Not guaranteed to be updated with EFI_SUCCESS. if (*VariableNameSize >= Size) { @@ -456,6 +541,7 @@ WrapGetNextVariableName ( } else { // // Switch to real GUID as stored in variable storage. + // TempName is already updated with the new name. // CopyGuid (&TempGuid, &gOcVendorVariableGuid); } @@ -468,7 +554,7 @@ WrapGetNextVariableName ( Status = mStoredGetNextVariableName (&Size, TempName, &TempGuid); if (!EFI_ERROR (Status)) { - if (IsOcBootVar (TempName, &TempGuid)) { + if (IsOcBootVar (TempName, &TempGuid, TempName)) { Size = StrSize (TempName); ///< Not guaranteed to be updated with EFI_SUCCESS. if (*VariableNameSize >= Size) { @@ -522,9 +608,18 @@ WrapSetVariable ( ) { EFI_STATUS Status; + CHAR16 TempName[OC_VARIABLE_NAME_SIZE]; BOOLEAN Ints; BOOLEAN Wp; +#ifdef OC_DEBUG_VAR_SERVICE + if (!mInsideVarService) { + mInsideVarService = TRUE; + DEBUG ((DEBUG_INFO, "SETVAR %g:%s (%u/%u)\n", VendorGuid, VariableName, (UINT32) DataSize, Attributes)); + mInsideVarService = FALSE; + } +#endif + // // Abort access when running with read-only NVRAM. // @@ -569,7 +664,8 @@ WrapSetVariable ( // Redirect Boot-prefixed variables to our own GUID. // if (gCurrentConfig->BootVariableRedirect - && IsEfiBootVar (VariableName, VendorGuid)) { + && IsEfiBootVar (VariableName, VendorGuid, TempName)) { + VariableName = TempName; VendorGuid = &gOcVendorVariableGuid; }