From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com [IPv6:2607:f8b0:4864:20::b32]) by sourceware.org (Postfix) with ESMTPS id 9F4B23858402 for ; Thu, 28 Oct 2021 05:16:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9F4B23858402 Received: by mail-yb1-xb32.google.com with SMTP id a6so12171221ybq.9 for ; Wed, 27 Oct 2021 22:16:52 -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:content-transfer-encoding; bh=zHWwrZL/RX0YEJh9jVvp47v4R054qzTS6KimmG105qI=; b=UFCz4y/OWwN0ALe9XK1nLKuhmvtBigDu/kmVj/lxb6iVyOBxNasf8Pjdt2uI9WFIl5 N4iZr7RpyXg/twEmBiAHHlqHxdp5q3fjqdRCDlmw1mFv4k030MmzCkD03eS52trqJKjG P3S0ClS/4g22Np2vZc4ebevpt2vGj33yG7AT7yIjS7+EW9lqvnZA43lgMsFPSwKK6Q2m SHAl+bfekmddWS5NCmj34iVIL4zIJeW47xvqjbz2kkbV0YHJHPBdwPTfasUggynjN5+S nMERVwb8w9FSjoYTcIdCoFZmcFiCrUhgrjvLVzrV/BHyMr8n11APZxbjhP4SWgm+2HiA SiBA== X-Gm-Message-State: AOAM532mlIW2xqmk9hW0r2DXPcU0AcDvY9LDW3/vTPiuJKqwQoryclhr SLMAzNUIg4Vica+eT7xQ6gsK5ZyQXSVL7ke19Whtdg== X-Google-Smtp-Source: ABdhPJzQr8Rn9h4uj81pW/cWphdRNFRRnMcE2LvQUkhovoaObTZCD3GORvAZvXLCuM7qlgyBdlDCNPvF5jN9fMs/MQc= X-Received: by 2002:a25:8002:: with SMTP id m2mr2301234ybk.308.1635398211986; Wed, 27 Oct 2021 22:16:51 -0700 (PDT) MIME-Version: 1.0 References: <20210925004227.1829014-1-maskray@google.com> <877df2i32a.fsf@oldenburg.str.redhat.com> <20210927175922.rofwqmouuymi7nlc@google.com> <20211006202305.j5lzyjsbf2n5pjm6@google.com> <20211015001339.3nrmohqvblck7gqa@google.com> In-Reply-To: From: =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Date: Wed, 27 Oct 2021 22:16:39 -0700 Message-ID: Subject: Re: [PATCH] elf: Add __libc_get_static_tls_bounds [BZ #16291] To: Florian Weimer Cc: libc-alpha@sourceware.org, Adhemerval Zanella , GCC Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-17.7 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, KAM_INFOUSMEBIZ, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Oct 2021 05:16:55 -0000 On Tue, Oct 19, 2021 at 12:37 PM F=C4=81ng-ru=C3=AC S=C3=B2ng wrote: > > On Thu, Oct 14, 2021 at 5:13 PM Fangrui Song wrote: > > > > On 2021-10-06, Fangrui Song wrote: > > >On 2021-09-27, Fangrui Song wrote: > > >>On 2021-09-27, Florian Weimer wrote: > > >>>* Fangrui Song: > > >>> > > >>>>Sanitizer runtimes need static TLS boundaries for a variety of use = cases. > > >>>> > > >>>>* asan/hwasan/msan/tsan need to unpoison static TLS blocks to preve= nt false > > >>>> positives due to reusing the TLS blocks with a previous thread. > > >>>>* lsan needs TCB for pointers into pthread_setspecific regions. > > >>>> > > >>>>See https://maskray.me/blog/2021-02-14-all-about-thread-local-stora= ge > > >>>>for details. > > >>>> > > >>>>compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp GetTls= has > > >>>>to infer the static TLS bounds from TP, _dl_get_tls_static_info, an= d > > >>>>hard-coded TCB sizes. Currently this is somewhat robust for > > >>>>aarch64/powerpc64/x86-64 but is brittle for many other architecture= s. > > >>>> > > >>>>This patch implements __libc_get_static_tls_bounds@@GLIBC_PRIVATE w= hich > > >>>>is available in Android bionic since API level 31. This API allows = the > > >>>>sanitizer code to be more robust. _dl_get_tls_static_info@@GLIBC_PR= IVATE > > >>>>can probably be removed when Clang/GCC sanitizers drop reliance on = it. > > >>>>I am unclear whether the version should be GLIBC_2.*. > > >>> > > >>>Does this really cover the right memory region? I assume LSAN needs > > >>>something that identifies pointers to malloc'ed memory that are stor= ed > > >>>in non-malloc'ed (mmap'ed) memory. The static TLS region is certain= ly a > > >>>place where such pointers can be stored. But struct pthread also > > >>>contains other such pointers: the DTV, the TPP data, and POSIX TLS > > >>>(pthread_setspecific) data, and struct pthread is not obviously part= of > > >>>the static TLS region. > > >> > > >>I know the pthread_setspecific leak detection is brittle but it is > > >>currently implemented this way ;-) > > >> > > >>https://maskray.me/blog/2021-02-14-all-about-thread-local-storage say= s > > >> > > >>"On glibc, GetTls returned range includes > > >>pthread::{specific_1stblock,specific} for thread-specific data keys. > > >>There is currently a hack to ignore allocations from ld.so allocated > > >>dynamic TLS blocks. Note: if the pthread::{specific_1stblock,specific= } > > >>pointers are encrypted, lsan cannot track the allocation." > > >> > > >>If pthread::{specific_1stblock,specific} use an XOR technique (like > > >>__cxa_atexit/setjmp) the pthread_setspecific leak detection will stop > > >>working :( > > >> > > >>--- > > >> > > >>In any case, the pthread_setspecific leak detection is a relatively > > >>minor issue. The big issue is asan/msan/tsan false positives due to > > >>reusing an (exited) thread stack or its TLS blocks. > > >> > > >>Around > > >>https://code.woboq.org/llvm/compiler-rt/lib/sanitizer_common/sanitize= r_linux_libcdep.cpp.html#435 > > >>there is very long messy code hard coding the thread descriptor size = in > > >>glibc. > > >> > > >>Android `__libc_get_static_tls_bounds(&start_addr, &end_addr);` is th= e > > >>most robust one. > > >> > > >>--- > > >> > > >>I ported sanitizers to musl (https://reviews.llvm.org/D93848) > > >>in LLVM 12.0.0 and fixed some TLS block detection aarch64/ppc64 issue= s > > >>(https://reviews.llvm.org/D98926 and its follow-up, due to the > > >>complexity I couldn't get it right in the first place), so I have som= e > > >>understanding about sanitizers' TLS usage. > > > > > >Adhemerval showed me that the __libc_get_static_tls_bounds behavior is > > >expected on aarch64 as well ( > > >__libc_get_static_tls_bounds should match sanitizer GetTls) > > > > > >From https://gist.github.com/MaskRay/e035b85dce008f0c6d4997b98354d355 > > >``` > > >$ ./testrun.sh ./test-tls-boundary > > >+++GetTls: 0x7f9c5fd6c000 4416 > > >get_tls=3D0x7f9c600b4050 > > >_dl_get_tls_static_info: 4416 64 > > >get_static=3D0x7f9c600b4070 > > >__libc_get_static_tls_bounds: 0x7f9c5fd6c000 4416 > > >``` > > > > > > > > > > > >Is there any concern adding the interface? > > > > Gentle ping... > > > CC gcc-patches which ports compiler-rt and may be interested in more > reliable sanitizers. PING^3