uefi-firmware: include fixes mentioned on forums

tiiuae · Jan 4, 2025 · 6277884 · 6277884
1 parent 6bac358
commit 6277884
Show file tree

Hide file tree

Showing 3 changed files with 226 additions and 0 deletions.
diff --git a/pkgs/uefi-firmware/default.nix b/pkgs/uefi-firmware/default.nix
@@ -89,6 +89,15 @@ let
       # using one from the kernel-dtb partition.
       # See: https://github.com/anduril/jetpack-nixos/pull/18
       ./edk2-uefi-dtb.patch
+
+      # Include patches to fix "Assertion 3" mentioned here:
+      # https://forums.developer.nvidia.com/t/assertion-issue-in-uefi-during-boot/315628
+      # From this PR: https://github.com/NVIDIA/edk2-nvidia/pull/110
+      # It is unclear if it does (as of 2025-01-03), but hopefully this also
+      # resolves the critical issue mentioned here:
+      # https://forums.developer.nvidia.com/t/possible-uefi-memory-leak-and-partition-full/308540
+      ./fix-bug-in-block-erase-logic.patch
+      ./fix-variant-read-records-per-erase-block-and-fix-leak.patch
     ];
     postPatch = lib.optionalString errorLevelInfo ''
       sed -i 's#PcdDebugPrintErrorLevel|.*#PcdDebugPrintErrorLevel|0x8000004F#' Platform/NVIDIA/NVIDIA.common.dsc.inc

