From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62b.google.com (mail-pl1-x62b.google.com [IPv6:2607:f8b0:4864:20::62b]) by sourceware.org (Postfix) with ESMTPS id 2E8633954C76 for ; Fri, 13 May 2022 22:19:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2E8633954C76 Received: by mail-pl1-x62b.google.com with SMTP id i1so9219202plg.7 for ; Fri, 13 May 2022 15:19:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=D8ml56i8+ym0iD2cCBw4fBIo7F8RBwOXu1atGtjFiis=; b=QLyfmuB5/X08fIsYolfh+DAx6/uujtRPS54DZDTEDQKn8s57EBMYyyUUrlf8lsAKge 6odIACOh7gsqVukZXLrDhFI1XYR4NxZbaqOb/XBGnVj+1YQ2Zg7kF4wtRQuVirLsKnrI 3tseK2MAoJtmxNEPWvayYkjfvDtWHfrjVeHXuxVYeGm2Aajs+5atjQXCFdJ2OSpQPrkI V1Sy2AZGvPyAMEsy2Gko1MyaPP2fyMQT5yo1xGIH7KhMeoFbTgPLC020vLlRy1SGJRwg D0tyEvSAnsTqKuceqvOlcvR+sWtSgRinm377X0ZNQiuQ7ZnYHW21mT5N41M/mZHqhfg+ cqrw== X-Gm-Message-State: AOAM532QeHcEenfpD09CGXIHvxIoVAw+0UhHz1C5Q4Dkp5pBkX9nm2V4 SuwDBo2bFFgCwH6Ph9rzEBgT1Mu/6ea03cvITRo= X-Google-Smtp-Source: ABdhPJxjKBdRUngTDFslUd1MsNnAG2z1t5WWClyp7w8C+IJM2Mkv/+WjEORQph9qIP0aEFef792Xa045K4Qz2VSjRhw= X-Received: by 2002:a17:902:bcc6:b0:15f:4990:baec with SMTP id o6-20020a170902bcc600b0015f4990baecmr6413233pls.102.1652480362163; Fri, 13 May 2022 15:19:22 -0700 (PDT) MIME-Version: 1.0 References: <20220501060615.1694317-1-maskray@google.com> <20220510074231.wteewlruwa2uuv3r@google.com> <20220512045623.gpub7ojtmilf6b6v@google.com> In-Reply-To: <20220512045623.gpub7ojtmilf6b6v@google.com> From: "H.J. Lu" Date: Fri, 13 May 2022 15:18:46 -0700 Message-ID: Subject: Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling To: Fangrui Song Cc: GNU C Library , Adhemerval Zanella , Szabolcs Nagy Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3025.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 May 2022 22:19:25 -0000 On Wed, May 11, 2022 at 9:56 PM Fangrui Song wrote: > > On 2022-05-10, H.J. Lu wrote: > >On Tue, May 10, 2022 at 12:42 AM Fangrui Song wrote: > >> > >> On 2022-05-09, H.J. Lu wrote: > >> >On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha > >> > wrote: > >> >> > >> >> Say both a.so and b.so define protected var and the executable copy > >> >> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange > >> >> semantics: a.so accesses the copy in the executable while b.so accesses > >> >> its own. This behavior requires that (a) the compiler emits > >> >> GOT-generating relocations (b) the linker produces GLOB_DAT instead of > >> >> RELATIVE. > >> >> > >> >> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT > >> >> will bind to the executable (normal behavior). > >> >> > >> >> For aarch64/arm it makes sense to restore the original behavior and don't > >> >> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very > >> >> unlikely used by anyone. > >> >> > >> >> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN: > >> >> no GOT-generating relocation in the first place. > >> >> * gold and lld reject copy relocation on a STV_PROTECTED symbol. > >> >> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses > >> >> GOT-generating relocation when accessing an default visibility > >> >> external symbol which avoids copy relocation. > >> >> > >> >> Fangrui Song (3): > >> >> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for > >> >> non-DL_EXTERN_PROTECTED_DATA ports > >> >> Revert "[AArch64][BZ #17711] Fix extern protected data handling" > >> >> Revert "[ARM][BZ #17711] Fix extern protected data handling" > >> >> > >> >> elf/dl-lookup.c | 46 ++++++++++++------------------------ > >> >> sysdeps/aarch64/dl-machine.h | 13 +++++----- > >> >> sysdeps/aarch64/dl-sysdep.h | 2 -- > >> >> sysdeps/arm/dl-machine.h | 10 +++----- > >> >> sysdeps/arm/dl-sysdep.h | 2 -- > >> >> 5 files changed, 24 insertions(+), 49 deletions(-) > >> >> > >> >> -- > >> >> 2.36.0.464.gb9c8b46e94-goog > >> >> > >> > > >> >Given that protected symbols never work properly with copy relocation, > >> >can we change protected symbol handling to simply warn copy relocation > >> >against protected data symbol and non-zero symbol values of undefined > >> >symbols against protected function symbols? > >> > >> "protected symbols never work properly with copy relocation" generally > >> applies to all non-x86 architectures (arm/aarch64 has some guarantee, > >> but not reliable). My goal is indeed to remove relevant code for all > >> non-x86 architectures. They should not even bother testing > >> STV_PROTECTED. > > > >Even on x86, protected symbols don't work as intended. They don't > >provide any performance benefits. I think we should move away from > >copy relocation on protected symbols. We can either issue a warning > >or an error. > > > >> On x86 there is some guarantee. The gcc>=5 -fpie > >> HAVE_LD_PIE_COPYRELOC behavior makes the problem trigger in some cases. > >> Having a warning in x86 specific files (e.g. > >> sysdeps/x86_64/dl-machine.h R_X86_64_COPY) seems appropriate. > > How about adding a warning for R_X86_64_COPY like the following, then removing > all code paths of (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA!=0)? > > From 3f905ff283178396840861a965acf35bd21cf5f0 Mon Sep 17 00:00:00 2001 > From: Fangrui Song > Date: Wed, 11 May 2022 21:54:02 -0700 > Subject: [PATCH] x86: Warn for copy relocations on a protected symbol > > --- > sysdeps/x86_64/dl-machine.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h > index c70af7ab1e..48f08ecc0b 100644 > --- a/sysdeps/x86_64/dl-machine.h > +++ b/sysdeps/x86_64/dl-machine.h > @@ -474,6 +474,12 @@ and creates an unsatisfiable circular dependency.\n", > break; > memcpy (reloc_addr_arg, (void *) value, > MIN (sym->st_size, refsym->st_size)); > + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED) > + { > + fmt = "\ > +%s: Protected symbol `%s' is incompatible with copy relocations\n"; > + goto print_err; > + } > if (__glibc_unlikely (sym->st_size > refsym->st_size) > || (__glibc_unlikely (sym->st_size < refsym->st_size) > && GLRO(dl_verbose))) Should it be handled in sysdeps/generic/dl-protected.h? BTW, I'd like to see a migration plan to remove copy relocation and use protected visibility in shared libraries. Qt uses protected/default visibilities to mark exported and imported symbols with GCC 12: https://github.com/qt/qtbase/commit/19b7f854a274812d9c95fc7aaf134a12530c105f Should glibc provide some EXPORT and IMPORT macros to help this? -- H.J.