Bug 246561 - [PATCH] rtld-elf: dlinfo() returns wrong address in RTLD_DI_LINKMAP's l_addr (breaking Wine, gdb, etc.)
Summary: [PATCH] rtld-elf: dlinfo() returns wrong address in RTLD_DI_LINKMAP's l_addr ...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-19 03:12 UTC by Damjan Jovanovic
Modified: 2020-06-02 15:21 UTC (History)
6 users (show)

See Also:
linimon: mfc-stable12+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Damjan Jovanovic 2020-05-19 03:12:34 UTC
Several low-level applications need access to ELF internals. For example Wine recently began calling ELF initializers for Winelibs within itself instead of having the dynamic linker do it. To do this, it calls the dlinfo() function with request RTLD_DI_LINKMAP, and searches the .dynamic section pointed to by link_map.l_ld for a custom tag with Winelib initializers.

However ELF libraries can be loaded at a different address to the one they expected (relocation), so the pointer stored in the initializer tag is not necessarily pointing to the correct address. We have to add the offset from the expected base address to the actual base address to it to determine the relocated address (in the rtld-elf code, this is the "relocbase" variable).

On most implementations of dlinfo(), the link_map.l_addr field stores this offset. However on FreeBSD, link_map.l_addr stores the absolute base address where the library was loaded instead ("mapbase"). A separate l_offs field was apparently added to return relocbase, but it only exists on MIPS.

Wine recently began crashing on startup because it was adding l_addr to the pointers in initializers, but since l_addr is mapbase instead of relocbase on FreeBSD, it was relocating them to wrong addresses.

NetBSD changed their dynamic linker to return relocbase in l_addr in 2002 (https://github.com/NetBSD/src/commit/d1351c627c5f4d5ac41a3f680243d57293e0ce1f). GNU always returned relocbase. Illumos returns relocbase. Only FreeBSD returns mapbase.

Can we please change rtld-elf to return relocbase? Either we can change l_addr itself to return relocbase like NetBSD did:

diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c
index 2d15560fb908..647626e3a41d 100644
--- a/libexec/rtld-elf/rtld.c
+++ b/libexec/rtld-elf/rtld.c
@@ -3951,7 +3951,7 @@ linkmap_add(Obj_Entry *obj)
     struct link_map *prev;
 
     obj->linkmap.l_name = obj->path;
-    obj->linkmap.l_addr = obj->mapbase;
+    obj->linkmap.l_addr = obj->relocbase;
     obj->linkmap.l_ld = obj->dynamic;
 #ifdef __mips__
     /* GDB needs load offset on MIPS to use the symbols */



or we can remove that "#ifdef __mips__" further down and return l_offs on all platforms, and patch applications to use that instead (like was apparently already done for GDB on MIPS). (Of course, being an ABI-breaking change, it can only be done in CURRENT.)

I eventually found a way to work out relocbase from mapbase within Wine (https://source.winehq.org/git/wine.git/blobdiff/0f31e48e9ba28ba8c0aee0678567d6b47e3c63aa..0fd3f0266e05f6afa710fa2b5a254b0ed88bac0f:/dlls/ntdll/loader.c), and the latest Wine Git is no longer crashing on startup. But I am not convinced that approach will always work, because it requires the first PT_LOAD segment to start at address 0 in the file, and I don't see that being required by the ELF specification.

Detailed discussion: https://bugs.winehq.org/show_bug.cgi?id=49139
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2020-05-19 14:48:30 UTC
I do not think we can break link_map ABI by changing the meaning of l_addr.

https://reviews.freebsd.org/D24918
Comment 2 Alex S 2020-05-19 15:27:56 UTC
(In reply to Konstantin Belousov from comment #1)

It's not clear if there exists any code at all which expects mapbase in l_addr. Would you at least consider renaming l_addr to something else then? That way existing code won't compile until it's patched to use the correct field (whatever it is).
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2020-05-19 15:49:52 UTC
(In reply to Alex S from comment #2)
Aren't the gdb binaries expect it (adopted to it) ?
Comment 4 John Baldwin freebsd_committer freebsd_triage 2020-05-19 16:51:26 UTC
GDB doesn't use the special field for MIPS (it ignores it).  GDB also uses the structure embedded in the rtld itself, it does not call dlinfo().  GDB does seem to expect l_addr + offset of ".dynamic" in the binary == l_ld.

I think what we have now only works because all of our shared libraries are linked at a virtual address of 0.  If we did "pre-linking" to set preferred addresses for shared libraries GDB would stop working.

Here's stepping through the function (gdb/solib-svr4.c:lm_addr_check()) that finds the base address of a shared library (libutil.so in this case):

(top-gdb) p *li
$10 = {<lm_info_base> = {<No data fields>}, l_addr = 0x0, 
  l_addr_inferior = 0x800250000, l_addr_p = false, lm_addr = 0x800232638, 
  l_ld = 0x800263360, l_next = 0x800232a38, l_prev = 0x800232238, 
  l_name = 0x800230260}
(top-gdb) n
225           l_addr = li->l_addr_inferior;
(top-gdb) n
227           if (! abfd || ! has_lm_dynamic_from_link_map ())
(top-gdb) 
230           l_dynaddr = li->l_ld;
(top-gdb) 
232           dyninfo_sect = bfd_get_section_by_name (abfd, ".dynamic");
(top-gdb) 
233           if (dyninfo_sect == NULL)
(top-gdb) 
236           dynaddr = bfd_section_vma (abfd, dyninfo_sect);
(top-gdb) 
238           if (dynaddr + l_addr != l_dynaddr)
(top-gdb) p dynaddr
$11 = 0x13360
(top-gdb) p l_addr
$12 = 0x800250000
(top-gdb) p l_dynaddr
$13 = 0x800263360

li->l_addr_inferior is the value of 'l_addr' from the linkmap, li->l_ld is 'l_ld'.

readelf -t libutil.so:

  [20] .dynamic
       DYNAMIC          0000000000013360  0000000000013360  6
       0000000000000160 0000000000000010  0                 8
       [0000000000000003]: WRITE, ALLOC

readelf -l shows first PT_LOAD with an offset of 0:

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flg    Align
  PHDR           0x0000000000000040 0x0000000000000040 0x0000000000000040
                 0x00000000000001f8 0x00000000000001f8  R      0x8
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x00000000000075cc 0x00000000000075cc  R      0x1000
  LOAD           0x0000000000008000 0x0000000000008000 0x0000000000008000
                 0x000000000000a5d0 0x000000000000a5d0  R E    0x1000
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-05-20 22:09:21 UTC
A commit references this bug:

Author: kib
Date: Wed May 20 22:08:27 UTC 2020
New revision: 361303
URL: https://svnweb.freebsd.org/changeset/base/361303

Log:
  Change the samantic of struct link_map l_addr member.

  It previously returned the object map base address, while all other
  ELF operating systems return load offset, i.e. the difference between
  map base and the link base.

  Explain the meaning of the field in the man page.

  Stop filling the mips-only l_offs member, which is apparently unused.

  PR:	246561
  Requested by:	Damjan Jovanovic <damjan.jov@gmail.com>
  Reviewed by:	emaste, jhb, cem (previous version)
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week
  Differential revision:	https://reviews.freebsd.org/D24918

Changes:
  head/lib/libc/gen/dlinfo.3
  head/libexec/rtld-elf/rtld.c
  head/sys/sys/link_elf.h
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2020-06-02 15:21:41 UTC
MFCed in r361564.