diff --git a/pkgs/uefi-firmware/fix-bug-in-block-erase-logic.patch b/pkgs/uefi-firmware/fix-bug-in-block-erase-logic.patch
@@ -0,0 +1,36 @@
+From 9d0ad7f2894cdfbec3a361b7a871de95e94ed48f Mon Sep 17 00:00:00 2001
+From: Girish Mahadevan <[email protected]>
+Date: Tue, 10 Dec 2024 09:43:00 -0700
+Subject: [PATCH] fix: bug in block erase logic
+
+When figuring out the LBA for the current block we accidentally
+used the partition offset and not the block size.
+
+Change-Id: I9d0cb3f38694663b832a3d34bd8dbcf9d73096f2
+Signed-off-by: Girish Mahadevan <[email protected]>
+---
+ Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c b/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c
+index 11cd4d4f94..59b09e9956 100644
+--- a/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c
++++ b/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c
+@@ -294,7 +294,7 @@ GetWriteOffset (
+         *Offset     = BlockOffset;
+         break;
+       } else if (ReadBuf[0] == VAR_INT_VALID) {
+-        ValidRecord = CurOffset;
++        ValidRecord = BlockOffset;
+       }
+
+       BlockOffset += This->MeasurementSize;
+@@ -313,7 +313,7 @@ GetWriteOffset (
+     if ((ValidRecord == 0) || (NumPartitionBlocks == 1)) {
+       *Offset = This->PartitionByteOffset;
+     } else {
+-      CurBlock = (ValidRecord / This->PartitionByteOffset);
++      CurBlock = (ValidRecord / This->BlockSize);
+       if (CurBlock == EndBlock) {
+         *Offset = This->PartitionByteOffset;
+       } else {
diff --git a/pkgs/uefi-firmware/fix-variant-read-records-per-erase-block-and-fix-leak.patch b/pkgs/uefi-firmware/fix-variant-read-records-per-erase-block-and-fix-leak.patch
@@ -0,0 +1,181 @@
+From 92cd5f962649bb544b781c8659388c78439e98ea Mon Sep 17 00:00:00 2001
+From: Girish Mahadevan <[email protected]>
+Date: Wed, 6 Nov 2024 13:41:22 -0800
+Subject: [PATCH] fix: varint: read records per erase block and fix leak
+
+When the write pointer moves to write a new record, it
+takes care not to straddle erase blocks; when reading back
+account for this.
+Fix a memory leak in the NvVarIntLibrary.
+
+Bug 4788402
+Change-Id: I5ff718c066860ca9f20a7bfa4a006b57405f1911
+Signed-off-by: Girish Mahadevan <[email protected]>
+Reviewed-on: https://git-master.nvidia.com/r/c/tegra/bootloader/uefi/edk2-nvidia/+/3244202
+Reviewed-by: svcacv <[email protected]>
+Reviewed-by: Ashish Singhal <[email protected]>
+GVS: buildbot_gerritrpt <[email protected]>
+Reviewed-by: svc-sw-mobile-l4t <[email protected]>
+---
+ .../Drivers/FvbNorFlashDxe/VarIntCheck.c      | 74 +++++++++++--------
+ .../Library/NvVarIntLibrary/NvVarIntLibrary.c | 22 +++---
+ 2 files changed, 51 insertions(+), 45 deletions(-)
+
+diff --git a/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c b/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c
+index 53d5fc2aa9..11cd4d4f94 100644
+--- a/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c
++++ b/Silicon/NVIDIA/Drivers/FvbNorFlashDxe/VarIntCheck.c
+@@ -483,6 +483,8 @@ GetLastValidMeasurements (
+   UINT64                     StartOffset;
+   UINT64                     EndOffset;
+   UINT64                     CurOffset;
++  UINT64                     BlockOffset;
++  UINT64                     BlockEnd;
+   UINT64                     NumValidRecords;
+   UINT8                      *ReadBuf;
+
+@@ -499,50 +501,58 @@ GetLastValidMeasurements (
+   EndOffset   = StartOffset + VarInt->PartitionSize;
+
+   CurOffset       = StartOffset;
++  BlockOffset     = CurOffset;
++  BlockEnd        = BlockOffset + VarInt->BlockSize;
+   NumValidRecords = 0;
+   *NumRecords     = 0;
+
+   while (CurOffset < EndOffset) {
+-    Status = NorFlash->Read (
+-                         NorFlash,
+-                         CurOffset,
+-                         VarInt->MeasurementSize,
+-                         ReadBuf
+-                         );
+-    if (EFI_ERROR (Status)) {
+-      DEBUG ((
+-        DEBUG_ERROR,
+-        "%a: NorFlash Read Failed at %lu offset %r\n",
+-        __FUNCTION__,
+-        CurOffset,
+-        Status
+-        ));
+-      goto ExitGetLastValidMeasuremets;
+-    }
+-
+-    if ((ReadBuf[0] == VAR_INT_VALID) ||
+-        (ReadBuf[0] == VAR_INT_PENDING))
+-    {
+-      NumValidRecords++;
+-      if (NumValidRecords > MAX_VALID_RECORDS) {
++    while (BlockOffset < BlockEnd) {
++      Status = NorFlash->Read (
++                           NorFlash,
++                           BlockOffset,
++                           VarInt->MeasurementSize,
++                           ReadBuf
++                           );
++      if (EFI_ERROR (Status)) {
+         DEBUG ((
+           DEBUG_ERROR,
+-          "%a: More than %d Valid measurements found %x\n",
++          "%a: NorFlash Read Failed at %lu offset %r\n",
+           __FUNCTION__,
+-          MAX_VALID_RECORDS,
+-          ReadBuf[0]
++          BlockOffset,
++          Status
+           ));
+-        Status = EFI_DEVICE_ERROR;
+         goto ExitGetLastValidMeasuremets;
+-      } else {
+-        DEBUG ((DEBUG_INFO, "Found Record at %lu Header %x\n", CurOffset, ReadBuf[0]));
+-        CopyMem (Records[(NumValidRecords - 1)]->Measurement, ReadBuf, VarInt->MeasurementSize);
+-        *NumRecords                               += 1;
+-        Records[(NumValidRecords - 1)]->ByteOffset = CurOffset;
+       }
++
++      if ((ReadBuf[0] == VAR_INT_VALID) ||
++          (ReadBuf[0] == VAR_INT_PENDING))
++      {
++        NumValidRecords++;
++        if (NumValidRecords > MAX_VALID_RECORDS) {
++          DEBUG ((
++            DEBUG_ERROR,
++            "%a: More than %d Valid measurements found %x\n",
++            __FUNCTION__,
++            MAX_VALID_RECORDS,
++            ReadBuf[0]
++            ));
++          Status = EFI_DEVICE_ERROR;
++          goto ExitGetLastValidMeasuremets;
++        } else {
++          DEBUG ((DEBUG_INFO, "Found Record at %lu Header %x\n", BlockOffset, ReadBuf[0]));
++          CopyMem (Records[(NumValidRecords - 1)]->Measurement, ReadBuf, VarInt->MeasurementSize);
++          *NumRecords                               += 1;
++          Records[(NumValidRecords - 1)]->ByteOffset = BlockOffset;
++        }
++      }
++
++      BlockOffset += VarInt->MeasurementSize;
+     }
+
+-    CurOffset += VarInt->MeasurementSize;
++    CurOffset  += VarInt->BlockSize;
++    BlockEnd   += VarInt->BlockSize;
++    BlockOffset = CurOffset;
+   }
+
+ ExitGetLastValidMeasuremets:
+diff --git a/Silicon/NVIDIA/Library/NvVarIntLibrary/NvVarIntLibrary.c b/Silicon/NVIDIA/Library/NvVarIntLibrary/NvVarIntLibrary.c
+index 236e68532d..16c239fb55 100644
+--- a/Silicon/NVIDIA/Library/NvVarIntLibrary/NvVarIntLibrary.c
++++ b/Silicon/NVIDIA/Library/NvVarIntLibrary/NvVarIntLibrary.c
+@@ -177,9 +177,16 @@ MeasureBootVars (
+
+     DEBUG ((DEBUG_INFO, "Adding %s Size %u %p\n", BootOptionName, BootOptionSize, BootOptions[Index]));
+     HashApiUpdate (HashContext, BootOptions[Index], BootOptionSize);
++    FreePool (BootOptions[Index]);
+   }
+
+ ExitMeasureBootVars:
++
++  if (BootOptions != NULL) {
++    FreePool (BootOptions);
++    BootOptions = NULL;
++  }
++
+   if ((UpdatingBootOrder == TRUE)) {
+     BootOrder = NULL;
+     BootCount = 0;
+@@ -598,7 +605,6 @@ ComputeVarMeasurement (
+   )
+ {
+   EFI_STATUS  Status;
+-  UINTN       Index;
+
+   if (HashContext == NULL) {
+     HashContext = AllocateRuntimeZeroPool (HashApiGetContextSize ());
+@@ -625,22 +631,12 @@ ComputeVarMeasurement (
+     Status = EFI_DEVICE_ERROR;
+   }
+
+-  if ((BootCount != 0) && (BootOptions != NULL)) {
+-    for (Index = 0; Index < BootCount; Index++) {
+-      if (BootOptions[Index] != NULL) {
+-        FreePool (BootOptions[Index]);
+-      }
+-    }
+-
+-    FreePool (BootOptions);
+-    BootCount   = 0;
+-    BootOptions = NULL;
+-  }
+-
+   if (BootOrder != NULL) {
+     FreePool (BootOrder);
+   }
+
++  BootCount = 0;
++
+   Status = EFI_SUCCESS;
+
+ ExitComputeVarMeasurement: