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

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

[Fangrui Song]

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;
-    }
   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

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

[H.J. Lu]

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.