[PATCH v2 2/2] elf: Add a zero terminator to the .eh_frame section of ld.so

[Adrian Ratiu]

ld.so is linked with -nostdlib -nostartfiles, so unlike other shared
objects it does not get the 4-byte zero terminator at the end of
.eh_frame from crtendS.o.

libc.so links elf/sofini.os last for this reason, but ld.so did not,
leaving its .eh_frame unterminated.

PT_GNU_EH_FRAME only provides the start of .eh_frame, so unwinders
which scan .eh_frame linearly rely on the terminator to find its end.
llvm-libunwind does this when a PC is not covered by the .eh_frame_hdr
search table, for example _start and _dl_start_user on x86_64 and i386,
which have no CFI (BZ #26854).

A backtrace from an ELF constructor of a SO reaches _dl_start_user
through _dl_init and the scan then continues past the end of .eh_frame.

This used to work by accident because the bytes following .eh_frame
in ld.so were zero padding. Since binutils 2.46 (commit 6f9157ad377e,
"ld/ELF: explicitly place .note.gnu.property"), GNU ld places
.note.gnu.property right after .eh_frame, and llvm-libunwind crashes
with SIGSEGV while reading it as frame records.

So we link sofini.os into ld.so after librtld.os.

sofini.os is now built as part of rtld, so that symbol-hacks.h does not
reference libc-internal __GI_* aliases, which ld.so cannot resolve. The
object only contains the terminator, so libc.so is unchanged by this.

This also shows that the reasoning in BZ #22053, which proposes removing
the terminator from libc.so because .eh_frame_hdr is always present,
does not fully hold: PT_GNU_EH_FRAME does not provide the size of
.eh_frame, so linear scans still need the terminator.

Also add tst-eh-frame-terminator-ldso and tst-eh-frame-terminator-libc
tests which check that ld.so and libc.so both end .eh_frame with a zero
terminator.

Tested on x86_64-linux-gnu with "make check subdirs=elf", and with an
aarch64-linux-gnu cross build, where both new tests now pass.
tst-eh-frame-terminator-ldso fails without the fix.

Co-authored-by: George Burgess IV <gbiv@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 elf/Makefile                   | 16 +++++++-
 elf/tst-eh-frame-terminator.py | 72 ++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-eh-frame-terminator.py

diff --git a/elf/Makefile b/elf/Makefile
index 2c74e11946..ca78e67cf8 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -711,6 +711,17 @@ $(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
 	    --required=__io_vtables \
 	  > $@ 2>&1; $(evaluate-test)
 
+tests-special += $(objpfx)tst-eh-frame-terminator-ldso.out \
+  $(objpfx)tst-eh-frame-terminator-libc.out
+$(objpfx)tst-eh-frame-terminator-ldso.out: tst-eh-frame-terminator.py \
+  $(..)/scripts/glibcelf.py $(objpfx)ld.so
+	$(PYTHON) tst-eh-frame-terminator.py $(objpfx)ld.so \
+	  > $@ 2>&1; $(evaluate-test)
+$(objpfx)tst-eh-frame-terminator-libc.out: tst-eh-frame-terminator.py \
+  $(..)/scripts/glibcelf.py $(common-objpfx)libc.so
+	$(PYTHON) tst-eh-frame-terminator.py $(common-objpfx)libc.so \
+	  > $@ 2>&1; $(evaluate-test)
+
 ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-valgrind-smoke.out
 endif
@@ -1683,7 +1694,8 @@ generated += librtld.map librtld.mk rtld-libc.a librtld.os.map
 
 z-now-yes = -Wl,-z,now
 
-$(objpfx)ld.so: $(objpfx)librtld.os $(ld-map)
+# sofini.os must be placed last since it terminates .eh_frame section.
+$(objpfx)ld.so: $(objpfx)librtld.os $(objpfx)sofini.os $(ld-map)
 # Link into a temporary file so that we don't touch $@ at all
 # if the sanity check below fails.
 	$(LINK.o) -nostdlib -nostartfiles -shared -o $@.new		\
@@ -1786,7 +1798,7 @@ CFLAGS-dl-diagnostics.c += $(SYSCONF-FLAGS) \
   -D'PREFIX="$(prefix)"' \
   -D'RTLD="$(rtlddir)/$(rtld-installed-name)"'
 
-cpp-srcs-left := $(all-rtld-routines:=.os)
+cpp-srcs-left := $(all-rtld-routines:=.os) sofini.os
 lib := rtld
 include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
 
diff --git a/elf/tst-eh-frame-terminator.py b/elf/tst-eh-frame-terminator.py
new file mode 100644
index 0000000000..f81ae45a02
--- /dev/null
+++ b/elf/tst-eh-frame-terminator.py
@@ -0,0 +1,72 @@
+#!/usr/bin/python3
+# Verify that the .eh_frame section ends with a zero terminator.
+# Copyright The GNU Toolchain Authors.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+"""Verify that the .eh_frame section of a shared object ends with a
+zero terminator.
+
+PT_GNU_EH_FRAME only gives the start of .eh_frame, so unwinders that
+scan .eh_frame linearly rely on the terminator to find its end.
+
+"""
+
+import os.path
+import sys
+
+# Make available glibc Python modules.
+sys.path.append(os.path.join(
+    os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
+
+import glibcelf
+
+def main(argv):
+    path, = argv
+    img = glibcelf.Image.readfile(path)
+    if img.ehdr.e_ident.ei_data == glibcelf.ElfData.ELFDATA2LSB:
+        byteorder = 'little'
+    else:
+        byteorder = 'big'
+
+    for shdr in img.shdrs():
+        if shdr.sh_name == b'.eh_frame':
+            break
+    else:
+        sys.stdout.write('{}: error: no .eh_frame section\n'.format(path))
+        sys.exit(1)
+
+    # Skip over the CIE and FDE records using their length fields, as
+    # a linear scan by an unwinder does, until a zero length.
+    data = img.image[shdr.sh_offset:shdr.sh_offset + shdr.sh_size]
+    offset = 0
+    while offset + 4 <= len(data):
+        length = int.from_bytes(data[offset:offset + 4], byteorder)
+        if length == 0:
+            break
+        if length == 0xffffffff:
+            # 64-bit DWARF: the actual length follows in 8 bytes.
+            length = 8 + int.from_bytes(data[offset + 4:offset + 12],
+                                        byteorder)
+        offset += 4 + length
+
+    if offset + 4 != len(data):
+        sys.stdout.write('{}: error: .eh_frame of size 0x{:x} does not end'
+                         ' with a zero terminator\n'.format(path, len(data)))
+        sys.exit(1)
+
+if __name__ == '__main__':
+    main(sys.argv[1:])
-- 
2.47.3