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 <pjo...@redhat.com>
One nit pick below... Otherwise Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> 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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel