[binutils-gdb] aarch64: Disallow copy relocations on protected data

[binutils-gdb] aarch64: Disallow copy relocations on protected data

[Fangrui Song]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=90b7a5df152a64d2bea20beb438e8b81049a5c30

commit 90b7a5df152a64d2bea20beb438e8b81049a5c30
Author: Fangrui Song <i@maskray.me>
Date:   Wed Jun 22 10:55:12 2022 -0700

    aarch64: Disallow copy relocations on protected data
    
    If an executable has copy relocations for extern protected data, that
    can only work if the shared object containing the definition is built
    with assumptions (a) the compiler emits GOT-generating relocations (b)
    the linker produces R_*_GLOB_DAT instead of R_*_RELATIVE.  Otherwise the
    shared object uses its own definition directly and the executable
    accesses a stale copy.  Note: the GOT relocations defeat the purpose of
    protected visibility as an optimization, and it turns out this never
    worked perfectly.
    
    glibc 2.36 will warn on copy relocations on protected data.  Let's
    produce a warning at link time, matching ld.lld which has been used on
    many aarch64 OSes.
    
    Note: x86 requires GNU_PROPERTY_NO_COPY_ON_PROTECTED to have the error.
    This is to largely due to GCC 5's "x86-64: Optimize access to globals in
    PIE with copy reloc" which started to use direct access relocations for
    external data symbols in -fpie mode.
    
    GCC's aarch64 port does not have the change.  Nowadays with most builds
    switching to -fpie/-fpic, aarch64 mostly doesn't need to worry about
    copy relocations.  So for aarch64 we simply don't check
    GNU_PROPERTY_NO_COPY_ON_PROTECTED.

Diff:
---
 bfd/elfnn-aarch64.c                            | 28 +++++++++++++++++++++++++-
 ld/testsuite/ld-aarch64/aarch64-elf.exp        |  9 +++++++++
 ld/testsuite/ld-aarch64/copy-reloc-protected.d |  2 ++
 ld/testsuite/ld-aarch64/protected.s            |  8 ++++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index ca24a6b5678..81311fc5a32 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -2579,6 +2579,9 @@ struct elf_aarch64_link_hash_entry
      this symbol.  */
   unsigned int got_type;
 
+  /* TRUE if symbol is defined as a protected symbol.  */
+  unsigned int def_protected : 1;
+
   /* A pointer to the most recently used stub hash entry against this
      symbol.  */
   struct elf_aarch64_stub_hash_entry *stub_cache;
@@ -2855,9 +2858,16 @@ elfNN_aarch64_copy_indirect_symbol (struct bfd_link_info *info,
 static void
 elfNN_aarch64_merge_symbol_attribute (struct elf_link_hash_entry *h,
 				      unsigned int st_other,
-				      bool definition ATTRIBUTE_UNUSED,
+				      bool definition,
 				      bool dynamic ATTRIBUTE_UNUSED)
 {
+  if (definition)
+    {
+      struct elf_aarch64_link_hash_entry *eh
+	  = (struct elf_aarch64_link_hash_entry *)h;
+      eh->def_protected = ELF_ST_VISIBILITY (st_other) == STV_PROTECTED;
+    }
+
   unsigned int isym_sto = st_other & ~ELF_ST_VISIBILITY (-1);
   unsigned int h_sto = h->other & ~ELF_ST_VISIBILITY (-1);
 
@@ -8701,6 +8711,22 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
   if (h->dyn_relocs == NULL)
     return true;
 
+  for (p = h->dyn_relocs; p != NULL; p = p->next)
+    if (eh->def_protected)
+      {
+	/* Disallow copy relocations against protected symbol.  */
+	asection *s = p->sec->output_section;
+	if (s != NULL && (s->flags & SEC_READONLY) != 0)
+	  {
+	    info->callbacks->einfo
+		/* xgettext:c-format */
+		(_ ("%F%P: %pB: copy relocation against non-copyable "
+		    "protected symbol `%s'\n"),
+		 p->sec->owner, h->root.root.string);
+	    return false;
+	  }
+      }
+
   /* In the shared -Bsymbolic case, discard space allocated for
      dynamic pc-relative relocs against symbols which turn out to be
      defined in regular objects.  For the normal shared case, discard
diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index 64476f111e0..31162277bd9 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -407,6 +407,8 @@ set aarch64elflinktests {
     {copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
   {"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
     {copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
+  {"Build .so with protected data" "-shared" "" "" {protected.s}
+    {} "protected.so"}
   {"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
     {} "func-in-so.so"}
   {"ld-aarch64/func sym hash opt for exe"
@@ -416,8 +418,15 @@ set aarch64elflinktests {
     {} "libbti-plt-so.so"}
 }
 
+set aarch64elfcclinktests [list \
+  [list "copy relocation on protected data" \
+    "-no-pie tmpdir/copy-reloc-exe.o tmpdir/protected.so" "" \
+    {} {{error_output copy-reloc-protected.d}} "copy-reloc-protected"]
+]
+
 if [check_shared_lib_support] {
     run_ld_link_tests $aarch64elflinktests
+    run_cc_link_tests $aarch64elfcclinktests
 }
 
 run_dump_test "bti-plt-3"
diff --git a/ld/testsuite/ld-aarch64/copy-reloc-protected.d b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
new file mode 100644
index 00000000000..99a356a3df6
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
@@ -0,0 +1,2 @@
+.*: tmpdir/copy-reloc-exe.o: copy relocation against non-copyable protected symbol `global_a'
+#...
diff --git a/ld/testsuite/ld-aarch64/protected.s b/ld/testsuite/ld-aarch64/protected.s
new file mode 100644
index 00000000000..eb3fb402dc4
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/protected.s
@@ -0,0 +1,8 @@
+.global global_a
+.protected global_a
+.type global_a, %object
+.size global_a, 4
+
+.data
+global_a:
+.word 0xcafedead