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 <[email protected]>
bexcran · Oct 4, 2024 · 54469a6 · 54469a6
1 parent b21cf3b
commit 54469a6
Showing 1 changed file with 9 additions and 5 deletions.
diff --git 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 {