(adding Leif and Andrew, who merged this code originally)

On 23 September 2015 at 21:31, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote:
> The base relocation type EFI_IMAGE_REL_BASED_ARM_MOV32T patches an
> absolute address into the immediate fields of an adjacent movt/movw
> instruction pair.
>
> As the instructions are not writable by the program itself, there is
> no need to keep track of the fixup data, since we can reapply the
> relocation for runtime unconditionally. So remove the fixup handling
> for this relocation type.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>  MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c 
> b/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c
> index d6bf42738d2b..03a532c60514 100644
> --- a/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c
> +++ b/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c
> @@ -158,13 +158,6 @@ PeCoffLoaderRelocateImageEx (
>    case EFI_IMAGE_REL_BASED_ARM_MOV32T:
>      FixupVal = ThumbMovwMovtImmediateAddress (Fixup16) + (UINT32)Adjust;
>      ThumbMovwMovtImmediatePatch (Fixup16, FixupVal);
> -
> -    if (*FixupData != NULL) {
> -      *FixupData = ALIGN_POINTER(*FixupData, sizeof(UINT64));
> -      // Fixup16 is not aligned so we must copy it. Thumb instructions are 
> streams of 16 bytes.
> -      CopyMem (*FixupData, Fixup16, sizeof (UINT64));
> -      *FixupData = *FixupData + sizeof(UINT64);
> -    }
>      break;
>
>    case EFI_IMAGE_REL_BASED_ARM_MOV32A:
> @@ -229,11 +222,8 @@ PeHotRelocateImageEx (
>    switch ((*Reloc) >> 12) {
>
>    case EFI_IMAGE_REL_BASED_ARM_MOV32T:
> -    *FixupData  = ALIGN_POINTER (*FixupData, sizeof (UINT64));
> -    if (*(UINT64 *) (*FixupData) == ReadUnaligned64 ((UINT64 *)Fixup16)) {
> -      FixupVal = ThumbMovwMovtImmediateAddress (Fixup16) + (UINT32)Adjust;
> -      ThumbMovwMovtImmediatePatch (Fixup16, FixupVal);
> -    }
> +    FixupVal = ThumbMovwMovtImmediateAddress (Fixup16) + (UINT32)Adjust;
> +    ThumbMovwMovtImmediatePatch (Fixup16, FixupVal);
>      break;
>
>    case EFI_IMAGE_REL_BASED_ARM_MOV32A:

To elaborate a bit, this patch actually fixes a bug on ARM: since
PeHotRelocateImageEx () fails to advance the FixupData pointer, it
will think the targets of these relocations have been updated by the
program since the first relocation occurred, and leave them alone
instead of relocating them to the OS VA mapping.

If people prefer, I could also fix this by adding the increment of
*FixupData in the correct place, but since the whole check is a bit
pointless, I preferred to just rip it out.

-- 
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to