From 54469a6918320591a3ec318eada60aed3c75334c Mon Sep 17 00:00:00 2001 From: Tormod Volden Date: Thu, 25 Jul 2024 14:30:56 +0200 Subject: [PATCH] ShellPkg: Fix Optional Data rewriting with bcfg When modifying the Optional Data of a boot option with bcfg boot -opt the result was corrupted data, for instance a concatenation of old data, heap contents, and new data. This was due to a erronous calculation of the original optional data length. In addition to fixing the calculation, add explaining comments and introduce a helper variable, to not abuse other variables and confuse readers (including the author). Signed-off-by: Tormod Volden --- .../UefiShellBcfgCommandLib.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c index 0b94d9f5a1..3ad96647ec 100644 --- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c +++ b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c @@ -99,6 +99,7 @@ UpdateOptionalData ( UINT8 *OriginalData; UINTN NewSize; UINT8 *NewData; + UINTN TmpSize; UINTN OriginalOptionDataSize; UnicodeSPrint (VariableName, sizeof (VariableName), L"%s%04x", Target == BcfgTargetBootOrder ? L"Boot" : L"Driver", Index); @@ -135,11 +136,14 @@ UpdateOptionalData ( // Allocate new struct and discard old optional data. // ASSERT (OriginalData != NULL); - OriginalOptionDataSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (((CHAR16 *)(OriginalData + sizeof (UINT32) + sizeof (UINT16)))); - OriginalOptionDataSize += (*(UINT16 *)(OriginalData + sizeof (UINT32))); - OriginalOptionDataSize -= OriginalSize; - NewSize = OriginalSize - OriginalOptionDataSize + DataSize; - NewData = AllocatePool (NewSize); + // Length of Attributes, FilePathListLength, Description fields + TmpSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (((CHAR16 *)(OriginalData + sizeof (UINT32) + sizeof (UINT16)))); + // Length of FilePathList field + TmpSize += (*(UINT16 *)(OriginalData + sizeof (UINT32))); + // What remains is the original OptionalData field + OriginalOptionDataSize = OriginalSize - TmpSize; + NewSize = OriginalSize - OriginalOptionDataSize + DataSize; + NewData = AllocatePool (NewSize); if (NewData == NULL) { Status = EFI_OUT_OF_RESOURCES; } else {