From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id AFD26398EC05 for ; Fri, 9 Apr 2021 14:55:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AFD26398EC05 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-283--sUDxOjTPgS1CPNeEJEBsA-1; Fri, 09 Apr 2021 10:55:18 -0400 X-MC-Unique: -sUDxOjTPgS1CPNeEJEBsA-1 Received: by mail-qk1-f197.google.com with SMTP id k68so3509062qke.2 for ; Fri, 09 Apr 2021 07:55:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=f9y/5erMX8en5KMOMK4vYoBXzEPp/jTRFbOTXi6wqdM=; b=PLB8NIzk1xBKPVL+9BziVE3Nxsd3R72RDSLXHqir4CnUNysS2uoKvSi9Lc/97P4TJD 5fyU+gV84Cc9axKaFCwr1ZeE7fCgp8Mbbx5JuP+SjicVWK6fWx0mFpW8GCx+LJLjE4pu ERdFnYJ+ThKoe8PIq1J3WN+U/SqUW41jlygXdRj4K1dYlNFCsv4oUnVm9HT7fWyrilML dIqCKBDD92cTHow2SznutR23XDzpu0MZG3zveNqcWubQL90dc7H7EAUqhTGseeiSsVyA Y0MA4o/W3T+AnK1rb5wYHssA4ErmArRcvZLGo0K/0o2T9S8z2UQ8aUDWu2qZKGYXDfqu XwlQ== X-Gm-Message-State: AOAM530OeCKLzn+xtzecXj71GTYWp80UqGrMt9vOiQEb7GgBemun3pZz o62Z+J3952ekJYsJ01yDt0ThSF52/+Qs8xYc7Ob2HevwZZMO+OEPSY/zkSUZd6bjn6jg3Y7bxD2 bYfwPNTygD0rqY32yuXQE X-Received: by 2002:a05:622a:1050:: with SMTP id f16mr6785371qte.354.1617980117387; Fri, 09 Apr 2021 07:55:17 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzSVZ5Y4OqHEXh4QIYplUgLFimtB2hGoIYfFcVo+EVez9GjXM2zpXKbMoyg3n4WGiXiWG3gkw== X-Received: by 2002:a05:622a:1050:: with SMTP id f16mr6785347qte.354.1617980116930; Fri, 09 Apr 2021 07:55:16 -0700 (PDT) Received: from [192.168.1.234] (47-208-193-143.trckcmtc01.res.dyn.suddenlink.net. [47.208.193.143]) by smtp.gmail.com with ESMTPSA id j15sm1988204qtr.34.2021.04.09.07.55.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 Apr 2021 07:55:16 -0700 (PDT) From: Ben Woodard Message-Id: Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.60.0.2.21\)) Subject: Re: [PATCH 11/15] x86_64: Avoid lazy relocation of tlsdesc [BZ #27137] Date: Fri, 9 Apr 2021 07:55:13 -0700 In-Reply-To: <20210409133809.GR23289@arm.com> Cc: libc-alpha@sourceware.org To: Szabolcs Nagy References: <8c6dae9ee45f9af51ce70cb8422582096dbf7ffc.1613390045.git.szabolcs.nagy@arm.com> <454904D4-36D6-4589-85FD-67094C7F13C4@redhat.com> <20210409133809.GR23289@arm.com> X-Mailer: Apple Mail (2.3654.60.0.2.21) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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, 09 Apr 2021 14:55:23 -0000 > On Apr 9, 2021, at 6:38 AM, Szabolcs Nagy wrote: >=20 > The 04/08/2021 17:14, Ben Woodard wrote: >> Don=E2=80=99t you also need to modify elf_machine_runtime_setup It also = has a reference to _dl_tlsdesc_resolve_rela that becomes undefined when you= try to compile with your patchset including patch 13 where you remove the = code. >>=20 >> To make a test build I just commented it out but I think that this patch= should remove that if statement as well. >=20 > thanks, > indeed this was wrong, i tested the wrong branch on x86_64. >=20 > i will fix this and post a v2 set with the other feedback. On the positive side, I=E2=80=99ve been tracking down a problem where a lib= rary compiled with the gnu2 variant of TLS in a way that I haven=E2=80=99t = been able to reproduce yet is crashing the dynamic loader when used with a = performance tool that uses LD_AUDIT.=20 This patch alone (with my tiny modification below) addresses the problem. I= say =E2=80=9Caddresses=E2=80=9D because it doesn=E2=80=99t exactly fix the= problem; it makes it so that the code with the bug in it isn=E2=80=99t run= . Patch 13 in your patch set removes the code with the bug in it.=20 I see that patches 1 and 2 of your patch set have already been committed. I= would encourage you to consider committing V2 of patch 11 and 13 (or maybe= 11-14) even before the rest of the patch set since it addresses a bug that= we are seeing in the wild. -ben >=20 >>=20 >> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h >> index 9a876a371e..2b1b36a739 100644 >> --- a/sysdeps/x86_64/dl-machine.h >> +++ b/sysdeps/x86_64/dl-machine.h >> @@ -127,9 +127,11 @@ elf_machine_runtime_setup (struct link_map *l, int = lazy, int profile) >> } >> } >>=20 >> - if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy) >> - *(ElfW(Addr)*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) + l->l_a= ddr) >> - =3D (ElfW(Addr)) &_dl_tlsdesc_resolve_rela; >> + /* Lazy binding of TLSDESC relocations is no longer done so this logi= c >> + won't apply */ >> + /* if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy) */ >> + /* *(ElfW(Addr)*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) + l->= l_addr) */ >> + /* =3D (ElfW(Addr)) &_dl_tlsdesc_resolve_rela; */ >>=20 >> return lazy; >> } >>=20 >>=20 >>> On Feb 15, 2021, at 4:02 AM, Szabolcs Nagy via Libc-alpha wrote: >>>=20 >>> Lazy tlsdesc relocation is racy because the static tls optimization and >>> tlsdesc management operations are done without holding the dlopen lock. >>>=20 >>> This similar to the commit b7cf203b5c17dd6d9878537d41e0c7cc3d270a67 >>> for aarch64, but it fixes a different race: bug 27137. >>> --- >>> sysdeps/x86_64/dl-machine.h | 19 ++++++++++++++----- >>> 1 file changed, 14 insertions(+), 5 deletions(-) >>>=20 >>> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h >>> index 103eee6c3f..9a876a371e 100644 >>> --- a/sysdeps/x86_64/dl-machine.h >>> +++ b/sysdeps/x86_64/dl-machine.h >>> @@ -570,12 +570,21 @@ elf_machine_lazy_rel (struct link_map *map, >>> } >>> else if (__glibc_likely (r_type =3D=3D R_X86_64_TLSDESC)) >>> { >>> - struct tlsdesc volatile * __attribute__((__unused__)) td =3D >>> -=09(struct tlsdesc volatile *)reloc_addr; >>> + const Elf_Symndx symndx =3D ELFW (R_SYM) (reloc->r_info); >>> + const ElfW (Sym) *symtab =3D (const void *)D_PTR (map, l_info[DT= _SYMTAB]); >>> + const ElfW (Sym) *sym =3D &symtab[symndx]; >>> + const struct r_found_version *version =3D NULL; >>>=20 >>> - td->arg =3D (void*)reloc; >>> - td->entry =3D (void*)(D_PTR (map, l_info[ADDRIDX (DT_TLSDESC_PLT= )]) >>> -=09=09=09 + map->l_addr); >>> + if (map->l_info[VERSYMIDX (DT_VERSYM)] !=3D NULL) >>> +=09{ >>> +=09 const ElfW (Half) *vernum =3D >>> +=09 (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); >>> +=09 version =3D &map->l_versions[vernum[symndx] & 0x7fff]; >>> +=09} >>> + >>> + /* Always initialize TLS descriptors completely at load time, in >>> +=09 case static TLS is allocated for it that requires locking. */ >>> + elf_machine_rela (map, reloc, sym, version, reloc_addr, skip_ifu= nc); >>> } >>> else if (__glibc_unlikely (r_type =3D=3D R_X86_64_IRELATIVE)) >>> { >>> --=20 >>> 2.17.1 >>>=20 >>=20 >=20 > --=20