All of lore.kernel.org
 help / color / mirror / Atom feed
From: pjones@redhat.com (Peter Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/7] arm64: efi: remove pointless dummy .reloc section
Date: Tue, 7 Feb 2017 13:30:57 -0500	[thread overview]
Message-ID: <20170207183056.42gqqg2jp32jexmy@redhat.com> (raw)
In-Reply-To: <1486398275-3966-6-git-send-email-ard.biesheuvel@linaro.org>

On Mon, Feb 06, 2017 at 04:24:33PM +0000, Ard Biesheuvel wrote:
> The kernel's EFI PE/COFF header contains a dummy .reloc section, and
> an explanatory comment that claims that this is required for the EFI
> application loader to accept the Image as a relocatable image (i.e.,
> one that can be loaded at any offset and fixed up in place)
> 
> This was inherited from the x86 implementation, which has elaborate host
> tooling to mangle the PE/COFF header post-link time, and which populates
> the .reloc section with a single dummy base relocation. On ARM, no such
> tooling exists, and the .reloc section remains empty, and is never even
> exposed via the BaseRelocationTable directory entry, which is where the
> PE/COFF loader looks for it.
> 
> The PE/COFF spec is unclear about relocatable images that do not require
> any fixups, but the EDK2 implementation, which is the de facto reference
> for PE/COFF in the UEFI space, clearly does not care, and explicitly
> mentions (in a comment) that relocatable images with no base relocations
> are perfectly fine, as long as they don't have the RELOCS_STRIPPED
> attribute set (which is not the case for our PE/COFF image)
> 
> So simply remove the .reloc section altogether.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Looks completely reasonable to me.

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

> ---
>  arch/arm64/kernel/efi-header.S | 22 +-------------------
>  1 file changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
> index 74a25c09a1b8..35b11654ecc5 100644
> --- a/arch/arm64/kernel/efi-header.S
> +++ b/arch/arm64/kernel/efi-header.S
> @@ -37,7 +37,7 @@ pe_header:
>  	.short 	0
>  coff_header:
>  	.short	0xaa64					// AArch64
> -	.short	2					// nr_sections
> +	.short	1					// nr_sections
>  	.long	0 					// TimeDateStamp
>  	.long	0					// PointerToSymbolTable
>  	.long	1					// NumberOfSymbols
> @@ -96,26 +96,6 @@ extra_header_fields:
>  
>  	// Section table
>  section_table:
> -
> -	/*
> -	 * The EFI application loader requires a relocation section
> -	 * because EFI applications must be relocatable.  This is a
> -	 * dummy section as far as we are concerned.
> -	 */
> -	.ascii	".reloc"
> -	.byte	0
> -	.byte	0					// end of 0 padding of section name
> -	.long	0
> -	.long	0
> -	.long	0					// SizeOfRawData
> -	.long	0					// PointerToRawData
> -	.long	0					// PointerToRelocations
> -	.long	0					// PointerToLineNumbers
> -	.short	0					// NumberOfRelocations
> -	.short	0					// NumberOfLineNumbers
> -	.long	0x42100040				// Characteristics (section flags)
> -
> -
>  	.ascii	".text"
>  	.byte	0
>  	.byte	0
> -- 
> 2.7.4
> 

-- 
  Peter

WARNING: multiple messages have this Message-ID (diff)
From: Peter Jones <pjones@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org, will.deacon@arm.com,
	catalin.marinas@arm.com, mark.rutland@arm.com,
	labbott@fedoraproject.org, kernel-hardening@lists.openwall.com,
	leif.lindholm@linaro.org
Subject: [kernel-hardening] Re: [PATCH 5/7] arm64: efi: remove pointless dummy .reloc section
Date: Tue, 7 Feb 2017 13:30:57 -0500	[thread overview]
Message-ID: <20170207183056.42gqqg2jp32jexmy@redhat.com> (raw)
In-Reply-To: <1486398275-3966-6-git-send-email-ard.biesheuvel@linaro.org>

On Mon, Feb 06, 2017 at 04:24:33PM +0000, Ard Biesheuvel wrote:
> The kernel's EFI PE/COFF header contains a dummy .reloc section, and
> an explanatory comment that claims that this is required for the EFI
> application loader to accept the Image as a relocatable image (i.e.,
> one that can be loaded at any offset and fixed up in place)
> 
> This was inherited from the x86 implementation, which has elaborate host
> tooling to mangle the PE/COFF header post-link time, and which populates
> the .reloc section with a single dummy base relocation. On ARM, no such
> tooling exists, and the .reloc section remains empty, and is never even
> exposed via the BaseRelocationTable directory entry, which is where the
> PE/COFF loader looks for it.
> 
> The PE/COFF spec is unclear about relocatable images that do not require
> any fixups, but the EDK2 implementation, which is the de facto reference
> for PE/COFF in the UEFI space, clearly does not care, and explicitly
> mentions (in a comment) that relocatable images with no base relocations
> are perfectly fine, as long as they don't have the RELOCS_STRIPPED
> attribute set (which is not the case for our PE/COFF image)
> 
> So simply remove the .reloc section altogether.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Looks completely reasonable to me.

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

> ---
>  arch/arm64/kernel/efi-header.S | 22 +-------------------
>  1 file changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
> index 74a25c09a1b8..35b11654ecc5 100644
> --- a/arch/arm64/kernel/efi-header.S
> +++ b/arch/arm64/kernel/efi-header.S
> @@ -37,7 +37,7 @@ pe_header:
>  	.short 	0
>  coff_header:
>  	.short	0xaa64					// AArch64
> -	.short	2					// nr_sections
> +	.short	1					// nr_sections
>  	.long	0 					// TimeDateStamp
>  	.long	0					// PointerToSymbolTable
>  	.long	1					// NumberOfSymbols
> @@ -96,26 +96,6 @@ extra_header_fields:
>  
>  	// Section table
>  section_table:
> -
> -	/*
> -	 * The EFI application loader requires a relocation section
> -	 * because EFI applications must be relocatable.  This is a
> -	 * dummy section as far as we are concerned.
> -	 */
> -	.ascii	".reloc"
> -	.byte	0
> -	.byte	0					// end of 0 padding of section name
> -	.long	0
> -	.long	0
> -	.long	0					// SizeOfRawData
> -	.long	0					// PointerToRawData
> -	.long	0					// PointerToRelocations
> -	.long	0					// PointerToLineNumbers
> -	.short	0					// NumberOfRelocations
> -	.short	0					// NumberOfLineNumbers
> -	.long	0x42100040				// Characteristics (section flags)
> -
> -
>  	.ascii	".text"
>  	.byte	0
>  	.byte	0
> -- 
> 2.7.4
> 

-- 
  Peter

  parent reply	other threads:[~2017-02-07 18:30 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-06 16:24 [PATCH 0/7] arm64: efi: PE/COFF cleanup/hardening Ard Biesheuvel
2017-02-06 16:24 ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 16:24 ` [PATCH 1/7] include: pe.h: allow for use in assembly Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 16:33   ` Mark Rutland
2017-02-06 16:33     ` [kernel-hardening] " Mark Rutland
2017-02-06 16:40     ` Ard Biesheuvel
2017-02-06 16:40       ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 16:24 ` [PATCH 2/7] include: pe.h: add some missing definitions Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 16:24 ` [PATCH 3/7] arm64: efi: move EFI header and related data to a separate .S file Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 17:03   ` Mark Rutland
2017-02-06 17:03     ` [kernel-hardening] " Mark Rutland
2017-02-06 17:07     ` Ard Biesheuvel
2017-02-06 17:07       ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 16:24 ` [PATCH 4/7] arm64: efi: ensure that the PE/COFF header pointer appears at offset 0x3c Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 17:05   ` Mark Rutland
2017-02-06 17:05     ` [kernel-hardening] " Mark Rutland
2017-02-06 16:24 ` [PATCH 5/7] arm64: efi: remove pointless dummy .reloc section Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 17:06   ` Mark Rutland
2017-02-06 17:06     ` [kernel-hardening] " Mark Rutland
2017-02-07 18:30   ` Peter Jones [this message]
2017-02-07 18:30     ` Peter Jones
2017-02-06 16:24 ` [PATCH 6/7] arm64: efi: replace open coded constants with symbolic ones Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel
2017-02-06 17:13   ` Mark Rutland
2017-02-06 17:13     ` [kernel-hardening] " Mark Rutland
2017-02-06 16:24 ` [PATCH 7/7] arm64: efi: split Image code and data into separate PE/COFF sections Ard Biesheuvel
2017-02-06 16:24   ` [kernel-hardening] " Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170207183056.42gqqg2jp32jexmy@redhat.com \
    --to=pjones@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.