From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by sourceware.org (Postfix) with ESMTPS id 8DBA03943406 for ; Tue, 7 Jun 2022 17:50:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8DBA03943406 Received: by mail-pj1-x102e.google.com with SMTP id gc3-20020a17090b310300b001e33092c737so16057811pjb.3 for ; Tue, 07 Jun 2022 10:50:26 -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=oXN67SOUKe30gzIDAdILs1fiF4mSM20SAL6Q6Eviibc=; b=W4xCPzSALQJrtRuL8SyuPdPq1ccA+awPYNy5OtfdOTI/52J0U9Kzko38GXRv8tJ3QR PAu866AB3jktaR/kFX/OGCbYJbluKOE1yXNpXaGTlmJyi3kSvcraT2Ztaa28PsFcKH5c w5dZgCSlFPv871iUe/8BndBdc9wTYQpjVnxNT4bapey8Yer/pppd6oUZip4TPqAG7fGP oW3YnOJcN7sXXaJNuUiLffe3I3hO5f5IiyVuVqYt23SXwUi61/FzHCb6jbsZO/rNnGnI dfA3jWtXF3z0qt8+/gsDeFF/i2y8s016jrqH5Fh+VTaBzshtgQRkzF0IOFTWSgjW7dCg +9UA== X-Gm-Message-State: AOAM533PW7dyJ3n+ZbbrNH3gzFZLgr3PySf9RId0NWZMgfQKYAUCUCiI 3H8CY/zv8+NsvaotqqGh2mM8bIzVOwxjvWTumP4TQvWyNkc= X-Google-Smtp-Source: ABdhPJzcGp2+rAKyCNeWxUb3UmnHe7PPPL1HHW4DbeCRs+9UVo4joNkcUoerDEPOjVzi7IHnolt5H1FWYMC0Q+ugc6c= X-Received: by 2002:a17:90b:2c0b:b0:1e3:25d3:e770 with SMTP id rv11-20020a17090b2c0b00b001e325d3e770mr46441157pjb.101.1654624225319; Tue, 07 Jun 2022 10:50:25 -0700 (PDT) MIME-Version: 1.0 References: <20220601175633.2407189-1-maskray@google.com> In-Reply-To: From: "H.J. Lu" Date: Tue, 7 Jun 2022 10:49:49 -0700 Message-ID: Subject: Re: [PATCH v3] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA To: Szabolcs Nagy Cc: Fangrui Song , GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3025.5 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: Tue, 07 Jun 2022 17:50:28 -0000 On Tue, Jun 7, 2022 at 6:25 AM Szabolcs Nagy via Libc-alpha wrote: > > The 06/01/2022 10:56, Fangrui Song wrote: > > If an executable has copy relocations for extern protected data, that > > can only work if the library 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 > > library 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, but allow rtld to make the executable and > > library use the same copy when copy relocations are present, but it > > turns out this never worked perfectly. > > > > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange semantics when both > > a.so and b.so define protected var and the executable copy relocates > > var: b.so accesses its own copy even with GLOB_DAT. The behavior change > > is from commit 62da1e3b00b51383ffa7efc89d8addda0502e107 (x86) and then > > copied to nios2 (ae5eae7cfc9c4a8297ff82ec6b794faca1976ecc) and arc > > (0e7d930c4c11de896fe807f67fa1eb756c9c1e05). > > > > Without ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, b.so accesses the copy > > relocated data like a.so. > > > > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has another effect in the absence > > of copy relocations: when a protected data symbol is defined in multiple > > objects, the code tries to bind the relocation locally. Without > > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, STV_PROTECTED is handled in the > > same way as STV_DEFAULT: if ld produces GLOB_DAT (some ports of GNU ld), > > the relocation will bind to the first definition; otherwise (e.g. > > ld.lld) ld does the binding locally and ld.so doesn't help. > > > > i think we should not change the interposition semantics. > we should go back to the old behaviour where only copy > relocs were broken (and there was an expensive workaround > to deal with protected symbol interposition). > > i think you want to revert the elf/dl-lookup.c changes of > > commit 62da1e3b00b51383ffa7efc89d8addda0502e107 > Author: H.J. Lu > CommitDate: 2015-03-31 05:16:57 -0700 > > Add ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA to x86 > I am OK to remove support of copy relocation against protected symbols since it doesn't work properly. My only question is if ld.so should issue a warning or an error when seeing a copy relocation against a protected symbol. Copy relocation against protected symbol defeats the purpose of protected symbol. > > It's extremely unlikely anyone relies on the > > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA behavior, so let's remove it: this > > removes a check in the symbol lookup code. > > > > -- > > Changes from v1: > > * Reword commit message as suggested by Szabolcs Nagy > > > > Changes from v2: > > * Explain interposition behavior > > --- > > elf/dl-lookup.c | 90 ------------------------------------- > > sysdeps/arc/dl-sysdep.h | 21 --------- > > sysdeps/generic/ldsodefs.h | 12 +---- > > sysdeps/i386/dl-machine.h | 3 +- > > sysdeps/nios2/dl-sysdep.h | 21 --------- > > sysdeps/x86/dl-lookupcfg.h | 4 -- > > sysdeps/x86_64/dl-machine.h | 8 +--- > > 7 files changed, 4 insertions(+), 155 deletions(-) > > delete mode 100644 sysdeps/arc/dl-sysdep.h > > delete mode 100644 sysdeps/nios2/dl-sysdep.h > > > > diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > > index a42f6d5390..41d108e0b8 100644 > > --- a/elf/dl-lookup.c > > +++ b/elf/dl-lookup.c > ... > > @@ -854,43 +801,6 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map, > > return 0; > > } > > > > - int protected = (*ref > > - && ELFW(ST_VISIBILITY) ((*ref)->st_other) == STV_PROTECTED); > > - if (__glibc_unlikely (protected != 0)) > > - { > > - /* It is very tricky. We need to figure out what value to > > - return for the protected symbol. */ > > - if (type_class == ELF_RTYPE_CLASS_PLT) > > - { > > - if (current_value.s != NULL && current_value.m != undef_map) > > - { > > - current_value.s = *ref; > > - current_value.m = undef_map; > > - } > > - } > > - else > > - { > > - struct sym_val protected_value = { NULL, NULL }; > > - > > - for (scope = symbol_scope; *scope != NULL; i = 0, ++scope) > > - if (do_lookup_x (undef_name, new_hash, &old_hash, *ref, > > - &protected_value, *scope, i, version, flags, > > - skip_map, > > - (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA > > - && ELFW(ST_TYPE) ((*ref)->st_info) == STT_OBJECT > > - && type_class == ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA) > > - ? ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA > > - : ELF_RTYPE_CLASS_PLT, NULL) != 0) > > - break; > > - > > - if (protected_value.s != NULL && protected_value.m != undef_map) > > - { > > - current_value.s = *ref; > > - current_value.m = undef_map; > > - } > > - } > > - } > > - > > i think we should keep this part without the > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA bit. -- H.J.