From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by sourceware.org (Postfix) with ESMTPS id 52C12395C04A for ; Fri, 13 May 2022 23:09:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 52C12395C04A Received: by mail-pj1-x1034.google.com with SMTP id gj17-20020a17090b109100b001d8b390f77bso12087706pjb.1 for ; Fri, 13 May 2022 16:09:48 -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:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=fke5nei8aOjtd5gJS/fs8q6MO8lPX+lVTCvHOM9K0O0=; b=u6bpXqf0sc/8BrVW9XmMe+f1MbFQkRnaU7jhdY+MZcngmWdvvlF/O/Agm041U7vr4a JpsPHyLGNMMl/IvoxqQoOBQtpqi+CbWxeejeAqUDik3bIAwxreP69LAZBW0xJ1C1BfD0 i/MtySNcxoU7LglIADvytNQNozwUQm4yKnmkOgOeB5Z754KERbK/mzZ0lJi/aiwiZkmo hkcz7R88VMgiahQ4aUzRZhHbfzyVv8ULGWF+1yaDOuy2XhPtZxdLAWBjJTujDeEiZkD7 CWWMRroW55VyFNEMsv4NvP7zVPAEkijB1Rg4eWPtUZaHiH5xQaz/KuKzQ4JaaJSXdCNT Q4sw== X-Gm-Message-State: AOAM532SXIEfdl0ylsw8TU9pbvZM3HBm7LdQSNmWrpSgMKM4VZd9yDuB wvoZILuomYXLPPNYKbw9tJ/0kQ== X-Google-Smtp-Source: ABdhPJzTqF+v7LyMeTBb5IBDkN5VBPv6coPqbclTchA5xysXlXK5dKksFyLfYvqdaFU9srBXLm2Fdw== X-Received: by 2002:a17:90a:a82:b0:1da:3763:5cf5 with SMTP id 2-20020a17090a0a8200b001da37635cf5mr7228186pjw.55.1652483387127; Fri, 13 May 2022 16:09:47 -0700 (PDT) Received: from google.com ([2620:15c:2ce:200:5263:b10a:bcd0:4831]) by smtp.gmail.com with ESMTPSA id t1-20020a17090aae0100b001d97fc5a544sm2129813pjq.2.2022.05.13.16.09.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 May 2022 16:09:46 -0700 (PDT) Date: Fri, 13 May 2022 16:09:42 -0700 From: Fangrui Song To: "H.J. Lu" Cc: GNU C Library , Adhemerval Zanella , Szabolcs Nagy Subject: Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling Message-ID: <20220513230942.xg63k2oxb2y3zyoj@google.com> References: <20220501060615.1694317-1-maskray@google.com> <20220510074231.wteewlruwa2uuv3r@google.com> <20220512045623.gpub7ojtmilf6b6v@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-27.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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 23:09:50 -0000 On 2022-05-13, H.J. Lu wrote: >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? Yes, look like the suitable place. I think we should report a warning regardless of GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS, at least for all non-x86 architectures. For x86, gating the logic under GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS may be fine but I think it may be a bit overengineering as well. >BTW, I'd like to see a migration plan to remove copy relocation and >use protected >visibility in shared libraries. As mentioned, the elf/dl-lookup.c code is to handle the case when a protected variable is defined in multiple shared objects and the executable copy relocates it. This is a pathologic case and does not have clear semantics. So IMO we should just remove the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code that let the second shared object access its own copy. I sent the patch series keeping ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, because I did not know whether you want to keep it for x86 :) Note: the single protected data copy relocated by the executable does not need any ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code to work. It simply depends on whether the compiler and the linker cooperate and produce a GLOB_DAT dynamic relocation. >Qt uses protected/default >visibilities to mark exported >and imported symbols with GCC 12: > >https://github.com/qt/qtbase/commit/19b7f854a274812d9c95fc7aaf134a12530c105f The Qt example belongs to the majority of cases where there is one single definition for a protected variable. Removing the dl-lookup.c STV_PROTECTED code won't affect it. If Qt ever defines the protected variable in two shared objects, we can see that it is in a very broken state. And the original issue was really related to GCC 5's x86-64 decision to use HAVE_LD_PIE_COPYRELOC. Other architectures don't have the problem. In recent years most (if not all) distributions have switched to default PIE. On all non-x86 architecture supporting shared objects, -fPIE codegen uses an indirection for an external data symbol. On the GCC side, we should just fix x86-64 -fpie by removing HAVE_LD_PIE_COPYRELOC (https://gcc.gnu.org/pipermail/gcc-patches/2021-November/582987.html). >Should glibc provide some EXPORT and IMPORT macros to help this? The -fno-pic codegen incompatibility with -Bsymbolic/protected/some --dynamic-list has been there since like forever for architectures with copy relocations. It doesn't help for glibc to tell GCC what to do. Programs specifying -fno-pic likely rely on the existing behavior. Changing GCC -fno-pic to default indirect access may be fine. That will just let them opt out as they currently do for some security hardening options pushed by Debian/Gentoo/RedHat/etc (e.g. -fno-stack-protector)