From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x232.google.com (mail-io0-x232.google.com [IPv6:2607:f8b0:4001:c06::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 08DB51A1E04 for ; Fri, 19 Aug 2016 03:36:20 -0700 (PDT) Received: by mail-io0-x232.google.com with SMTP id m101so44252941ioi.2 for ; Fri, 19 Aug 2016 03:36:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=08FSZXDjzZfuSzsd9yFON/wm2mB5pAumZxBtBx+UlZo=; b=aRt3xSn6KEpe2RVsf5ALfFoc9txXeEvWdSmCp0F9d9Fy2+r00lq6udgpxLMsg+uBf+ rfwzXpRPWe9/BhaMSrtM9gzIGFFKxBaiBjMk7vHnMMy4WqpjefmH8Zx5uHo0im6zZkmi aVNM0NzoSdzOFbsfyjDfehfQlkWL3/tf5uQpw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=08FSZXDjzZfuSzsd9yFON/wm2mB5pAumZxBtBx+UlZo=; b=KVYv5iB2EQqy//240tqG35L5U/lfkoftTL4eph3RaI70YSjE5Fcs9l9euI2lduHCIH J2Zwx+qmmE8NJfZftZS0GzWj9K7eUPyHNPCPZhd7LKzUvziCppcI5f3LV5NQXNG7xmfF Ssu6Up34SLtkFnXe2+SqZH+nbE3fjUQc2Wn1Y03vSMG/DcwFTrfgTuTmL2FZKmqnLXuK 56vB2VfIq8T2JNmDG8UFfJFPQboF9jA1PefbxqOFzVGnxQAumWuaVbZQzd7jeJr4/sA7 RnugbN+Ghgcg9R7YCzQNPv5VgoIBiTis2BTP4P+//ny/RCqbLahCMnqZnjl50z/7Y++9 LFaA== X-Gm-Message-State: AEkooutuS3S8neUpGYpy7F4E94A9TzoZcjR79XmqL9IO4mpVlclgO5+Uh0g4vpx4N+/KoN4z9xWoFPIrffbDS0/D X-Received: by 10.107.41.67 with SMTP id p64mr8614177iop.130.1471602979170; Fri, 19 Aug 2016 03:36:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Fri, 19 Aug 2016 03:36:18 -0700 (PDT) In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A1155ED043@shsmsx102.ccr.corp.intel.com> References: <1471539628-6257-1-git-send-email-ard.biesheuvel@linaro.org> <4A89E2EF3DFEDB4C8BFDE51014F606A1155ED043@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 19 Aug 2016 12:36:18 +0200 Message-ID: To: "Gao, Liming" Cc: "edk2-devel@lists.01.org" , "Zhu, Yonghong" , "leif.lindholm@linaro.org" Subject: Re: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc section X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Aug 2016 10:36:20 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 19 August 2016 at 09:51, Gao, Liming wrote: > Ard: > RelocationsStripped is decided by Pe32.FileHeader.Characteristics EFI_I= MAGE_FILE_RELOCS_STRIPPED flag. Per PE/COFF spec, if IMAGE_FILE_RELOCS_STRI= PPED 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 t= his 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) !=3D 0)) { ImageContext->RelocationsStripped =3D TRUE; - } else if ((ImageContext->IsTeImage) && (TeHdr->DataDirectory[0].Size =3D=3D 0)) { - ImageContext->RelocationsStripped =3D TRUE; } else { ImageContext->RelocationsStripped =3D 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? --=20 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 ; Zhu, >> Yonghong >> Cc: leif.lindholm@linaro.org; Ard Biesheuvel >> Subject: [PATCH] BaseTools/GenFv: allow TE/PE images with no .reloc sect= ion >> >> 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 >> --- >> 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 sectio= n.", >> FileName); >> - continue; >> - } >> + } else { >> >> - // >> - // Relocation exist and rebase >> - // >> - // >> - // Load and Relocate Image Data >> - // >> - MemoryImagePointer =3D (UINT8 *) malloc ((UINTN) >> ImageContext.ImageSize + ImageContext.SectionAlignment); >> - if (MemoryImagePointer =3D=3D 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 =3D ((UINTN) MemoryImagePointer + >> ImageContext.SectionAlignment - 1) & (~((UINTN) >> ImageContext.SectionAlignment - 1)); >> - >> - Status =3D PeCoffLoaderLoadImage (&ImageContext); >> - if (EFI_ERROR (Status)) { >> - Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on re= base of %s", >> FileName); >> - free ((VOID *) MemoryImagePointer); >> - return Status; >> - } >> - >> - ImageContext.DestinationAddress =3D NewPe32BaseAddress; >> - Status =3D PeCoffLoaderRelocateImage (&Ima= geContext); >> - 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 =3D (UINT8 *) malloc ((UINTN) >> ImageContext.ImageSize + ImageContext.SectionAlignment); >> + if (MemoryImagePointer =3D=3D NULL) { >> + Error (NULL, 0, 4001, "Resource", "memory cannot be allocated o= n >> rebase of %s", FileName); >> + return EFI_OUT_OF_RESOURCES; >> + } >> + memset ((VOID *) MemoryImagePointer, 0, (UINTN) >> ImageContext.ImageSize + ImageContext.SectionAlignment); >> + ImageContext.ImageAddress =3D ((UINTN) MemoryImagePointer + >> ImageContext.SectionAlignment - 1) & (~((UINTN) >> ImageContext.SectionAlignment - 1)); >> >> - // >> - // Copy Relocated data to raw image file. >> - // >> - SectionHeader =3D (EFI_IMAGE_SECTION_HEADER *) ( >> - (UINTN) ImgHdr + >> - sizeof (UINT32) + >> - sizeof (EFI_IMAGE_FILE_HEADER) + >> - ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader >> - ); >> - >> - for (Index =3D 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSections; >> Index ++, SectionHeader ++) { >> - CopyMem ( >> - (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize + >> SectionHeader->PointerToRawData, >> - (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader- >> >VirtualAddress), >> - SectionHeader->SizeOfRawData >> - ); >> - } >> + Status =3D 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 =3D NULL; >> - if (PeFileBuffer !=3D NULL) { >> - free (PeFileBuffer); >> - PeFileBuffer =3D NULL; >> + ImageContext.DestinationAddress =3D NewPe32BaseAddress; >> + Status =3D PeCoffLoaderRelocateImage (&I= mageContext); >> + if (EFI_ERROR (Status)) { >> + Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed o= n rebase >> of %s", FileName); >> + free ((VOID *) MemoryImagePointer); >> + return Status; >> + } >> + >> + // >> + // Copy Relocated data to raw image file. >> + // >> + SectionHeader =3D (EFI_IMAGE_SECTION_HEADER *) ( >> + (UINTN) ImgHdr + >> + sizeof (UINT32) + >> + sizeof (EFI_IMAGE_FILE_HEADER) + >> + ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader >> + ); >> + >> + for (Index =3D 0; Index < ImgHdr->Pe32.FileHeader.NumberOfSection= s; >> Index ++, SectionHeader ++) { >> + CopyMem ( >> + (UINT8 *) CurrentPe32Section.Pe32Section + CurSecHdrSize + >> SectionHeader->PointerToRawData, >> + (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader- >> >VirtualAddress), >> + SectionHeader->SizeOfRawData >> + ); >> + } >> + >> + free ((VOID *) MemoryImagePointer); >> + MemoryImagePointer =3D NULL; >> + if (PeFileBuffer !=3D NULL) { >> + free (PeFileBuffer); >> + PeFileBuffer =3D NULL; >> + } >> } >> - >> + >> // >> // Update Image Base Address >> // >> @@ -3730,70 +3730,69 @@ Returns: >> // >> if (ImageContext.RelocationsStripped) { >> Warning (NULL, 0, 0, "Invalid", "The file %s has no .reloc sectio= n.", >> FileName); >> - continue; >> - } >> + } else { >> + // >> + // Relocation exist and rebase >> + // >> + // >> + // Load and Relocate Image Data >> + // >> + MemoryImagePointer =3D (UINT8 *) malloc ((UINTN) >> ImageContext.ImageSize + ImageContext.SectionAlignment); >> + if (MemoryImagePointer =3D=3D NULL) { >> + Error (NULL, 0, 4001, "Resource", "memory cannot be allocated o= n >> rebase of %s", FileName); >> + return EFI_OUT_OF_RESOURCES; >> + } >> + memset ((VOID *) MemoryImagePointer, 0, (UINTN) >> ImageContext.ImageSize + ImageContext.SectionAlignment); >> + ImageContext.ImageAddress =3D ((UINTN) MemoryImagePointer + >> ImageContext.SectionAlignment - 1) & (~((UINTN) >> ImageContext.SectionAlignment - 1)); >> >> - // >> - // Relocation exist and rebase >> - // >> - // >> - // Load and Relocate Image Data >> - // >> - MemoryImagePointer =3D (UINT8 *) malloc ((UINTN) >> ImageContext.ImageSize + ImageContext.SectionAlignment); >> - if (MemoryImagePointer =3D=3D 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 =3D ((UINTN) MemoryImagePointer + >> ImageContext.SectionAlignment - 1) & (~((UINTN) >> ImageContext.SectionAlignment - 1)); >> + Status =3D 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 =3D NewPe32BaseAddress; >> + Status =3D PeCoffLoaderRelocateImage (&I= mageContext); >> + if (EFI_ERROR (Status)) { >> + Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed o= n rebase of >> TE image %s", FileName); >> + free ((VOID *) MemoryImagePointer); >> + return Status; >> + } >> >> - Status =3D PeCoffLoaderLoadImage (&ImageContext); >> - if (EFI_ERROR (Status)) { >> - Error (NULL, 0, 3000, "Invalid", "LocateImage() call failed on re= base of %s", >> FileName); >> - free ((VOID *) MemoryImagePointer); >> - return Status; >> - } >> - // >> - // Reloacate TeImage >> - // >> - ImageContext.DestinationAddress =3D NewPe32BaseAddress; >> - Status =3D PeCoffLoaderRelocateImage (&Ima= geContext); >> - 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 =3D (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1= ); >> + for (Index =3D 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 =3D (EFI_IMAGE_SECTION_HEADER *) (TEImageHeader + 1); >> - for (Index =3D 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 =3D NULL; >> + if (PeFileBuffer !=3D NULL) { >> + free (PeFileBuffer); >> + PeFileBuffer =3D NULL; >> } >> } >> - >> - // >> - // Free the allocated memory resource >> - // >> - free ((VOID *) MemoryImagePointer); >> - MemoryImagePointer =3D NULL; >> - if (PeFileBuffer !=3D NULL) { >> - free (PeFileBuffer); >> - PeFileBuffer =3D NULL; >> - } >> - >> + >> // >> // Update Image Base Address >> // >> -- >> 2.7.4 >