PING: [PATCH] aarch64: Disallow copy relocations on protected data

[Fangrui Song <i@maskray.me>]

On Fri, May 27, 2022 at 3:43 PM Fangrui Song <i@maskray.me> wrote:
>
> On Mon, May 23, 2022 at 10:10 PM Fangrui Song <i@maskray.me> wrote:
> >
> > The behavior matches ld.lld which has been used on many aarch64 OSes.
> >
> > Rationale: copy relocations indicate that the executable and the shared object
> > would access different copies, if the shared object does not reference the
> > variable with GLOB_DAT.
> >
> > 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.
>
> The diagnostic is requested on
> https://sourceware.org/pipermail/libc-alpha/2022-May/139014.html
>
> > ---
> >  bfd/elfnn-aarch64.c                           | 28 ++++++++++++++++++-
> >  ld/testsuite/ld-aarch64/aarch64-elf.exp       |  9 ++++++
> >  .../ld-aarch64/copy-reloc-protected.d         |  2 ++
> >  ld/testsuite/ld-aarch64/protected.s           |  8 ++++++
> >  4 files changed, 46 insertions(+), 1 deletion(-)
> >  create mode 100644 ld/testsuite/ld-aarch64/copy-reloc-protected.d
> >  create mode 100644 ld/testsuite/ld-aarch64/protected.s
> >
> > diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
> > index 4926bab9cf2..8896b0f78dd 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
> > --
> > 2.36
> >

Ping