From a7249a65aff174d2a51d6a7bf77dbbf58744a170 Mon Sep 17 00:00:00 2001

From: Peter Jones <pjones@redhat.com>

Date: Thu, 18 Sep 2014 18:34:38 -0400

Subject: [PATCH 55/74] Actually refer to the base relocation table of our

 loaded image.

Currently when we process base relocations, we get the correct Data

Directory pointer from the headers (context->RelocDir), and that header

has been copied into our pristine allocated image when we copied up to

SizeOfHeaders.  But the data it points to has not been mirrored in to

the new image, so it is whatever data AllocPool() gave us.

This patch changes relocate_coff() to refer to the base relocation table

from the image we loaded from disk, but apply the fixups to the new

copy.

I have no idea how x86_64 worked without this, but I can't make aarch64

work without it.  I also don't know how Ard or Leif have seen aarch64

work.  Maybe they haven't?  Leif indicated on irc that they may have

only tested shim with simple "hello world" applications from gnu-efi;

they are certainly much less complex than grub.efi, and are generated

through a different linking process.

My only theory is that we're getting recycled data there pretty reliably

that just makes us /not/ process any relocations, but since our

ImageBase is 0, and I don't think we ever load grub with 0 as its base

virtual address, that doesn't follow.  I'm open to any other ideas

anybody has.

I do know that on x86_64 (and presumably aarch64 as well), we don't

actually start seeing *symptoms* of this bug until the first chunk[0] of

94c9a77f is applied[1].  Once that is applied, relocate_coff() starts

seeing zero[2] for both RelocBase->VirtualAddress and

RelocBase->SizeOfBlock, because RelocBase is a (generated, relative)

pointer that only makes sense in the context of the original binary, not

our partial copy.  Since RelocBase->SizeOfBlock is tested first,

relocate_base() gives us "Reloc block size is invalid"[3] and returns

EFI_UNSUPPORTED.  At that point shim exits with an error.

[0] The second chunk of 94c9a77f patch makes no difference on this

    issue.

[1] I don't see why at all.

[2] Which could really be any value since it's AllocatePool() and not

    AllocateZeroPool() results, but 0 is all I've observed; I think

    AllocatePool() has simply never recycled any memory in my test

    cases.

[3] which is silent because perror() tries to avoid talking because that

    has caused much crashing in the past; work needs to go in to 0.9 for

    this.

Signed-off-by: Peter Jones <pjones@redhat.com>

---

 shim.c | 42 +++++++++++++++++++++---------------------

 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/shim.c b/shim.c

index 1ec1e11..4b4d31a 100644

--- a/shim.c

+++ b/shim.c

@@ -122,7 +122,7 @@ static void *ImageAddress (void *image, unsigned int size, unsigned int address)

  * Perform the actual relocation

  */

 static EFI_STATUS relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context,

-				 void *data)

+				 void *orig, void *data)

 {

 	EFI_IMAGE_BASE_RELOCATION *RelocBase, *RelocBaseEnd;

 	UINT64 Adjust;

@@ -132,7 +132,7 @@ static EFI_STATUS relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context,

 	UINT32 *Fixup32;

 	UINT64 *Fixup64;

 	int size = context->ImageSize;

-	void *ImageEnd = (char *)data + size;

+	void *ImageEnd = (char *)orig + size;

 #if __LP64__

 	context->PEHdr->Pe32Plus.OptionalHeader.ImageBase = (UINT64)data;

@@ -140,16 +140,8 @@ static EFI_STATUS relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context,

 	context->PEHdr->Pe32.OptionalHeader.ImageBase = (UINT32)data;

 #endif

-	if (context->NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {

-		perror(L"Image has no relocation entry\n");

-		return EFI_UNSUPPORTED;

-	}

-

-	if (!context->RelocDir->Size)

-		return EFI_SUCCESS;

-

-	RelocBase = ImageAddress(data, size, context->RelocDir->VirtualAddress);

-	RelocBaseEnd = ImageAddress(data, size, context->RelocDir->VirtualAddress + context->RelocDir->Size - 1);

+	RelocBase = ImageAddress(orig, size, context->RelocDir->VirtualAddress);

+	RelocBaseEnd = ImageAddress(orig, size, context->RelocDir->VirtualAddress + context->RelocDir->Size - 1);

 	if (!RelocBase || !RelocBaseEnd) {

 		perror(L"Reloc table overflows binary\n");

@@ -170,7 +162,7 @@ static EFI_STATUS relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context,

 		}

 		RelocEnd = (UINT16 *) ((char *) RelocBase + RelocBase->SizeOfBlock);

-		if ((void *)RelocEnd < data || (void *)RelocEnd > ImageEnd) {

+		if ((void *)RelocEnd < orig || (void *)RelocEnd > ImageEnd) {

 			perror(L"Reloc entry overflows binary\n");

 			return EFI_UNSUPPORTED;

 		}

@@ -1049,15 +1041,23 @@ static EFI_STATUS handle_image (void *data, unsigned int datasize,

 			ZeroMem (base + size, Section->Misc.VirtualSize - size);

 	}

-	/*

-	 * Run the relocation fixups

-	 */

-	efi_status = relocate_coff(&context, buffer);

-

-	if (efi_status != EFI_SUCCESS) {

-		perror(L"Relocation failed: %r\n", efi_status);

+	if (context.NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {

+		perror(L"Image has no relocation entry\n");

 		FreePool(buffer);

-		return efi_status;

+		return EFI_UNSUPPORTED;

+	}

+

+	if (context.RelocDir->Size) {

+		/*

+		 * Run the relocation fixups

+		 */

+		efi_status = relocate_coff(&context, data, buffer);

+

+		if (efi_status != EFI_SUCCESS) {

+			perror(L"Relocation failed: %r\n", efi_status);

+			FreePool(buffer);

+			return efi_status;

+		}

 	}

 	entry_point = ImageAddress(buffer, context.ImageSize, context.EntryPoint);

-- 

1.9.3