From 4d3cf37ff05d532c5750f6387193c3f30f2c401b Mon Sep 17 00:00:00 2001 From: Phil Noh Date: Tue, 5 Nov 2024 11:24:01 -0600 Subject: [PATCH 01/10] MdePkg/SmmPciExpressLib: Ensure gBS variable for the constructor The PCD token, PcdPciExpressBaseAddress is referred in the constructor. If the token is defined as PcdsDynamic type, the PCD function that gets the token value uses the gBS service to locate PCD protocol internally. In this case, it is possible for the function to be called before initializing gBS variable, then cause a system hang due to gBS variable. Need to ensure the availability of gBS variable. Signed-off-by: Phil Noh --- MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf | 1 + 1 file changed, 1 insertion(+) diff --git a/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf b/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf index 78cab6352fac..f6482ca18e3b 100644 --- a/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf +++ b/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf @@ -32,6 +32,7 @@ PcdLib DebugLib IoLib + UefiBootServicesTableLib [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## CONSUMES From edb312d5d0f00685e75639b8607d54c93d47aca8 Mon Sep 17 00:00:00 2001 From: Phil Noh Date: Thu, 21 Nov 2024 10:05:01 -0600 Subject: [PATCH 02/10] MdePkg/BaseRngLib: Remove global variable for RDRAND state update As a BASE type library, some PEI drivers could link and use it. Tcg2Pei.inf is an example. On edk2-stable202408 version, PEI drivers that link the library include the global variable of mRdRandSupported. The previous commit (c3a8ca7) that refers to the global variable actually is found to influence the link status. Updating the global variable in PEI drivers could affect the following issues. PEI ROM Boot : Global variable is not updated PEI RAM Boot : PEI FV integration/security check is failed To address these issues, remove the global variable usage. Signed-off-by: Phil Noh --- MdePkg/Library/BaseRngLib/Rand/RdRand.c | 37 ++++++++++++------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c b/MdePkg/Library/BaseRngLib/Rand/RdRand.c index 06d2a6f12d2e..85441c392cc2 100644 --- a/MdePkg/Library/BaseRngLib/Rand/RdRand.c +++ b/MdePkg/Library/BaseRngLib/Rand/RdRand.c @@ -2,6 +2,7 @@ Random number generator services that uses RdRand instruction access to provide high-quality random numbers. +Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
Copyright (c) 2023, Arm Limited. All rights reserved.
Copyright (c) 2022, Pedro Falcato. All rights reserved.
Copyright (c) 2021, NUVIA Inc. All rights reserved.
@@ -23,8 +24,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // #define RDRAND_MASK BIT30 -STATIC BOOLEAN mRdRandSupported; - // // Intel SDM says 10 tries is good enough for reliable RDRAND usage. // @@ -124,20 +123,6 @@ BaseRngLibConstructor ( VOID ) { - UINT32 RegEcx; - - // - // Determine RDRAND support by examining bit 30 of the ECX register returned by - // CPUID. A value of 1 indicates that processor support RDRAND instruction. - // - AsmCpuid (1, 0, 0, &RegEcx, 0); - - mRdRandSupported = ((RegEcx & RDRAND_MASK) == RDRAND_MASK); - - if (mRdRandSupported) { - mRdRandSupported = TestRdRand (); - } - return EFI_SUCCESS; } @@ -156,7 +141,6 @@ ArchGetRandomNumber16 ( OUT UINT16 *Rand ) { - ASSERT (mRdRandSupported); return AsmRdRand16 (Rand); } @@ -175,7 +159,6 @@ ArchGetRandomNumber32 ( OUT UINT32 *Rand ) { - ASSERT (mRdRandSupported); return AsmRdRand32 (Rand); } @@ -194,7 +177,6 @@ ArchGetRandomNumber64 ( OUT UINT64 *Rand ) { - ASSERT (mRdRandSupported); return AsmRdRand64 (Rand); } @@ -211,7 +193,22 @@ ArchIsRngSupported ( VOID ) { - return mRdRandSupported; + BOOLEAN RdRandSupported; + UINT32 RegEcx; + + // + // Determine RDRAND support by examining bit 30 of the ECX register returned by + // CPUID. A value of 1 indicates that processor support RDRAND instruction. + // + AsmCpuid (1, 0, 0, &RegEcx, 0); + + RdRandSupported = ((RegEcx & RDRAND_MASK) == RDRAND_MASK); + + if (RdRandSupported) { + RdRandSupported = TestRdRand (); + } + + return RdRandSupported; } /** From f3bc6013d2262b1a40898e5940c5e5dc000724df Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Tue, 5 Nov 2024 13:11:54 -0800 Subject: [PATCH 03/10] MdeModulePkg HobPrintLib: Add Guid to Guids section. gEfiHobMemoryAllocModuleGuid is referenced in the HobPrintLib, but it is not defined in the INF file, causing an unresolved external error when the module is consumed by code. Signed-off-by: Aaron Pop --- MdeModulePkg/Library/HobPrintLib/HobPrintLib.inf | 1 + 1 file changed, 1 insertion(+) diff --git a/MdeModulePkg/Library/HobPrintLib/HobPrintLib.inf b/MdeModulePkg/Library/HobPrintLib/HobPrintLib.inf index a88cabf67def..3b366a9a079f 100644 --- a/MdeModulePkg/Library/HobPrintLib/HobPrintLib.inf +++ b/MdeModulePkg/Library/HobPrintLib/HobPrintLib.inf @@ -32,3 +32,4 @@ gEfiHobMemoryAllocBspStoreGuid gEfiHobMemoryAllocStackGuid gEfiMemoryTypeInformationGuid + gEfiHobMemoryAllocModuleGuid From 0d129450c2c436e047f8ee929db63224e643cc30 Mon Sep 17 00:00:00 2001 From: Oliver Smith-Denny Date: Tue, 8 Oct 2024 10:12:29 -0700 Subject: [PATCH 04/10] NetworkPkg: Restore TPL Before Return This patch fixes a few instances of error cases in NetworkPkg returning after a RaiseTPL call without restoring the TPL first. Signed-off-by: Oliver Smith-Denny --- NetworkPkg/Ip6Dxe/Ip6Impl.c | 3 ++- NetworkPkg/SnpDxe/Get_status.c | 4 ++-- NetworkPkg/SnpDxe/Transmit.c | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/NetworkPkg/Ip6Dxe/Ip6Impl.c b/NetworkPkg/Ip6Dxe/Ip6Impl.c index a4bfd0f9a303..5fbb81cd2ae2 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Impl.c +++ b/NetworkPkg/Ip6Dxe/Ip6Impl.c @@ -61,7 +61,6 @@ EfiIp6GetModeData ( return EFI_INVALID_PARAMETER; } - OldTpl = gBS->RaiseTPL (TPL_CALLBACK); IpInstance = IP6_INSTANCE_FROM_PROTOCOL (This); IpSb = IpInstance->Service; IpIf = IpInstance->Interface; @@ -70,6 +69,8 @@ EfiIp6GetModeData ( return EFI_INVALID_PARAMETER; } + OldTpl = gBS->RaiseTPL (TPL_CALLBACK); + if (Ip6ModeData != NULL) { // // IsStarted is "whether the EfiIp6Configure has been called". diff --git a/NetworkPkg/SnpDxe/Get_status.c b/NetworkPkg/SnpDxe/Get_status.c index 14b678fd364c..2097fbcbaad1 100644 --- a/NetworkPkg/SnpDxe/Get_status.c +++ b/NetworkPkg/SnpDxe/Get_status.c @@ -216,12 +216,12 @@ SnpUndi32GetStatus ( Snp = EFI_SIMPLE_NETWORK_DEV_FROM_THIS (This); - OldTpl = gBS->RaiseTPL (TPL_CALLBACK); - if (Snp == NULL) { return EFI_DEVICE_ERROR; } + OldTpl = gBS->RaiseTPL (TPL_CALLBACK); + switch (Snp->Mode.State) { case EfiSimpleNetworkInitialized: break; diff --git a/NetworkPkg/SnpDxe/Transmit.c b/NetworkPkg/SnpDxe/Transmit.c index e2c7467b8670..7947cde4368c 100644 --- a/NetworkPkg/SnpDxe/Transmit.c +++ b/NetworkPkg/SnpDxe/Transmit.c @@ -287,12 +287,12 @@ SnpUndi32Transmit ( Snp = EFI_SIMPLE_NETWORK_DEV_FROM_THIS (This); - OldTpl = gBS->RaiseTPL (TPL_CALLBACK); - if (Snp == NULL) { return EFI_DEVICE_ERROR; } + OldTpl = gBS->RaiseTPL (TPL_CALLBACK); + switch (Snp->Mode.State) { case EfiSimpleNetworkInitialized: break; From a6f1433e9598bfb7948d9af6b817e521d299a57d Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 14 Nov 2024 12:43:16 +0100 Subject: [PATCH 05/10] DynamicTablesPkg/ArmGicCParser: Parse VGIC interrupt for all CPUs There are two issues in the GIC FDT parsing code: - the GICC Flags 'Enabled' bit is overwritten when parsing the VGIC Maintenance Interrupt, whose trigger type occupies another bit in the same field; - only the first CPU's Flags field is updated. This breaks both SMP boot and KVM support on Linux, given that the boot CPU is disabled in the MADT, and the VGIC maintenance interrupt is set to 0x0 on all others. Fix this, by OR'ing the trigger type into the field, and by iterating over all discovered CPUs. Signed-off-by: Ard Biesheuvel --- .../Arm/Gic/ArmGicCParser.c | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/Gic/ArmGicCParser.c index 395521914671..f74e5d5fefe3 100644 --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/Gic/ArmGicCParser.c +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/Gic/ArmGicCParser.c @@ -167,7 +167,8 @@ CpusNodeParser ( IN CONST VOID *Fdt, IN INT32 CpusNode, IN UINT32 GicVersion, - OUT CM_OBJ_DESCRIPTOR **NewGicCmObjDesc + OUT CM_OBJ_DESCRIPTOR **NewGicCmObjDesc, + OUT UINTN *CpuCount ) { EFI_STATUS Status; @@ -202,6 +203,8 @@ CpusNodeParser ( return EFI_NOT_FOUND; } + *CpuCount = CpuNodeCount; + // Allocate memory for CpuNodeCount CM_ARM_GICC_INFO structures. GicCInfoBufferSize = CpuNodeCount * sizeof (CM_ARM_GICC_INFO); GicCInfoBuffer = AllocateZeroPool (GicCInfoBufferSize); @@ -280,6 +283,7 @@ EFIAPI GicCIntcNodeParser ( IN CONST VOID *Fdt, IN INT32 GicIntcNode, + IN UINTN CpuCount, IN OUT CM_OBJ_DESCRIPTOR *GicCCmObjDesc ) { @@ -289,6 +293,8 @@ GicCIntcNodeParser ( CONST UINT8 *Data; INT32 DataSize; + UINT32 MaintenanceInterrupt; + UINT32 Flags; if (GicCCmObjDesc == NULL) { ASSERT (0); @@ -308,14 +314,17 @@ GicCIntcNodeParser ( // but it is assumed that only one Gic is available. Data = fdt_getprop (Fdt, GicIntcNode, "interrupts", &DataSize); if ((Data != NULL) && (DataSize == (IntCells * sizeof (UINT32)))) { - GicCInfo = (CM_ARM_GICC_INFO *)GicCCmObjDesc->Data; - GicCInfo->VGICMaintenanceInterrupt = - FdtGetInterruptId ((CONST UINT32 *)Data); - GicCInfo->Flags = DT_IRQ_IS_EDGE_TRIGGERED ( - fdt32_to_cpu (((UINT32 *)Data)[IRQ_FLAGS_OFFSET]) - ) ? - EFI_ACPI_6_3_VGIC_MAINTENANCE_INTERRUPT_MODE_FLAGS : - 0; + MaintenanceInterrupt = FdtGetInterruptId ((CONST UINT32 *)Data); + Flags = DT_IRQ_IS_EDGE_TRIGGERED ( + fdt32_to_cpu (((UINT32 *)Data)[IRQ_FLAGS_OFFSET]) + ) ? + EFI_ACPI_6_3_VGIC_MAINTENANCE_INTERRUPT_MODE_FLAGS : + 0; + for (GicCInfo = (CM_ARM_GICC_INFO *)GicCCmObjDesc->Data; CpuCount--; GicCInfo++) { + GicCInfo->VGICMaintenanceInterrupt = MaintenanceInterrupt; + GicCInfo->Flags |= Flags; + } + return Status; } else if (DataSize < 0) { // This property is optional and was not found. Just return. @@ -816,6 +825,7 @@ ArmGicCInfoParser ( UINT32 GicVersion; CM_OBJ_DESCRIPTOR *NewCmObjDesc; VOID *Fdt; + UINTN CpuCount; if (FdtParserHandle == NULL) { ASSERT (0); @@ -846,7 +856,7 @@ ArmGicCInfoParser ( // Parse the "cpus" nodes and its children "cpu" nodes, // and create a CM_OBJ_DESCRIPTOR. - Status = CpusNodeParser (Fdt, FdtBranch, GicVersion, &NewCmObjDesc); + Status = CpusNodeParser (Fdt, FdtBranch, GicVersion, &NewCmObjDesc, &CpuCount); if (EFI_ERROR (Status)) { ASSERT (0); return Status; @@ -878,7 +888,7 @@ ArmGicCInfoParser ( } // Parse the Gic information common to Gic v2 and v3. - Status = GicCIntcNodeParser (Fdt, IntcNode, NewCmObjDesc); + Status = GicCIntcNodeParser (Fdt, IntcNode, CpuCount, NewCmObjDesc); if (EFI_ERROR (Status)) { ASSERT (0); goto exit_handler; From f0d2bc3ab268c8e3c6da4158208df38bc9d3677e Mon Sep 17 00:00:00 2001 From: Tom Lendacky Date: Mon, 18 Nov 2024 12:59:32 -0600 Subject: [PATCH 06/10] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Do not use flash with SEV-SNP SEV-SNP does not support the use of the Qemu flash device as SEV-SNP guests are started using the Qemu -bios option instead of the Qemu -drive if=pflash option. Perform runtime detection of SEV-SNP and exit early from the Qemu flash device initialization, indicating the Qemu flash device is not present. SEV-SNP guests will use the emulated variable support. Signed-off-by: Tom Lendacky --- OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c index a577aea55614..5e393e98ed2d 100644 --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c @@ -259,6 +259,14 @@ QemuFlashInitialize ( VOID ) { + // + // The SNP model does not provide for QEMU flash device support, so exit + // early before attempting to initialize any QEMU flash device support. + // + if (MemEncryptSevSnpIsEnabled ()) { + return EFI_UNSUPPORTED; + } + mFlashBase = (UINT8 *)(UINTN)PcdGet32 (PcdOvmfFdBaseAddress); mFdBlockSize = PcdGet32 (PcdOvmfFirmwareBlockSize); ASSERT (PcdGet32 (PcdOvmfFirmwareFdSize) % mFdBlockSize == 0); From 52fa7e78d282f8434b41aff24b3a5a745611ff87 Mon Sep 17 00:00:00 2001 From: Tom Lendacky Date: Mon, 18 Nov 2024 12:59:32 -0600 Subject: [PATCH 07/10] OvmfPkg/PlatformPei: Move NV vars init to after SEV-SNP memory acceptance When OVMF is built with the SECURE_BOOT_ENABLE set to true, reserving and initializing the emulated variable store happens before memory has been accepted under SEV-SNP. This results in a #VC exception for accessing memory that hasn't been validated (error code 0x404). The #VC handler treats this error code as a fatal error, causing the OVMF boot to fail. Move the call to ReserveEmuVariableNvStore() to after memory has been accepted by AmdSevInitialize(). Signed-off-by: Tom Lendacky --- OvmfPkg/PlatformPei/Platform.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index dc81ce9e2bff..7b4ea1b82730 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -353,10 +353,6 @@ InitializePlatform ( InitializeRamRegions (PlatformInfoHob); if (PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) { - if (!PlatformInfoHob->SmmSmramRequire) { - ReserveEmuVariableNvStore (); - } - PeiFvInitialization (PlatformInfoHob); MemTypeInfoInitialization (PlatformInfoHob); MemMapInitialization (PlatformInfoHob); @@ -378,5 +374,15 @@ InitializePlatform ( RelocateSmBase (); } + // + // Performed after CoCo (SEV/TDX) initialization to allow the memory + // used to be validated before being used. + // + if (PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) { + if (!PlatformInfoHob->SmmSmramRequire) { + ReserveEmuVariableNvStore (); + } + } + return EFI_SUCCESS; } From d502cc7702e4d537c2bcbe5256e26cba6d4ca8c6 Mon Sep 17 00:00:00 2001 From: Tom Lendacky Date: Mon, 18 Nov 2024 12:59:32 -0600 Subject: [PATCH 08/10] OvmfPkg/PlatformInitLib: Retry NV vars FV check as shared When OVMF is built with SECURE_BOOT_ENABLE, the variable store will be populated and validated in PlatformValidateNvVarStore(). When an SEV or an SEV-ES guest is running, this may be encrypted or unencrypted depending on how the guest was started. If the guest was started with the combined code and variable contents (OVMF.fd), then the variable store will be encrypted. If the guest was started with the separate code and variables contents (OVMF_CODE.fd and OVMF_VARS.fd), then the variable store will be unencrypted. When PlatformValidateNvVarStore() is first invoked, the variable store area is initially mapped encrypted, which may or may not pass the variable validation step depending how the guest was launched. To accomodate this, retry the validation step on failure after remapping the variable store area as unencrypted. Signed-off-by: Tom Lendacky --- OvmfPkg/Library/PlatformInitLib/Platform.c | 32 +++++++++++++++++-- .../PlatformInitLib/PlatformInitLib.inf | 1 + 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c b/OvmfPkg/Library/PlatformInitLib/Platform.c index 10fc17355fac..715533b1f20f 100644 --- a/OvmfPkg/Library/PlatformInitLib/Platform.c +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c @@ -34,6 +34,7 @@ #include #include #include +#include #include @@ -774,6 +775,8 @@ PlatformValidateNvVarStore ( EFI_FIRMWARE_VOLUME_HEADER *NvVarStoreFvHeader; VARIABLE_STORE_HEADER *NvVarStoreHeader; AUTHENTICATED_VARIABLE_HEADER *VariableHeader; + BOOLEAN Retry; + EFI_STATUS Status; static EFI_GUID FvHdrGUID = EFI_SYSTEM_NV_DATA_FV_GUID; static EFI_GUID VarStoreHdrGUID = EFI_AUTHENTICATED_VARIABLE_GUID; @@ -792,6 +795,15 @@ PlatformValidateNvVarStore ( // NvVarStoreFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)NvVarStoreBase; + // + // SEV and SEV-ES can use separate flash devices for OVMF code and + // OVMF variables. In this case, the OVMF variables will need to be + // mapped unencrypted. If the initial validation fails, remap the + // NV variable store as unencrypted and retry the validation. + // + Retry = MemEncryptSevIsEnabled (); + +RETRY: if ((!IsZeroBuffer (NvVarStoreFvHeader->ZeroVector, 16)) || (!CompareGuid (&FvHdrGUID, &NvVarStoreFvHeader->FileSystemGuid)) || (NvVarStoreFvHeader->Signature != EFI_FVH_SIGNATURE) || @@ -801,8 +813,24 @@ PlatformValidateNvVarStore ( (NvVarStoreFvHeader->FvLength != NvVarStoreSize) ) { - DEBUG ((DEBUG_ERROR, "NvVarStore FV headers were invalid.\n")); - return FALSE; + if (!Retry) { + DEBUG ((DEBUG_ERROR, "NvVarStore FV headers were invalid.\n")); + return FALSE; + } + + DEBUG ((DEBUG_INFO, "Remapping NvVarStore as shared\n")); + Status = MemEncryptSevClearMmioPageEncMask ( + 0, + (UINTN)NvVarStoreBase, + EFI_SIZE_TO_PAGES (NvVarStoreSize) + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Failed to map NvVarStore as shared\n")); + return FALSE; + } + + Retry = FALSE; + goto RETRY; } // diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf index 3e63ef44239a..fb179e679151 100644 --- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf +++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf @@ -48,6 +48,7 @@ HobLib QemuFwCfgLib QemuFwCfgSimpleParserLib + MemEncryptSevLib MemoryAllocationLib MtrrLib PcdLib From 6142f0a8a53557ba50300c762a15bf3c18382162 Mon Sep 17 00:00:00 2001 From: Tom Lendacky Date: Mon, 18 Nov 2024 12:59:32 -0600 Subject: [PATCH 09/10] OvmfPkg/EmuVariableFvbRuntimeDxe: Issue NV vars initializitation message Add a debug message that indicates when the NV variables are being initialized through the template structure. Signed-off-by: Tom Lendacky --- OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c index c07e38966e36..cc476c7df210 100644 --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c @@ -692,6 +692,8 @@ InitializeFvAndVariableStoreHeaders ( // Fv = (EFI_FIRMWARE_VOLUME_HEADER *)Ptr; Fv->Checksum = CalculateCheckSum16 (Ptr, Fv->HeaderLength); + + DEBUG ((DEBUG_INFO, "EMU Variable FVB: Initialized FV using template structure\n")); } /** From 800205678fc9f4d290890de2db1e1de87759da66 Mon Sep 17 00:00:00 2001 From: Tormod Volden Date: Thu, 25 Jul 2024 10:31:14 +0200 Subject: [PATCH 10/10] ShellPkg: Fix check on OldArgv in UpdateArgcArgv() The UpdateArgcArgv() function documentation says "If OldArgv or OldArgc is NULL then that value is not returned." However, only OldArgc was checked for NULL, probably because of copy-pasto. In case OldArgc was non-NULL, but OldArgv was null, it could cause a segmentation fault. Check OldArgv is not NULL before dereferencing the value. Signed-off-by: Tormod Volden --- ShellPkg/Application/Shell/ShellParametersProtocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c b/ShellPkg/Application/Shell/ShellParametersProtocol.c index 8464cbf61612..1784898169ba 100644 --- a/ShellPkg/Application/Shell/ShellParametersProtocol.c +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c @@ -1470,7 +1470,7 @@ UpdateArgcArgv ( *OldArgc = ShellParameters->Argc; } - if (OldArgc != NULL) { + if (OldArgv != NULL) { *OldArgv = ShellParameters->Argv; }