public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section
@ 2016-08-18 17:00 Ard Biesheuvel
  2016-08-19  7:51 ` Gao, Liming
  0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2016-08-18 17:00 UTC (permalink / raw)
  To: edk2-devel, liming.gao, yonghong.zhu; +Cc: leif.lindholm, Ard Biesheuvel

Currently, GenFv warns about input files without a relocation section,
but still includes the FFS file in the image, but with its ImageBase
field left at zero. This confuses the loader routines in PEI core,
resulting in spurious write attempts to be made at the NOR flash.

When using the GCC LTO compiler for AArch64, the optimizations are so
effective that in some cases, all absolute symbol references are
optimized away completely, resulting in a PE/COFF image that has no
.reloc section at all (i.e., the PE/COFF image is position independent,
but purely by accident)

This means that, while unusual, it is not an error for a XIP image to
have no .reloc section. So teach GenFv about this, i.e., let it skip
the relocation routines, but still update the ImageBase address in the
header, and recalculate the checksums.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 223 ++++++++++----------
 1 file changed, 111 insertions(+), 112 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 7c839e277944..db0978d4089d 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -3500,63 +3500,63 @@ Returns:
     //
     if (ImageContext.RelocationsStripped) {
       Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.", FileName);
-      continue;
-    }
+    } else {
 
-    //
-    // Relocation exist and rebase
-    //
-    //
-    // Load and Relocate Image Data
-    //
-    MemoryImagePointer = (UINT8 *) malloc ((UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
-    if (MemoryImagePointer == NULL) {
-      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase of %s", FileName);
-      return EFI_OUT_OF_RESOURCES;
-    }
-    memset ((VOID *) MemoryImagePointer, 0, (UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
-    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer + ImageContext.SectionAlignment - 1) & (~((UINTN) ImageContext.SectionAlignment - 1));
-    
-    Status =  PeCoffLoaderLoadImage (&ImageContext);
-    if (EFI_ERROR (Status)) {
-      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s", FileName);
-      free ((VOID *) MemoryImagePointer);
-      return Status;
-    }
-         
-    ImageContext.DestinationAddress = NewPe32BaseAddress;
-    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
-    if (EFI_ERROR (Status)) {
-      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of %s", FileName);
-      free ((VOID *) MemoryImagePointer);
-      return Status;
-    }
+      //
+      // Relocation exist and rebase
+      //
+      //
+      // Load and Relocate Image Data
+      //
+      MemoryImagePointer = (UINT8 *) malloc ((UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
+      if (MemoryImagePointer == NULL) {
+        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase of %s", FileName);
+        return EFI_OUT_OF_RESOURCES;
+      }
+      memset ((VOID *) MemoryImagePointer, 0, (UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
+      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer + ImageContext.SectionAlignment - 1) & (~((UINTN) ImageContext.SectionAlignment - 1));
 
-    //
-    // Copy Relocated data to raw image file.
-    //
-    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
-                       (UINTN) ImgHdr +
-                       sizeof (UINT32) + 
-                       sizeof (EFI_IMAGE_FILE_HEADER) +  
-                       ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
-                       );
-    
-    for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections; Index ++, SectionHeader ++) {
-      CopyMem (
-        (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize + SectionHeader->PointerToRawData, 
-        (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress), 
-        SectionHeader->SizeOfRawData
-        );
-    }
+      Status =  PeCoffLoaderLoadImage (&ImageContext);
+      if (EFI_ERROR (Status)) {
+        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s", FileName);
+        free ((VOID *) MemoryImagePointer);
+        return Status;
+      }
 
-    free ((VOID *) MemoryImagePointer);
-    MemoryImagePointer = NULL;
-    if (PeFileBuffer != NULL) {
-      free (PeFileBuffer);
-      PeFileBuffer = NULL;
+      ImageContext.DestinationAddress = NewPe32BaseAddress;
+      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
+      if (EFI_ERROR (Status)) {
+        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of %s", FileName);
+        free ((VOID *) MemoryImagePointer);
+        return Status;
+      }
+
+      //
+      // Copy Relocated data to raw image file.
+      //
+      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
+                         (UINTN) ImgHdr +
+                         sizeof (UINT32) +
+                         sizeof (EFI_IMAGE_FILE_HEADER) +
+                         ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
+                         );
+
+      for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections; Index ++, SectionHeader ++) {
+        CopyMem (
+          (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize + SectionHeader->PointerToRawData,
+          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
+          SectionHeader->SizeOfRawData
+          );
+      }
+
+      free ((VOID *) MemoryImagePointer);
+      MemoryImagePointer = NULL;
+      if (PeFileBuffer != NULL) {
+        free (PeFileBuffer);
+        PeFileBuffer = NULL;
+      }
     }
-    
+
     //
     // Update Image Base Address
     //
@@ -3730,70 +3730,69 @@ Returns:
     //
     if (ImageContext.RelocationsStripped) {
       Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.", FileName);
-      continue;
-    }
+    } else {
+      //
+      // Relocation exist and rebase
+      //
+      //
+      // Load and Relocate Image Data
+      //
+      MemoryImagePointer = (UINT8 *) malloc ((UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
+      if (MemoryImagePointer == NULL) {
+        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase of %s", FileName);
+        return EFI_OUT_OF_RESOURCES;
+      }
+      memset ((VOID *) MemoryImagePointer, 0, (UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
+      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer + ImageContext.SectionAlignment - 1) & (~((UINTN) ImageContext.SectionAlignment - 1));
 
-    //
-    // Relocation exist and rebase
-    //
-    //
-    // Load and Relocate Image Data
-    //
-    MemoryImagePointer = (UINT8 *) malloc ((UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
-    if (MemoryImagePointer == NULL) {
-      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase of %s", FileName);
-      return EFI_OUT_OF_RESOURCES;
-    }
-    memset ((VOID *) MemoryImagePointer, 0, (UINTN) ImageContext.ImageSize + ImageContext.SectionAlignment);
-    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer + ImageContext.SectionAlignment - 1) & (~((UINTN) ImageContext.SectionAlignment - 1));
+      Status =  PeCoffLoaderLoadImage (&ImageContext);
+      if (EFI_ERROR (Status)) {
+        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s", FileName);
+        free ((VOID *) MemoryImagePointer);
+        return Status;
+      }
+      //
+      // Relocate TeImage
+      //
+      ImageContext.DestinationAddress = NewPe32BaseAddress;
+      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
+      if (EFI_ERROR (Status)) {
+        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of TE image %s", FileName);
+        free ((VOID *) MemoryImagePointer);
+        return Status;
+      }
 
-    Status =  PeCoffLoaderLoadImage (&ImageContext);
-    if (EFI_ERROR (Status)) {
-      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s", FileName);
-      free ((VOID *) MemoryImagePointer);
-      return Status;
-    }
-    //
-    // Reloacate TeImage
-    // 
-    ImageContext.DestinationAddress = NewPe32BaseAddress;
-    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
-    if (EFI_ERROR (Status)) {
-      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of TE image %s", FileName);
+      //
+      // Copy the relocated image into raw image file.
+      //
+      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1);
+      for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++, SectionHeader ++) {
+        if (!ImageContext.IsTeImage) {
+          CopyMem (
+            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
+            (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
+            SectionHeader->SizeOfRawData
+            );
+        } else {
+          CopyMem (
+            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
+            (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader->VirtualAddress),
+            SectionHeader->SizeOfRawData
+            );
+        }
+      }
+
+      //
+      // Free the allocated memory resource
+      //
       free ((VOID *) MemoryImagePointer);
-      return Status;
-    }
-    
-    //
-    // Copy the relocated image into raw image file.
-    //
-    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1);
-    for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++, SectionHeader ++) {
-      if (!ImageContext.IsTeImage) {
-        CopyMem (
-          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader->PointerToRawData, 
-          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress), 
-          SectionHeader->SizeOfRawData
-          );
-      } else {
-        CopyMem (
-          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader->PointerToRawData, 
-          (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader->VirtualAddress), 
-          SectionHeader->SizeOfRawData
-          );
+      MemoryImagePointer = NULL;
+      if (PeFileBuffer != NULL) {
+        free (PeFileBuffer);
+        PeFileBuffer = NULL;
       }
     }
-    
-    //
-    // Free the allocated memory resource
-    //
-    free ((VOID *) MemoryImagePointer);
-    MemoryImagePointer = NULL;
-    if (PeFileBuffer != NULL) {
-      free (PeFileBuffer);
-      PeFileBuffer = NULL;
-    }
-    
+
     //
     // Update Image Base Address
     //
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section
  2016-08-18 17:00 [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section Ard Biesheuvel
@ 2016-08-19  7:51 ` Gao, Liming
  2016-08-19 10:36   ` Ard Biesheuvel
  0 siblings, 1 reply; 4+ messages in thread
From: Gao, Liming @ 2016-08-19  7:51 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org, Zhu, Yonghong
  Cc: leif.lindholm@linaro.org

Ard:
  RelocationsStripped is decided by Pe32.FileHeader.Characteristics EFI_IMAGE_FILE_RELOCS_STRIPPED flag. Per PE/COFF spec, if IMAGE_FILE_RELOCS_STRIPPED flag is set, the image must therefore be loaded at its preferred base address. If the image could be loaded at other address, it should not set this flag in its PE header. Could you help check your PE/COFF image?

Thanks
Liming
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Friday, August 19, 2016 1:00 AM
> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu,
> Yonghong <yonghong.zhu@intel.com>
> Cc: leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section
> 
> Currently, GenFv warns about input files without a relocation section,
> but still includes the FFS file in the image, but with its ImageBase
> field left at zero. This confuses the loader routines in PEI core,
> resulting in spurious write attempts to be made at the NOR flash.
> 
> When using the GCC LTO compiler for AArch64, the optimizations are so
> effective that in some cases, all absolute symbol references are
> optimized away completely, resulting in a PE/COFF image that has no
> .reloc section at all (i.e., the PE/COFF image is position independent,
> but purely by accident)
> 
> This means that, while unusual, it is not an error for a XIP image to
> have no .reloc section. So teach GenFv about this, i.e., let it skip
> the relocation routines, but still update the ImageBase address in the
> header, and recalculate the checksums.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 223 ++++++++++----------
>  1 file changed, 111 insertions(+), 112 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index 7c839e277944..db0978d4089d 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -3500,63 +3500,63 @@ Returns:
>      //
>      if (ImageContext.RelocationsStripped) {
>        Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.",
> FileName);
> -      continue;
> -    }
> +    } else {
> 
> -    //
> -    // Relocation exist and rebase
> -    //
> -    //
> -    // Load and Relocate Image Data
> -    //
> -    MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> -    if (MemoryImagePointer == NULL) {
> -      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase
> of %s", FileName);
> -      return EFI_OUT_OF_RESOURCES;
> -    }
> -    memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> -    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> ImageContext.SectionAlignment - 1) & (~((UINTN)
> ImageContext.SectionAlignment - 1));
> -
> -    Status =  PeCoffLoaderLoadImage (&ImageContext);
> -    if (EFI_ERROR (Status)) {
> -      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s",
> FileName);
> -      free ((VOID *) MemoryImagePointer);
> -      return Status;
> -    }
> -
> -    ImageContext.DestinationAddress = NewPe32BaseAddress;
> -    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> -    if (EFI_ERROR (Status)) {
> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
> of %s", FileName);
> -      free ((VOID *) MemoryImagePointer);
> -      return Status;
> -    }
> +      //
> +      // Relocation exist and rebase
> +      //
> +      //
> +      // Load and Relocate Image Data
> +      //
> +      MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> +      if (MemoryImagePointer == NULL) {
> +        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
> rebase of %s", FileName);
> +        return EFI_OUT_OF_RESOURCES;
> +      }
> +      memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> +      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> ImageContext.SectionAlignment - 1) & (~((UINTN)
> ImageContext.SectionAlignment - 1));
> 
> -    //
> -    // Copy Relocated data to raw image file.
> -    //
> -    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
> -                       (UINTN) ImgHdr +
> -                       sizeof (UINT32) +
> -                       sizeof (EFI_IMAGE_FILE_HEADER) +
> -                       ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
> -                       );
> -
> -    for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections;
> Index ++, SectionHeader ++) {
> -      CopyMem (
> -        (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize +
> SectionHeader->PointerToRawData,
> -        (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >VirtualAddress),
> -        SectionHeader->SizeOfRawData
> -        );
> -    }
> +      Status =  PeCoffLoaderLoadImage (&ImageContext);
> +      if (EFI_ERROR (Status)) {
> +        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
> of %s", FileName);
> +        free ((VOID *) MemoryImagePointer);
> +        return Status;
> +      }
> 
> -    free ((VOID *) MemoryImagePointer);
> -    MemoryImagePointer = NULL;
> -    if (PeFileBuffer != NULL) {
> -      free (PeFileBuffer);
> -      PeFileBuffer = NULL;
> +      ImageContext.DestinationAddress = NewPe32BaseAddress;
> +      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> +      if (EFI_ERROR (Status)) {
> +        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
> of %s", FileName);
> +        free ((VOID *) MemoryImagePointer);
> +        return Status;
> +      }
> +
> +      //
> +      // Copy Relocated data to raw image file.
> +      //
> +      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
> +                         (UINTN) ImgHdr +
> +                         sizeof (UINT32) +
> +                         sizeof (EFI_IMAGE_FILE_HEADER) +
> +                         ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
> +                         );
> +
> +      for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections;
> Index ++, SectionHeader ++) {
> +        CopyMem (
> +          (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize +
> SectionHeader->PointerToRawData,
> +          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >VirtualAddress),
> +          SectionHeader->SizeOfRawData
> +          );
> +      }
> +
> +      free ((VOID *) MemoryImagePointer);
> +      MemoryImagePointer = NULL;
> +      if (PeFileBuffer != NULL) {
> +        free (PeFileBuffer);
> +        PeFileBuffer = NULL;
> +      }
>      }
> -
> +
>      //
>      // Update Image Base Address
>      //
> @@ -3730,70 +3730,69 @@ Returns:
>      //
>      if (ImageContext.RelocationsStripped) {
>        Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.",
> FileName);
> -      continue;
> -    }
> +    } else {
> +      //
> +      // Relocation exist and rebase
> +      //
> +      //
> +      // Load and Relocate Image Data
> +      //
> +      MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> +      if (MemoryImagePointer == NULL) {
> +        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
> rebase of %s", FileName);
> +        return EFI_OUT_OF_RESOURCES;
> +      }
> +      memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> +      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> ImageContext.SectionAlignment - 1) & (~((UINTN)
> ImageContext.SectionAlignment - 1));
> 
> -    //
> -    // Relocation exist and rebase
> -    //
> -    //
> -    // Load and Relocate Image Data
> -    //
> -    MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> -    if (MemoryImagePointer == NULL) {
> -      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase
> of %s", FileName);
> -      return EFI_OUT_OF_RESOURCES;
> -    }
> -    memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> ImageContext.ImageSize + ImageContext.SectionAlignment);
> -    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> ImageContext.SectionAlignment - 1) & (~((UINTN)
> ImageContext.SectionAlignment - 1));
> +      Status =  PeCoffLoaderLoadImage (&ImageContext);
> +      if (EFI_ERROR (Status)) {
> +        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
> of %s", FileName);
> +        free ((VOID *) MemoryImagePointer);
> +        return Status;
> +      }
> +      //
> +      // Relocate TeImage
> +      //
> +      ImageContext.DestinationAddress = NewPe32BaseAddress;
> +      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> +      if (EFI_ERROR (Status)) {
> +        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of
> TE image %s", FileName);
> +        free ((VOID *) MemoryImagePointer);
> +        return Status;
> +      }
> 
> -    Status =  PeCoffLoaderLoadImage (&ImageContext);
> -    if (EFI_ERROR (Status)) {
> -      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s",
> FileName);
> -      free ((VOID *) MemoryImagePointer);
> -      return Status;
> -    }
> -    //
> -    // Reloacate TeImage
> -    //
> -    ImageContext.DestinationAddress = NewPe32BaseAddress;
> -    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> -    if (EFI_ERROR (Status)) {
> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of
> TE image %s", FileName);
> +      //
> +      // Copy the relocated image into raw image file.
> +      //
> +      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1);
> +      for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++,
> SectionHeader ++) {
> +        if (!ImageContext.IsTeImage) {
> +          CopyMem (
> +            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> +            (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >VirtualAddress),
> +            SectionHeader->SizeOfRawData
> +            );
> +        } else {
> +          CopyMem (
> +            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> +            (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof
> (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader-
> >VirtualAddress),
> +            SectionHeader->SizeOfRawData
> +            );
> +        }
> +      }
> +
> +      //
> +      // Free the allocated memory resource
> +      //
>        free ((VOID *) MemoryImagePointer);
> -      return Status;
> -    }
> -
> -    //
> -    // Copy the relocated image into raw image file.
> -    //
> -    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1);
> -    for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++,
> SectionHeader ++) {
> -      if (!ImageContext.IsTeImage) {
> -        CopyMem (
> -          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> -          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >VirtualAddress),
> -          SectionHeader->SizeOfRawData
> -          );
> -      } else {
> -        CopyMem (
> -          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> -          (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof
> (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader-
> >VirtualAddress),
> -          SectionHeader->SizeOfRawData
> -          );
> +      MemoryImagePointer = NULL;
> +      if (PeFileBuffer != NULL) {
> +        free (PeFileBuffer);
> +        PeFileBuffer = NULL;
>        }
>      }
> -
> -    //
> -    // Free the allocated memory resource
> -    //
> -    free ((VOID *) MemoryImagePointer);
> -    MemoryImagePointer = NULL;
> -    if (PeFileBuffer != NULL) {
> -      free (PeFileBuffer);
> -      PeFileBuffer = NULL;
> -    }
> -
> +
>      //
>      // Update Image Base Address
>      //
> --
> 2.7.4



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section
  2016-08-19  7:51 ` Gao, Liming
