1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [edk2-platform][patch v2] FitGen: Fix the issue to run in X64 linux machine

Liming Gao posted 1 patch 4 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
Silicon/Intel/Tools/FitGen/FitGen.c | 31 ++++++++++++++++++++-----------
Silicon/Intel/Tools/FitGen/FitGen.h |  2 +-
2 files changed, 21 insertions(+), 12 deletions(-)
[edk2-devel] [edk2-platform][patch v2] FitGen: Fix the issue to run in X64 linux machine
Posted by Liming Gao 4 years, 2 months ago 
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2466
Memory allocation (malloc) may return the buffer address be above 4G.
Current logic always converts the memory address to UINT32. It will
cause memory read and free corrupt. This patch uses pointer to store
the allocated memory address.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
In V2, add the detail commit message.

 Silicon/Intel/Tools/FitGen/FitGen.c | 31 ++++++++++++++++++++-----------
 Silicon/Intel/Tools/FitGen/FitGen.h |  2 +-
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/Silicon/Intel/Tools/FitGen/FitGen.c b/Silicon/Intel/Tools/FitGen/FitGen.c
index 833610f2a0..c9dfa23a2c 100644
--- a/Silicon/Intel/Tools/FitGen/FitGen.c
+++ b/Silicon/Intel/Tools/FitGen/FitGen.c
@@ -2,7 +2,7 @@
 This utility is part of build process for IA32/X64 FD.
 It generates FIT table.
 
-Copyright (c) 2010-2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010-2020, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -226,9 +226,17 @@ typedef struct {
 #define FIT_TABLE_TYPE_BIOS_DATA_AREA         13
 #define FIT_TABLE_TYPE_CSE_SECURE_BOOT        16
 
+//
+// With OptionalModule Address isn't known until free space has been
+// identified and the optional module has been copied into the FLASH
+// image buffer (or initialized to be populated later by another program).
+// This is very dangerous code as it can truncate 64b pointers to
+// allocated memory buffers.  The full pointer is in Buffer for that case.
+//
 typedef struct {
   UINT32  Type;
   UINT32  Address;
+  UINT8   *Buffer; // Used by OptionalModule only
   UINT32  Size;
   UINT32  Version; // Used by OptionalModule and PortModule only
 } FIT_TABLE_CONTEXT_ENTRY;
@@ -575,9 +583,9 @@ Returns:
   UINT64                      FvLength;
   EFI_GUID                    *TempGuid;
   UINT8                       *FixPoint;
-  UINT32                      Offset;
-  UINT32                      FileLength;
-  UINT32                      FileOccupiedSize;
+  UINTN                       Offset;
+  UINTN                       FileLength;
+  UINTN                       FileOccupiedSize;
 
   //
   // Find the FFS file
@@ -595,7 +603,7 @@ Returns:
     InitializeFvLib (FvHeader, (UINT32)FvLength);
 
     FileHeader       = (EFI_FFS_FILE_HEADER *)((UINTN)FvHeader + FvHeader->HeaderLength);
-    Offset           = (UINT32) (UINTN) FileHeader - (UINT32) (UINTN) FvHeader;
+    Offset           = (UINTN) FileHeader - (UINTN) FvHeader;
 
     while (Offset < FvLength) {
       TempGuid = (EFI_GUID *)&(FileHeader->Name);
@@ -625,7 +633,7 @@ Returns:
         return FixPoint;
       }
       FileHeader = (EFI_FFS_FILE_HEADER *)((UINTN)FileHeader + FileOccupiedSize);
-      Offset = (UINT32) (UINTN) FileHeader - (UINT32) (UINTN) FvHeader;
+      Offset = (UINTN) FileHeader - (UINTN) FvHeader;
     }
 
     //
@@ -1082,7 +1090,7 @@ Returns:
                 return 0;
               }
               gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Type = FIT_TABLE_TYPE_MICROCODE;
-              gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address = MicrocodeBase + ((UINT32) (UINTN) MicrocodeBuffer - (UINT32) (UINTN) MicrocodeFileBuffer);
+              gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address = MicrocodeBase + (UINT32)((UINTN) MicrocodeBuffer - (UINTN) MicrocodeFileBuffer);
               gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Size = MicrocodeSize;
               gFitTableContext.MicrocodeNumber++;
               gFitTableContext.FitEntryNumber++;
@@ -1110,7 +1118,7 @@ Returns:
               ///
               while (MicrocodeBuffer + SlotSize <= MicrocodeFileBuffer + MicrocodeFileSize) {
                 gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Type = FIT_TABLE_TYPE_MICROCODE;
-                gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address = MicrocodeBase + ((UINT32) (UINTN) MicrocodeBuffer - (UINT32) (UINTN) MicrocodeFileBuffer);
+                gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address = MicrocodeBase + (UINT32)((UINTN) MicrocodeBuffer - (UINTN) MicrocodeFileBuffer);
                 gFitTableContext.MicrocodeNumber++;
                 gFitTableContext.FitEntryNumber++;
 
@@ -1428,7 +1436,7 @@ Returns:
         return 0;
       }
       gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Type = FIT_TABLE_TYPE_MICROCODE;
-      gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address = MicrocodeBase + ((UINT32) (UINTN) MicrocodeBuffer - (UINT32) (UINTN) MicrocodeFileBuffer);
+      gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address = MicrocodeBase + (UINT32)((UINTN) MicrocodeBuffer - (UINTN) MicrocodeFileBuffer);
       gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Size = MicrocodeSize;
       gFitTableContext.MicrocodeNumber++;
       gFitTableContext.FitEntryNumber++;
@@ -1557,6 +1565,7 @@ Returns:
     }
     gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber].Type = Type;
     gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber].Address = (UINT32) (UINTN) FileBuffer;
+    gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber].Buffer = FileBuffer;
     gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber].Size = FileSize;
 
     //
@@ -1846,8 +1855,8 @@ Returns:
           }
         }
       }
-      memcpy (OptionalModuleAddress, (VOID *) (UINTN) gFitTableContext.OptionalModule[Index].Address, gFitTableContext.OptionalModule[Index].Size);
-      free ((VOID *) (UINTN) gFitTableContext.OptionalModule[Index].Address);
+      memcpy (OptionalModuleAddress, gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber].Buffer, gFitTableContext.OptionalModule[Index].Size);
+      free (gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber].Buffer);
       gFitTableContext.OptionalModule[Index].Address = MEMORY_TO_FLASH (OptionalModuleAddress, FvBuffer, FvSize);
     }
     //
diff --git a/Silicon/Intel/Tools/FitGen/FitGen.h b/Silicon/Intel/Tools/FitGen/FitGen.h
index 9bd3f6824b..ecb5822d32 100644
--- a/Silicon/Intel/Tools/FitGen/FitGen.h
+++ b/Silicon/Intel/Tools/FitGen/FitGen.h
@@ -31,7 +31,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 // Utility version information
 //
 #define UTILITY_MAJOR_VERSION 0
-#define UTILITY_MINOR_VERSION 56
+#define UTILITY_MINOR_VERSION 57
 #define UTILITY_DATE          __DATE__
 
 //
-- 
2.13.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53716): https://edk2.groups.io/g/devel/message/53716
Mute This Topic: https://groups.io/mt/70968976/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.