From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by sourceware.org (Postfix) with ESMTPS id D6C263858288 for ; Wed, 15 Jun 2022 22:28:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D6C263858288 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=maskray.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-f41.google.com with SMTP id a10so12421174pju.3 for ; Wed, 15 Jun 2022 15:28:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=dAqFwK00bC8Zj0lsvI9DSAndlgWQtfWwDlvMO28tjHs=; b=E+sIelrMjS7dklF1+4l6++tbJHjWQGo8eVEDwnOo4thUwF9LHZ81NNSEmrVBgsVbdh pqhmQT+cQdI1UMEdx62tM/y8wW/IQwlSgyr6Ie9kpq3cDH1fudwjU1WwWmTdOvYfK5C7 bnoFu/Bi+GyjKPihAdaC+y/CKY0NlBM7CvbpEpofrXIwH8vguk0A0K2YKQiROYj0JZ0/ s24sRoY9mKom3xBrY1JOaV2XW58KZSYmgIZdXghClC7xgH0B5b2k7t/QNCHhphHZHags jkFJhn5+or4vfl48xso3IeSrlLDXSJaOIPky/l7MQofpgzatFml1swM/7FtDQjlv5TzZ Zr5w== X-Gm-Message-State: AJIora/etKNqGjwT4NWv9FMB3f4TnXuVsnmCAMTPeP33LoweTxXPwQMv xaJjLLPPpuvJU1sYB7FG52InrDcWsC0= X-Google-Smtp-Source: AGRyM1vmUKDQm69GWR4SZNEG/jQQmQKzwwoNJL/oHLFAQDfpFfi9Ly/4JQnAvuud2zN5SCg/CjMbnA== X-Received: by 2002:a17:902:c403:b0:167:4a9f:2785 with SMTP id k3-20020a170902c40300b001674a9f2785mr1458639plk.67.1655332084318; Wed, 15 Jun 2022 15:28:04 -0700 (PDT) Received: from localhost ([2620:15c:2d1:206:6ece:a7fa:a827:39]) by smtp.gmail.com with ESMTPSA id x27-20020aa78f1b000000b0051c78a6fb9csm135661pfr.111.2022.06.15.15.28.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Jun 2022 15:28:03 -0700 (PDT) Date: Wed, 15 Jun 2022 15:28:03 -0700 From: Fangrui Song To: binutils@sourceware.org, Adhemerval Zanella , Szabolcs Nagy , Nick Clifton Subject: Re: PING: [PATCH] aarch64: Disallow copy relocations on protected data Message-ID: <20220615222803.aj66irsqhopnbsd7@gmail.com> References: <20220524051017.2244288-1-i@maskray.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-8.9 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_INFOUSMEBIZ, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jun 2022 22:28:08 -0000 On 2022-05-31, Fangrui Song wrote: >On Fri, May 27, 2022 at 3:43 PM Fangrui Song wrote: >> >> On Mon, May 23, 2022 at 10:10 PM Fangrui Song 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 >> > glibc now has a diagnostic for copy relocations on a protected symbol https://sourceware.org/git/?p=glibc.git;a=commit;h=7374c02b683b7110b853a32496a619410364d70b :) Let's catch the issue on ld side, too. Ping