@ 2016-08-19 10:36   ` Ard Biesheuvel
  2016-08-22  3:07     ` Gao, Liming
  0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2016-08-19 10:36 UTC (permalink / raw)
  To: Gao, Liming
  Cc: edk2-devel@lists.01.org, Zhu, Yonghong, leif.lindholm@linaro.org

On 19 August 2016 at 09:51, Gao, Liming <liming.gao@intel.com> wrote:
> Ard:
>   RelocationsStripped is decided by Pe32.FileHeader.Characteristics EFI_IMAGE_FILE_RELOCS_STRIPPED flag. Per PE/COFF spec, if IMAGE_FILE_RELOCS_STRIPPED flag is set, the image must therefore be loaded at its preferred base address. If the image could be loaded at other address, it should not set this flag in its PE header. Could you help check your PE/COFF image?
>

I think the problem is in the TE handling in the PE/COFF library:

--- a/BaseTools/Source/C/Common/BasePeCoff.c
+++ b/BaseTools/Source/C/Common/BasePeCoff.c
@@ -336,8 +336,6 @@ Returns:
   //
   if ((!(ImageContext->IsTeImage)) &&
((PeHdr->Pe32.FileHeader.Characteristics &
EFI_IMAGE_FILE_RELOCS_STRIPPED) != 0)) {
     ImageContext->RelocationsStripped = TRUE;
-  } else if ((ImageContext->IsTeImage) &&
(TeHdr->DataDirectory[0].Size == 0)) {
-    ImageContext->RelocationsStripped = TRUE;
   } else {
     ImageContext->RelocationsStripped = FALSE;
   }

