Re: [PATCH] Don't set ET_EXEC for -pie -Ttext-segment= (PR31795)

["H.J. Lu" <hjl.tools@gmail.com>]

On Mon, May 27, 2024 at 9:28 PM Fangrui Song <maskray@google.com> wrote:
>
> From: Fangrui Song <maskray@gcc.gnu.org>
>
> Commit 58e7ebacdd97c858834c07c7dce098aeacd500fb added the special case
> that sets ET_EXEC for -pie -Ttext-segment=$non_zero.  This was to
> support a way[1] to ensure that the address of the first byte is >=
> -Ttext-segment=.
>
>     % cat a.c
>     #include <stdio.h>
>     int main() { printf("%p\n", main); }
>     % gcc -pie a.c -fuse-ld=bfd -Wl,--no-relax,-Ttext-segment=0x600000000000 -o a
>     % ./a
>     0x600000001139
>     % ./a
>     0x600000001139  # no ASLR
>
> While changing ET_DYN to ET_EXEC fulfills the address requirement, it
> disables ASLR, which may be undesired. (While Linux kernel/glibc do not
> provide the address>=p_vaddr guarantee, this is technically feasible to
> support.)
>
> Since commit e8f6c2a5bab10b039a12b69a30a8248c91161e11, --no-pie has been
> available and we can do the following to use crtbeginS.o and ET_EXEC.
> (Using crtbegin.o could lead to R_X86_64_32/R_X86_64_32S relocation
> issues with a high address.)
>
>     % gcc -pie -Wl,-no-pie a.c -fuse-ld=bfd -Wl,--no-relax,-Ttext-segment=0x600000000000 -o a
>     % ./a
>     0x600000001139
>     % ./a
>     0x600000001139  # no ASLR
>
> [1]: https://sourceware.org/pipermail/binutils/2013-December/083381.html
> ---
>  bfd/elf.c                     | 26 +++-----------------------
>  ld/testsuite/ld-pie/vaddr-1.d |  2 +-
>  2 files changed, 4 insertions(+), 24 deletions(-)
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index 74236a658fd..a8a9be53c17 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -7150,35 +7150,15 @@ _bfd_elf_init_file_header (bfd *abfd,
>    return true;
>  }
>
> -/* Set e_type in ELF header to ET_EXEC for -pie -Ttext-segment=.
> -
> -   FIXME: We used to have code here to sort the PT_LOAD segments into
> +/* FIXME: We used to have code here to sort the PT_LOAD segments into
>     ascending order, as per the ELF spec.  But this breaks some programs,
>     including the Linux kernel.  But really either the spec should be
>     changed or the programs updated.  */
>
>  bool
> -_bfd_elf_modify_headers (bfd *obfd, struct bfd_link_info *link_info)
> +_bfd_elf_modify_headers (bfd *obfd ATTRIBUTE_UNUSED,
> +                        struct bfd_link_info *link_info ATTRIBUTE_UNUSED)
>  {
> -  if (link_info != NULL && bfd_link_pie (link_info))
> -    {
> -      Elf_Internal_Ehdr *i_ehdrp = elf_elfheader (obfd);
> -      unsigned int num_segments = i_ehdrp->e_phnum;
> -      struct elf_obj_tdata *tdata = elf_tdata (obfd);
> -      Elf_Internal_Phdr *segment = tdata->phdr;
> -      Elf_Internal_Phdr *end_segment = &segment[num_segments];
> -
> -      /* Find the lowest p_vaddr in PT_LOAD segments.  */
> -      bfd_vma p_vaddr = (bfd_vma) -1;
> -      for (; segment < end_segment; segment++)
> -       if (segment->p_type == PT_LOAD && p_vaddr > segment->p_vaddr)
> -         p_vaddr = segment->p_vaddr;
> -
> -      /* Set e_type to ET_EXEC if the lowest p_vaddr in PT_LOAD
> -        segments is non-zero.  */
> -      if (p_vaddr)
> -       i_ehdrp->e_type = ET_EXEC;

This is required to create a small model executable guaranteed to be
loaded at an address specified with -Wl,-Ttext-segment=0x100000000
on x86-64.   This change will break such programs.

> -    }
>    return true;
>  }
>
> diff --git a/ld/testsuite/ld-pie/vaddr-1.d b/ld/testsuite/ld-pie/vaddr-1.d
> index 7b5f9922718..98eff841193 100644
> --- a/ld/testsuite/ld-pie/vaddr-1.d
> +++ b/ld/testsuite/ld-pie/vaddr-1.d
> @@ -5,5 +5,5 @@
>
>  ELF Header:
>  #...
> -  Type:                              EXEC \(Executable file\)
> +  Type:                              DYN \(Position-Independent Executable file\)
>  #pass
> --
> 2.45.1.288.g0e0cd299f1-goog
>


-- 
H.J.