[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] mkimage: avoid copying relocations for sections that
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v2 2/3] mkimage: avoid copying relocations for sections that won't be copied. |
Date: |
Wed, 21 Feb 2018 12:18:54 +0100 |
User-agent: |
Mutt/1.3.28i |
On Tue, Feb 20, 2018 at 06:25:32PM -0500, Peter Jones wrote:
> Some versions of gcc include a plugin called "annobin", and in some
> build systems this is enabled by default. This plugin creates special
> ELF note sections to track which ABI-breaking features are used by a
> binary, as well as a series of relocations to annotate where.
>
> If grub is compiled with this feature, then when grub-mkimage translates
> the binary to another file format which does not strongly associate
> relocation data with sections (i.e. when platform is *-efi), these
> relocations appear to be against the .text section rather than the
> original note section. When the binary is loaded by the PE runtime
> loader, hilarity ensues.
>
> This issue is not necessarily limited to the annobin, but could arise
> any time there are relocations in sections that are not represented in
> grub-mkimage's output.
>
> This patch seeks to avoid this issue by only including relocations that
> refer to sections which will be included in the final binary.
>
> As an aside, this should also obviate the need to avoid -funwind-tables,
> -fasynchronous-unwind-tables, and any sections similar to .eh_frame in
> the future. I've tested it on x86-64-efi with the following gcc command
> line options (as recorded by -grecord-gcc-flags), but I still need to
> test the result on some other platforms that have been problematic in
> the past (especially ARM Aarch64) before I feel comfortable making
> changes to the configure.ac bits:
>
> GNU C11 7.2.1 20180116 (Red Hat 7.2.1-7) -mno-mmx -mno-sse -mno-sse2
> -mno-sse3 -mno-3dnow -msoft-float -mno-stack-arg-probe -mcmodel=large
> -mno-red-zone -m64 -mtune=generic -march=x86-64 -g3 -Os -freg-struct-return
> -fno-stack-protector -ffreestanding -funwind-tables
> -fasynchronous-unwind-tables -fno-strict-aliasing -fstack-clash-protection
> -fno-ident -fplugin=annobin
>
> Signed-off-by: Peter Jones <address@hidden>
One nit pick below...
Otherwise Reviewed-by: Daniel Kiper <address@hidden>
And +/- sdata name change...
> ---
> util/grub-mkimagexx.c | 79
> +++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 71 insertions(+), 8 deletions(-)
>
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index 5c787ec56bf..c15079c96ba 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -716,6 +716,12 @@ arm_get_trampoline_size (Elf_Ehdr *e,
> }
> #endif
>
> +static int
> +SUFFIX (is_kept_section) (Elf_Shdr *s, const struct
> grub_install_image_target_desc *image_target);
> +static int
> +SUFFIX (is_kept_reloc_section) (Elf_Shdr *s, const struct
> grub_install_image_target_desc *image_target,
> + struct section_metadata *sdata);
> +
> /* Deal with relocation information. This function relocates addresses
> within the virtual address space starting from 0. So only relative
> addresses can be fully resolved. Absolute addresses must be relocated
> @@ -750,6 +756,14 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, struct
> section_metadata *sdata,
> Elf_Shdr *target_section;
> Elf_Word j;
>
> + if (!SUFFIX (is_kept_section) (s, image_target) &&
> + !SUFFIX (is_kept_reloc_section) (s, image_target, sdata))
> + {
> + grub_util_info ("not translating relocations for omitted section
> %s",
> + sdata->strtab + grub_le_to_cpu32 (s->sh_name));
> + continue;
> + }
> +
> target_section_index = grub_target_to_host32 (s->sh_info);
> target_section_addr = sdata->addresses[target_section_index];
> target_section = (Elf_Shdr *) ((char *) sdata->sections
> @@ -1654,6 +1668,13 @@ make_reloc_section (Elf_Ehdr *e, struct
> grub_mkimage_layout *layout,
> Elf_Addr section_address;
> Elf_Word j;
>
> + if (!SUFFIX (is_kept_reloc_section) (s, image_target, sdata))
> + {
> + grub_util_info ("not translating the skipped relocation section %s",
> + sdata->strtab + grub_le_to_cpu32 (s->sh_name));
> + continue;
> + }
> +
> grub_util_info ("translating the relocation section %s",
> sdata->strtab + grub_le_to_cpu32 (s->sh_name));
>
> @@ -1729,6 +1750,55 @@ SUFFIX (is_bss_section) (Elf_Shdr *s, const struct
> grub_install_image_target_des
> == SHF_ALLOC) && (grub_target_to_host32 (s->sh_type) == SHT_NOBITS);
> }
>
> +/* Determine if a section is going to be in the final output */
> +static int
> +SUFFIX (is_kept_section) (Elf_Shdr *s, const struct
> grub_install_image_target_desc *image_target)
> +{
> + /* We keep .text and .data */
> + if (SUFFIX (is_text_section) (s, image_target)
> + || SUFFIX (is_data_section) (s, image_target))
> + return 1;
> +
> + /* And we keep .bss if we're producing PE binaries or the target doesn't
/*
* And we keep .bss if we're producing PE binaries or the target doesn't
> + * have a relocating loader. Platforms other than EFI and U-boot shouldn't
> + * have .bss in their binaries as we build with -Wl,-Ttext.
> + */
Daniel
- Re: [PATCH 1/2] mkimage: avoid copying relocations for sections that won't be copied., Peter Jones, 2018/02/15
- Re: [PATCH 1/2] mkimage: avoid copying relocations for sections that won't be copied., Vladimir 'phcoder' Serbinenko, 2018/02/15
- Re: [PATCH 1/2] mkimage: avoid copying relocations for sections that won't be copied., Daniel Kiper, 2018/02/20
- [PATCH v2 1/3] mkimage: refactor a bunch of section data into a struct., Peter Jones, 2018/02/20
- [PATCH v2 3/3] .mod files: Strip annobin annotations and .eh_frame, and their relocations, Peter Jones, 2018/02/20
- Re: [PATCH v2 3/3] .mod files: Strip annobin annotations and .eh_frame, and their relocations, Daniel Kiper, 2018/02/21
- [PATCH v2 2/3] mkimage: avoid copying relocations for sections that won't be copied., Peter Jones, 2018/02/20
- Re: [PATCH v2 2/3] mkimage: avoid copying relocations for sections that won't be copied.,
Daniel Kiper <=
- Re: [PATCH v2 1/3] mkimage: refactor a bunch of section data into a struct., Daniel Kiper, 2018/02/21
- Re: [PATCH v2 1/3] mkimage: refactor a bunch of section data into a struct., Peter Jones, 2018/02/21
- [PATCH v3 1/7] aout.h: Fix missing include., Peter Jones, 2018/02/21
- [PATCH v3 7/7] .mod files: Strip annobin annotations and .eh_frame, and their relocations, Peter Jones, 2018/02/21
- Re: [PATCH v3 7/7] .mod files: Strip annobin annotations and .eh_frame, and their relocations, Daniel Kiper, 2018/02/23
- Re: [PATCH v3 7/7] .mod files: Strip annobin annotations and .eh_frame, and their relocations, Daniel Kiper, 2018/02/23
- [PATCH v3 3/7] mkimage: rename a couple of things to be less confusing later., Peter Jones, 2018/02/21
- Re: [PATCH v3 3/7] mkimage: rename a couple of things to be less confusing later., Daniel Kiper, 2018/02/23
- [PATCH v3 2/7] mkimage: make it easier to run syntax checkers on grub-mkimagexx.c, Peter Jones, 2018/02/21
- Re: [PATCH v3 2/7] mkimage: make it easier to run syntax checkers on grub-mkimagexx.c, Daniel Kiper, 2018/02/23