IOW, it sets the RelocationsStripped flag for any TE image that has no
base reloc directory, but the TE image in question may not need one to
begin with (as is the case in my example)

Do you think we should apply the above change instead?

-- 
Ard.

>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Friday, August 19, 2016 1:00 AM
>> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu,
>> Yonghong <yonghong.zhu@intel.com>
>> Cc: leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section
>>
>> Currently, GenFv warns about input files without a relocation section,
>> but still includes the FFS file in the image, but with its ImageBase
>> field left at zero. This confuses the loader routines in PEI core,
>> resulting in spurious write attempts to be made at the NOR flash.
>>
>> When using the GCC LTO compiler for AArch64, the optimizations are so
>> effective that in some cases, all absolute symbol references are
>> optimized away completely, resulting in a PE/COFF image that has no
>> .reloc section at all (i.e., the PE/COFF image is position independent,
>> but purely by accident)
>>
>> This means that, while unusual, it is not an error for a XIP image to
>> have no .reloc section. So teach GenFv about this, i.e., let it skip
>> the relocation routines, but still update the ImageBase address in the
>> header, and recalculate the checksums.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 223 ++++++++++----------
>>  1 file changed, 111 insertions(+), 112 deletions(-)
>>
>> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> index 7c839e277944..db0978d4089d 100644
>> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> @@ -3500,63 +3500,63 @@ Returns:
>>      //
>>      if (ImageContext.RelocationsStripped) {
>>        Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.",
>> FileName);
>> -      continue;
>> -    }
>> +    } else {
>>
>> -    //
>> -    // Relocation exist and rebase
>> -    //
>> -    //
>> -    // Load and Relocate Image Data
>> -    //
>> -    MemoryImagePointer = (UINT8 *) malloc ((UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> -    if (MemoryImagePointer == NULL) {
>> -      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase
>> of %s", FileName);
>> -      return EFI_OUT_OF_RESOURCES;
>> -    }
>> -    memset ((VOID *) MemoryImagePointer, 0, (UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> -    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
>> ImageContext.SectionAlignment - 1) & (~((UINTN)
>> ImageContext.SectionAlignment - 1));
>> -
>> -    Status =  PeCoffLoaderLoadImage (&ImageContext);
>> -    if (EFI_ERROR (Status)) {
>> -      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s",
>> FileName);
>> -      free ((VOID *) MemoryImagePointer);
>> -      return Status;
>> -    }
>> -
>> -    ImageContext.DestinationAddress = NewPe32BaseAddress;
>> -    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
>> -    if (EFI_ERROR (Status)) {
>> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
>> of %s", FileName);
>> -      free ((VOID *) MemoryImagePointer);
>> -      return Status;
>> -    }
>> +      //
>> +      // Relocation exist and rebase
>> +      //
>> +      //
>> +      // Load and Relocate Image Data
>> +      //
>> +      MemoryImagePointer = (UINT8 *) malloc ((UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> +      if (MemoryImagePointer == NULL) {
>> +        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
>> rebase of %s", FileName);
>> +        return EFI_OUT_OF_RESOURCES;
>> +      }
>> +      memset ((VOID *) MemoryImagePointer, 0, (UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> +      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
>> ImageContext.SectionAlignment - 1) & (~((UINTN)
>> ImageContext.SectionAlignment - 1));
>>
>> -    //
>> -    // Copy Relocated data to raw image file.
>> -    //
>> -    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
>> -                       (UINTN) ImgHdr +
>> -                       sizeof (UINT32) +
>> -                       sizeof (EFI_IMAGE_FILE_HEADER) +
>> -                       ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
>> -                       );
>> -
>> -    for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections;
>> Index ++, SectionHeader ++) {
>> -      CopyMem (
>> -        (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize +
>> SectionHeader->PointerToRawData,
>> -        (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
>> >VirtualAddress),
>> -        SectionHeader->SizeOfRawData
>> -        );
>> -    }
>> +      Status =  PeCoffLoaderLoadImage (&ImageContext);
>> +      if (EFI_ERROR (Status)) {
>> +        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
>> of %s", FileName);
>> +        free ((VOID *) MemoryImagePointer);
>> +        return Status;
>> +      }
>>
>> -    free ((VOID *) MemoryImagePointer);
>> -    MemoryImagePointer = NULL;
>> -    if (PeFileBuffer != NULL) {
>> -      free (PeFileBuffer);
>> -      PeFileBuffer = NULL;
>> +      ImageContext.DestinationAddress = NewPe32BaseAddress;
>> +      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
>> +      if (EFI_ERROR (Status)) {
>> +        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
>> of %s", FileName);
>> +        free ((VOID *) MemoryImagePointer);
>> +        return Status;
>> +      }
>> +
>> +      //
>> +      // Copy Relocated data to raw image file.
>> +      //
>> +      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
>> +                         (UINTN) ImgHdr +
>> +                         sizeof (UINT32) +
>> +                         sizeof (EFI_IMAGE_FILE_HEADER) +
>> +                         ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
>> +                         );
>> +
>> +      for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections;
>> Index ++, SectionHeader ++) {
>> +        CopyMem (
>> +          (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize +
>> SectionHeader->PointerToRawData,
>> +          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
>> >VirtualAddress),
>> +          SectionHeader->SizeOfRawData
>> +          );
>> +      }
>> +
>> +      free ((VOID *) MemoryImagePointer);
>> +      MemoryImagePointer = NULL;
>> +      if (PeFileBuffer != NULL) {
>> +        free (PeFileBuffer);
>> +        PeFileBuffer = NULL;
>> +      }
>>      }
>> -
>> +
>>      //
>>      // Update Image Base Address
>>      //
>> @@ -3730,70 +3730,69 @@ Returns:
>>      //
>>      if (ImageContext.RelocationsStripped) {
>>        Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.",
>> FileName);
>> -      continue;
>> -    }
>> +    } else {
>> +      //
>> +      // Relocation exist and rebase
>> +      //
>> +      //
>> +      // Load and Relocate Image Data
>> +      //
>> +      MemoryImagePointer = (UINT8 *) malloc ((UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> +      if (MemoryImagePointer == NULL) {
>> +        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
>> rebase of %s", FileName);
>> +        return EFI_OUT_OF_RESOURCES;
>> +      }
>> +      memset ((VOID *) MemoryImagePointer, 0, (UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> +      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
>> ImageContext.SectionAlignment - 1) & (~((UINTN)
>> ImageContext.SectionAlignment - 1));
>>
>> -    //
>> -    // Relocation exist and rebase
>> -    //
>> -    //
>> -    // Load and Relocate Image Data
>> -    //
>> -    MemoryImagePointer = (UINT8 *) malloc ((UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> -    if (MemoryImagePointer == NULL) {
>> -      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase
>> of %s", FileName);
>> -      return EFI_OUT_OF_RESOURCES;
>> -    }
>> -    memset ((VOID *) MemoryImagePointer, 0, (UINTN)
>> ImageContext.ImageSize + ImageContext.SectionAlignment);
>> -    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
>> ImageContext.SectionAlignment - 1) & (~((UINTN)
>> ImageContext.SectionAlignment - 1));
>> +      Status =  PeCoffLoaderLoadImage (&ImageContext);
>> +      if (EFI_ERROR (Status)) {
>> +        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
>> of %s", FileName);
>> +        free ((VOID *) MemoryImagePointer);
>> +        return Status;
>> +      }
>> +      //
>> +      // Relocate TeImage
>> +      //
>> +      ImageContext.DestinationAddress = NewPe32BaseAddress;
>> +      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
>> +      if (EFI_ERROR (Status)) {
>> +        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of
>> TE image %s", FileName);
>> +        free ((VOID *) MemoryImagePointer);
>> +        return Status;
>> +      }
>>
>> -    Status =  PeCoffLoaderLoadImage (&ImageContext);
>> -    if (EFI_ERROR (Status)) {
>> -      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase of %s",
>> FileName);
>> -      free ((VOID *) MemoryImagePointer);
>> -      return Status;
>> -    }
>> -    //
>> -    // Reloacate TeImage
>> -    //
>> -    ImageContext.DestinationAddress = NewPe32BaseAddress;
>> -    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
>> -    if (EFI_ERROR (Status)) {
>> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of
>> TE image %s", FileName);
>> +      //
>> +      // Copy the relocated image into raw image file.
>> +      //
>> +      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1);
>> +      for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++,
>> SectionHeader ++) {
>> +        if (!ImageContext.IsTeImage) {
>> +          CopyMem (
>> +            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
>> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
>> +            (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
>> >VirtualAddress),
>> +            SectionHeader->SizeOfRawData
>> +            );
>> +        } else {
>> +          CopyMem (
>> +            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
>> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
>> +            (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof
>> (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader-
>> >VirtualAddress),
>> +            SectionHeader->SizeOfRawData
>> +            );
>> +        }
>> +      }
>> +
>> +      //
>> +      // Free the allocated memory resource
>> +      //
>>        free ((VOID *) MemoryImagePointer);
>> -      return Status;
>> -    }
>> -
>> -    //
>> -    // Copy the relocated image into raw image file.
>> -    //
>> -    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1);
>> -    for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++,
>> SectionHeader ++) {
>> -      if (!ImageContext.IsTeImage) {
>> -        CopyMem (
>> -          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
>> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
>> -          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
>> >VirtualAddress),
>> -          SectionHeader->SizeOfRawData
>> -          );
>> -      } else {
>> -        CopyMem (
>> -          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
>> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
>> -          (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof
>> (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize + SectionHeader-
>> >VirtualAddress),
>> -          SectionHeader->SizeOfRawData
>> -          );
>> +      MemoryImagePointer = NULL;
>> +      if (PeFileBuffer != NULL) {
>> +        free (PeFileBuffer);
>> +        PeFileBuffer = NULL;
>>        }
>>      }
>> -
>> -    //
>> -    // Free the allocated memory resource
>> -    //
>> -    free ((VOID *) MemoryImagePointer);
>> -    MemoryImagePointer = NULL;
>> -    if (PeFileBuffer != NULL) {
>> -      free (PeFileBuffer);
>> -      PeFileBuffer = NULL;
>> -    }
>> -
>> +
>>      //
>>      // Update Image Base Address
>>      //
>> --
>> 2.7.4
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section
  2016-08-19 10:36   ` Ard Biesheuvel
@ 2016-08-22  3:07     ` Gao, Liming
  0 siblings, 0 replies; 4+ messages in thread
From: Gao, Liming @ 2016-08-22  3:07 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Zhu, Yonghong, leif.lindholm@linaro.org

Ard:
  If PE image has no relocation section, and has not set EFI_IMAGE_FILE_RELOCS_STRIPPED, after it is converted to TE image, GenFw will set its relocation section VirtualAddress to non-zero address, and keep Size value as Zero. If PE image is without relocation section, and set EFI_IMAGE_FILE_RELOCS_STRIPPED, after it is converted to TE image, GenFw will keep its relocation section VA and Size as zero. Then, BasePeCoffLib can base on its relocation section fields to know its relocation attribute. edk2 MdePkg BasePeCoffLib has applied this rule to get RelocationsStripped attribute. But, it is missing in BaseTools BasePeCoffLib and cause your failure. We just need to sync the fix from MdePkg one. I will send the patch for review. 

Thanks
Liming
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Friday, August 19, 2016 6:36 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: edk2-devel@lists.01.org; Zhu, Yonghong <yonghong.zhu@intel.com>;
> leif.lindholm@linaro.org
> Subject: Re: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc
> section
> 
> On 19 August 2016 at 09:51, Gao, Liming <liming.gao@intel.com> wrote:
> > Ard:
> >   RelocationsStripped is decided by Pe32.FileHeader.Characteristics
> EFI_IMAGE_FILE_RELOCS_STRIPPED flag. Per PE/COFF spec, if
> IMAGE_FILE_RELOCS_STRIPPED flag is set, the image must therefore be
> loaded at its preferred base address. If the image could be loaded at other
> address, it should not set this flag in its PE header. Could you help check your
> PE/COFF image?
> >
> 
> I think the problem is in the TE handling in the PE/COFF library:
> 
> --- a/BaseTools/Source/C/Common/BasePeCoff.c
> +++ b/BaseTools/Source/C/Common/BasePeCoff.c
> @@ -336,8 +336,6 @@ Returns:
>    //
>    if ((!(ImageContext->IsTeImage)) &&
> ((PeHdr->Pe32.FileHeader.Characteristics &
> EFI_IMAGE_FILE_RELOCS_STRIPPED) != 0)) {
>      ImageContext->RelocationsStripped = TRUE;
> -  } else if ((ImageContext->IsTeImage) &&
> (TeHdr->DataDirectory[0].Size == 0)) {
> -    ImageContext->RelocationsStripped = TRUE;
>    } else {
>      ImageContext->RelocationsStripped = FALSE;
>    }
> 
> IOW, it sets the RelocationsStripped flag for any TE image that has no
> base reloc directory, but the TE image in question may not need one to
> begin with (as is the case in my example)
> 
> Do you think we should apply the above change instead?
> 
> --
> Ard.
> 
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: Friday, August 19, 2016 1:00 AM
> >> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu,
> >> Yonghong <yonghong.zhu@intel.com>
> >> Cc: leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Subject: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc
> section
> >>
> >> Currently, GenFv warns about input files without a relocation section,
> >> but still includes the FFS file in the image, but with its ImageBase
> >> field left at zero. This confuses the loader routines in PEI core,
> >> resulting in spurious write attempts to be made at the NOR flash.
> >>
> >> When using the GCC LTO compiler for AArch64, the optimizations are so
> >> effective that in some cases, all absolute symbol references are
> >> optimized away completely, resulting in a PE/COFF image that has no
> >> .reloc section at all (i.e., the PE/COFF image is position independent,
> >> but purely by accident)
> >>
> >> This means that, while unusual, it is not an error for a XIP image to
> >> have no .reloc section. So teach GenFv about this, i.e., let it skip
> >> the relocation routines, but still update the ImageBase address in the
> >> header, and recalculate the checksums.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 223 ++++++++++--------
> --
> >>  1 file changed, 111 insertions(+), 112 deletions(-)
> >>
> >> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >> index 7c839e277944..db0978d4089d 100644
> >> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> >> @@ -3500,63 +3500,63 @@ Returns:
> >>      //
> >>      if (ImageContext.RelocationsStripped) {
> >>        Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.",
> >> FileName);
> >> -      continue;
> >> -    }
> >> +    } else {
> >>
> >> -    //
> >> -    // Relocation exist and rebase
> >> -    //
> >> -    //
> >> -    // Load and Relocate Image Data
> >> -    //
> >> -    MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> -    if (MemoryImagePointer == NULL) {
> >> -      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
> rebase
> >> of %s", FileName);
> >> -      return EFI_OUT_OF_RESOURCES;
> >> -    }
> >> -    memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> -    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> >> ImageContext.SectionAlignment - 1) & (~((UINTN)
> >> ImageContext.SectionAlignment - 1));
> >> -
> >> -    Status =  PeCoffLoaderLoadImage (&ImageContext);
> >> -    if (EFI_ERROR (Status)) {
> >> -      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
> of %s",
> >> FileName);
> >> -      free ((VOID *) MemoryImagePointer);
> >> -      return Status;
> >> -    }
> >> -
> >> -    ImageContext.DestinationAddress = NewPe32BaseAddress;
> >> -    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> >> -    if (EFI_ERROR (Status)) {
> >> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
> >> of %s", FileName);
> >> -      free ((VOID *) MemoryImagePointer);
> >> -      return Status;
> >> -    }
> >> +      //
> >> +      // Relocation exist and rebase
> >> +      //
> >> +      //
> >> +      // Load and Relocate Image Data
> >> +      //
> >> +      MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> +      if (MemoryImagePointer == NULL) {
> >> +        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
> >> rebase of %s", FileName);
> >> +        return EFI_OUT_OF_RESOURCES;
> >> +      }
> >> +      memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> +      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> >> ImageContext.SectionAlignment - 1) & (~((UINTN)
> >> ImageContext.SectionAlignment - 1));
> >>
> >> -    //
> >> -    // Copy Relocated data to raw image file.
> >> -    //
> >> -    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
> >> -                       (UINTN) ImgHdr +
> >> -                       sizeof (UINT32) +
> >> -                       sizeof (EFI_IMAGE_FILE_HEADER) +
> >> -                       ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
> >> -                       );
> >> -
> >> -    for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections;
> >> Index ++, SectionHeader ++) {
> >> -      CopyMem (
> >> -        (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize +
> >> SectionHeader->PointerToRawData,
> >> -        (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >> >VirtualAddress),
> >> -        SectionHeader->SizeOfRawData
> >> -        );
> >> -    }
> >> +      Status =  PeCoffLoaderLoadImage (&ImageContext);
> >> +      if (EFI_ERROR (Status)) {
> >> +        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
> >> of %s", FileName);
> >> +        free ((VOID *) MemoryImagePointer);
> >> +        return Status;
> >> +      }
> >>
> >> -    free ((VOID *) MemoryImagePointer);
> >> -    MemoryImagePointer = NULL;
> >> -    if (PeFileBuffer != NULL) {
> >> -      free (PeFileBuffer);
> >> -      PeFileBuffer = NULL;
> >> +      ImageContext.DestinationAddress = NewPe32BaseAddress;
> >> +      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> >> +      if (EFI_ERROR (Status)) {
> >> +        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
> >> of %s", FileName);
> >> +        free ((VOID *) MemoryImagePointer);
> >> +        return Status;
> >> +      }
> >> +
> >> +      //
> >> +      // Copy Relocated data to raw image file.
> >> +      //
> >> +      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (
> >> +                         (UINTN) ImgHdr +
> >> +                         sizeof (UINT32) +
> >> +                         sizeof (EFI_IMAGE_FILE_HEADER) +
> >> +                         ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader
> >> +                         );
> >> +
> >> +      for (Index = 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections;
> >> Index ++, SectionHeader ++) {
> >> +        CopyMem (
> >> +          (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize +
> >> SectionHeader->PointerToRawData,
> >> +          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >> >VirtualAddress),
> >> +          SectionHeader->SizeOfRawData
> >> +          );
> >> +      }
> >> +
> >> +      free ((VOID *) MemoryImagePointer);
> >> +      MemoryImagePointer = NULL;
> >> +      if (PeFileBuffer != NULL) {
> >> +        free (PeFileBuffer);
> >> +        PeFileBuffer = NULL;
> >> +      }
> >>      }
> >> -
> >> +
> >>      //
> >>      // Update Image Base Address
> >>      //
> >> @@ -3730,70 +3730,69 @@ Returns:
> >>      //
> >>      if (ImageContext.RelocationsStripped) {
> >>        Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc section.",
> >> FileName);
> >> -      continue;
> >> -    }
> >> +    } else {
> >> +      //
> >> +      // Relocation exist and rebase
> >> +      //
> >> +      //
> >> +      // Load and Relocate Image Data
> >> +      //
> >> +      MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> +      if (MemoryImagePointer == NULL) {
> >> +        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
> >> rebase of %s", FileName);
> >> +        return EFI_OUT_OF_RESOURCES;
> >> +      }
> >> +      memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> +      ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> >> ImageContext.SectionAlignment - 1) & (~((UINTN)
> >> ImageContext.SectionAlignment - 1));
> >>
> >> -    //
> >> -    // Relocation exist and rebase
> >> -    //
> >> -    //
> >> -    // Load and Relocate Image Data
> >> -    //
> >> -    MemoryImagePointer = (UINT8 *) malloc ((UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> -    if (MemoryImagePointer == NULL) {
> >> -      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on
> rebase
> >> of %s", FileName);
> >> -      return EFI_OUT_OF_RESOURCES;
> >> -    }
> >> -    memset ((VOID *) MemoryImagePointer, 0, (UINTN)
> >> ImageContext.ImageSize + ImageContext.SectionAlignment);
> >> -    ImageContext.ImageAddress = ((UINTN) MemoryImagePointer +
> >> ImageContext.SectionAlignment - 1) & (~((UINTN)
> >> ImageContext.SectionAlignment - 1));
> >> +      Status =  PeCoffLoaderLoadImage (&ImageContext);
> >> +      if (EFI_ERROR (Status)) {
> >> +        Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
> >> of %s", FileName);
> >> +        free ((VOID *) MemoryImagePointer);
> >> +        return Status;
> >> +      }
> >> +      //
> >> +      // Relocate TeImage
> >> +      //
> >> +      ImageContext.DestinationAddress = NewPe32BaseAddress;
> >> +      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> >> +      if (EFI_ERROR (Status)) {
> >> +        Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
> of
> >> TE image %s", FileName);
> >> +        free ((VOID *) MemoryImagePointer);
> >> +        return Status;
> >> +      }
> >>
> >> -    Status =  PeCoffLoaderLoadImage (&ImageContext);
> >> -    if (EFI_ERROR (Status)) {
> >> -      Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on rebase
> of %s",
> >> FileName);
> >> -      free ((VOID *) MemoryImagePointer);
> >> -      return Status;
> >> -    }
> >> -    //
> >> -    // Reloacate TeImage
> >> -    //
> >> -    ImageContext.DestinationAddress = NewPe32BaseAddress;
> >> -    Status                          = PeCoffLoaderRelocateImage (&ImageContext);
> >> -    if (EFI_ERROR (Status)) {
> >> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase
> of
> >> TE image %s", FileName);
> >> +      //
> >> +      // Copy the relocated image into raw image file.
> >> +      //
> >> +      SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader
> + 1);
> >> +      for (Index = 0; Index < TEImageHeader->NumberOfSections; Index
> ++,
> >> SectionHeader ++) {
> >> +        if (!ImageContext.IsTeImage) {
> >> +          CopyMem (
> >> +            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> >> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> >> +            (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >> >VirtualAddress),
> >> +            SectionHeader->SizeOfRawData
> >> +            );
> >> +        } else {
> >> +          CopyMem (
> >> +            (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> >> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> >> +            (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof
> >> (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize +
> SectionHeader-
> >> >VirtualAddress),
> >> +            SectionHeader->SizeOfRawData
> >> +            );
> >> +        }
> >> +      }
> >> +
> >> +      //
> >> +      // Free the allocated memory resource
> >> +      //
> >>        free ((VOID *) MemoryImagePointer);
> >> -      return Status;
> >> -    }
> >> -
> >> -    //
> >> -    // Copy the relocated image into raw image file.
> >> -    //
> >> -    SectionHeader = (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader +
> 1);
> >> -    for (Index = 0; Index < TEImageHeader->NumberOfSections; Index ++,
> >> SectionHeader ++) {
> >> -      if (!ImageContext.IsTeImage) {
> >> -        CopyMem (
> >> -          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> >> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> >> -          (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
> >> >VirtualAddress),
> >> -          SectionHeader->SizeOfRawData
> >> -          );
> >> -      } else {
> >> -        CopyMem (
> >> -          (UINT8 *) TEImageHeader + sizeof (EFI_TE_IMAGE_HEADER) -
> >> TEImageHeader->StrippedSize + SectionHeader->PointerToRawData,
> >> -          (VOID*) (UINTN) (ImageContext.ImageAddress + sizeof
> >> (EFI_TE_IMAGE_HEADER) - TEImageHeader->StrippedSize +
> SectionHeader-
> >> >VirtualAddress),
> >> -          SectionHeader->SizeOfRawData
> >> -          );
> >> +      MemoryImagePointer = NULL;
> >> +      if (PeFileBuffer != NULL) {
> >> +        free (PeFileBuffer);
> >> +        PeFileBuffer = NULL;
> >>        }
> >>      }
> >> -
> >> -    //
> >> -    // Free the allocated memory resource
> >> -    //
> >> -    free ((VOID *) MemoryImagePointer);
> >> -    MemoryImagePointer = NULL;
> >> -    if (PeFileBuffer != NULL) {
> >> -      free (PeFileBuffer);
> >> -      PeFileBuffer = NULL;
> >> -    }
> >> -
> >> +
> >>      //
> >>      // Update Image Base Address
> >>      //
> >> --
> >> 2.7.4
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-08-22  3:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-18 17:00 [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section Ard Biesheuvel
2016-08-19  7:51 ` Gao, Liming
2016-08-19 10:36   ` Ard Biesheuvel
2016-08-22  3:07     ` Gao, Liming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox