Bug 214972 - rtld relies on non-zero _DYNAMIC GOT entry (on !sparc64)
Summary: rtld relies on non-zero _DYNAMIC GOT entry (on !sparc64)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Ed Maste
URL: https://reviews.freebsd.org/D9180
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-01 02:56 UTC by Ed Maste
Modified: 2019-01-21 19:24 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Maste freebsd_committer freebsd_triage 2016-12-01 02:56:14 UTC
Over a decade ago a binutils import broke sparc64 rtld, and it was fixed in r130661 Quoting from that commit message:

    The rtld code tested &_DYNAMIC against 0 to see whether rtld itself
    was built as PIC or not. While the sparc64 MD code did not rely
    on the preset value of the GOT slot for _DYNAMIC any more due
    to previous binutils changes, it still used to not be 0, so
    that this check did work. The new binutils do however initialize
    this slot with 0. As a consequence, rtld would not properly initialize
    itself and crash.

    Fix that by introducing a new macro, RTLD_IS_DYNAMIC, to take the role
    of this test. For sparc64, it is implemented using the rtld_dynamic()
    code that was already there. If an architecture does not provide its
    own implementation, we default to the old check.

I have been testing LLD as a linker for FreeBSD, and a recent LLD change (their r288107) started producing a zeroed GOT on amd64. See http://llvm.org/pr31221.

I don't believe the linker is required to populate the GOT with nonzero values, so we should probably make a change for all architectures akin to r130661 for sparc64 .
Comment 1 Ed Maste freebsd_committer freebsd_triage 2016-12-01 03:58:20 UTC
Corresponding LLD bug report: https://llvm.org/bugs/show_bug.cgi?id=31221
Comment 2 Ed Maste freebsd_committer freebsd_triage 2016-12-01 20:48:30 UTC
One related change for review: https://reviews.freebsd.org/D8687
Comment 3 commit-hook freebsd_committer freebsd_triage 2016-12-02 14:24:02 UTC
A commit references this bug:

Author: emaste
Date: Fri Dec  2 14:23:27 UTC 2016
New revision: 309411
URL: https://svnweb.freebsd.org/changeset/base/309411

Log:
  Retire long-broken/unused static rtld support

  rtld-elf has some vestigial support for building as a static executable.
  r45501 introduced a partial implementation with a prescient note that it
  "might never be enabled." r153515 introduced ELF symbol versioning
  support, and removed part of the unused build infrastructure for static
  rtld.

  GNU ld populates rela relocation addends and GOT entries with the same
  values, and rtld's run-time dynamic executable check relied on this.
  Alternate toolchains may not populate the GOT entries, which caused
  RTLD_IS_DYNAMIC to return false. Simplify rtld by just removing the
  unused check.

  If we want to restore static rtld support later on we ought to introduce
  a build-time #ifdef flag.

  PR:		214972
  Reviewed by:	kan
  MFC after:	1 month
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D8687

Changes:
  head/libexec/rtld-elf/aarch64/rtld_machdep.h
  head/libexec/rtld-elf/riscv/rtld_machdep.h
  head/libexec/rtld-elf/rtld.c
  head/libexec/rtld-elf/sparc64/rtld_machdep.h
Comment 4 Ed Maste freebsd_committer freebsd_triage 2016-12-02 15:47:23 UTC
For reference, musl's approach:
	lea _DYNAMIC(%rip),%rsi
https://git.musl-libc.org/cgit/musl/tree/arch/x86_64/crt_arch.h
Comment 5 commit-hook freebsd_committer freebsd_triage 2017-01-03 16:00:35 UTC
A commit references this bug:

Author: emaste
Date: Tue Jan  3 15:59:40 UTC 2017
New revision: 311156
URL: https://svnweb.freebsd.org/changeset/base/311156

Log:
  MFC r309411: Retire long-broken/unused static rtld support

  rtld-elf has some vestigial support for building as a static executable.
  r45501 introduced a partial implementation with a prescient note that it
  "might never be enabled." r153515 introduced ELF symbol versioning
  support, and removed part of the unused build infrastructure for static
  rtld.

  GNU ld populates rela relocation addends and GOT entries with the same
  values, and rtld's run-time dynamic executable check relied on this.
  Alternate toolchains may not populate the GOT entries, which caused
  RTLD_IS_DYNAMIC to return false. Simplify rtld by just removing the
  unused check.

  If we want to restore static rtld support later on we ought to introduce
  a build-time #ifdef flag.

  PR:		214972

Changes:
_U  stable/11/
  stable/11/libexec/rtld-elf/aarch64/rtld_machdep.h
  stable/11/libexec/rtld-elf/riscv/rtld_machdep.h
  stable/11/libexec/rtld-elf/rtld.c
  stable/11/libexec/rtld-elf/sparc64/rtld_machdep.h
Comment 6 Ed Maste freebsd_committer freebsd_triage 2017-01-15 00:46:17 UTC
amd64 fix in https://reviews.freebsd.org/D9180
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-01-16 14:50:34 UTC
A commit references this bug:

Author: emaste
Date: Mon Jan 16 14:49:29 UTC 2017
New revision: 312288
URL: https://svnweb.freebsd.org/changeset/base/312288

Log:
  rtld: do not rely on a populated GOT on amd64

  On rela architectures GNU BFD ld and gold store the relocation addend
  in GOT entries (in addition to the relocation's r_addend field).
  rtld previously relied on this to access its own _DYNAMIC symbol in
  order to apply its own relocations.

  However, recording addends in the GOT is not specified by the ABI,
  and some versions of LLVM's LLD linker leave the GOT uninitialized on
  rela architectures.

  BFD ld does not populate the GOT on sparc64, and sparc64 rtld has a
  machine-dependent rtld_dynamic_addr() function that returns the
  _DYNAMIC address. Use the same approach on amd64, obtaining the %rip-
  relative _DYNAMIC address following a suggestion from Rafael Esp?ndola.

  Architectures other than amd64 should be addressed in future work.

  PR:		214972
  Reviewed by:	kib
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D9180

Changes:
  head/libexec/rtld-elf/amd64/rtld_machdep.h
  head/libexec/rtld-elf/amd64/rtld_start.S
Comment 8 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:45:07 UTC
batch change:

For bugs that match the following
-  Status Is In progress 
AND
- Untouched since 2018-01-01.
AND
- Affects Base System OR Documentation

DO:

Reset to open status.


Note:
I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
Comment 9 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-21 19:24:00 UTC
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved.

Thanks