Patch looks good. I'm sick right now. I'll commit in few days when I feel
better unless someone commits it before

Le mer. 31 janv. 2018 à 17:28, Peter Jones <pjo...@redhat.com> a écrit :

> 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 <pjo...@redhat.com>
> ---
>  util/grub-mkimagexx.c | 81
> ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 73 insertions(+), 8 deletions(-)
>
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index a2bb05439f0..b016b061c8c 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -708,6 +708,13 @@ 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,
> +                               Elf_Shdr *sections, Elf_Half
> section_entsize, Elf_Half num_sections,
> +                               const char *strtab);
> +
>  /* 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
> @@ -747,6 +754,14 @@ SUFFIX (relocate_addresses) (Elf_Ehdr *e, Elf_Shdr
> *sections,
>         Elf_Shdr *target_section;
>         Elf_Word j;
>
> +       if (!SUFFIX (is_kept_section) (s, image_target) &&
> +           !SUFFIX (is_kept_reloc_section) (s, image_target, sections,
> section_entsize, num_sections, strtab))
> +         {
> +           grub_util_info ("not translating relocations for omitted
> section %s",
> +                       strtab + grub_le_to_cpu32 (s->sh_name));
> +           continue;
> +         }
> +
>         symtab_section = (Elf_Shdr *) ((char *) sections
>                                          + (grub_target_to_host32
> (s->sh_link)
>                                             * section_entsize));
> @@ -1656,6 +1671,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, sections,
> section_entsize, num_sections, strtab))
> +         {
> +           grub_util_info ("not translating the skipped relocation
> section %s",
> +                           strtab + grub_le_to_cpu32 (s->sh_name));
> +           continue;
> +         }
> +
>         grub_util_info ("translating the relocation section %s",
>                         strtab + grub_le_to_cpu32 (s->sh_name));
>
> @@ -1731,6 +1753,56 @@ 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
> +   * have a relocating loader.  Platforms other than EFI and U-boot
> shouldn't
> +   * have .bss in their binaries as we build with -Wl,-Ttext.
> +   */
> +  if (SUFFIX (is_bss_section) (s, image_target)
> +      && (image_target->id == IMAGE_EFI || !is_relocatable
> (image_target)))
> +    return 1;
> +
> +  /* Otherwise this is not a section we're keeping in the final output. */
> +  return 0;
> +}
> +
> +static int
> +SUFFIX (is_kept_reloc_section) (Elf_Shdr *s, const struct
> grub_install_image_target_desc *image_target,
> +                               Elf_Shdr *sections, Elf_Half
> section_entsize, Elf_Half num_sections,
> +                               const char *strtab)
> +{
> +  int i;
> +  int r = 0;
> +  const char *name = strtab + grub_host_to_target32 (s->sh_name);
> +
> +  if (!strncmp (name, ".rela.", 6))
> +    name += 5;
> +  else if (!strncmp (name, ".rel.", 5))
> +    name += 4;
> +  else
> +    return 1;
> +
> +  for (i = 0, s = sections; i < num_sections;
> +       i++, s = (Elf_Shdr *) ((char *) s + section_entsize))
> +    {
> +      const char *sname = strtab + grub_host_to_target32 (s->sh_name);
> +      if (strcmp (sname, name))
> +       continue;
> +
> +      return SUFFIX (is_kept_section) (s, image_target);
> +    }
> +
> +  return r;
> +}
> +
>  /* Return if the ELF header is valid.  */
>  static int
>  SUFFIX (check_elf_header) (Elf_Ehdr *e, size_t size, const struct
> grub_install_image_target_desc *image_target)
> @@ -2087,14 +2159,7 @@ SUFFIX (grub_mkimage_load_image) (const char
> *kernel_path,
>    for (i = 0, s = sections;
>         i < num_sections;
>         i++, s = (Elf_Shdr *) ((char *) s + section_entsize))
> -    if (SUFFIX (is_data_section) (s, image_target)
> -       /* Explicitly initialize BSS
> -          when producing PE32 to avoid a bug in EFI implementations.
> -          Platforms other than EFI and U-boot shouldn't have .bss in
> -          their binaries as we build with -Wl,-Ttext.
> -       */
> -       || (SUFFIX (is_bss_section) (s, image_target) && (image_target->id
> == IMAGE_EFI || !is_relocatable (image_target)))
> -       || SUFFIX (is_text_section) (s, image_target))
> +    if (SUFFIX (is_kept_section) (s, image_target))
>        {
>         if (grub_target_to_host32 (s->sh_type) == SHT_NOBITS)
>           memset (out_img + section_addresses[i], 0,
> --
> 2.15.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to