From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by sourceware.org (Postfix) with ESMTPS id CD6AD3858C2C for ; Mon, 27 Sep 2021 17:59:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD6AD3858C2C Received: by mail-pj1-x102a.google.com with SMTP id r7so12444679pjo.3 for ; Mon, 27 Sep 2021 10:59:28 -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=+IOTVf0JvdO/WgiFD2lBBr9dP51bOuCdbDrdBNg+2II=; b=VV4lTbZFDf+ivvvZV3GNhGqNuGexexE6g10WxEFL1qpF0uctXNspl69HDNR6FxXSGY UtImw86518Z6myaFHNTImvEnxUZIqFZmIUa4So5sh+2p0IH17YssCOQBWWx2kdrr7Jt1 5c+m4lXFWHi0kH34hQ48XXlQSvFpVFCtMz4JrpFK7IQhh8BB/GMylIQbVr8nz0fB25nt NmgSvmgYZ+elGnsfgTHtSmj9KeImHVKrdqxfEpbRDXHHg9Awy1ROktVR7lIVBTRYSldt bWCK/IcKXRHJO1PKFf2JlzvFhvWPUjJtM2M8dwzFMAmdbHmjcWdmesigTing3j+JWtIj GiIA== X-Gm-Message-State: AOAM532dUZ5b4gJoQJNUDIEJYXMH3TVphuhdkVJE/t2YsizJ6PNoM484 AWhvuJSAIbxtqxzkL/KslDIq/w== X-Google-Smtp-Source: ABdhPJwyJvDa1cYAs3gvSpP+nA2a3tSScsOpE1cijtiwKtauhm6Xsz8Hjl0Z5hS6edqrcDwD9sJosw== X-Received: by 2002:a17:90a:bf82:: with SMTP id d2mr392185pjs.201.1632765567709; Mon, 27 Sep 2021 10:59:27 -0700 (PDT) Received: from google.com ([2620:15c:2ce:200:a67:5114:baf5:bd8b]) by smtp.gmail.com with ESMTPSA id y13sm119729pjt.15.2021.09.27.10.59.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Sep 2021 10:59:27 -0700 (PDT) Date: Mon, 27 Sep 2021 10:59:22 -0700 From: Fangrui Song To: Florian Weimer Cc: libc-alpha@sourceware.org, Adhemerval Zanella Subject: Re: [PATCH] elf: Add __libc_get_static_tls_bounds [BZ #16291] Message-ID: <20210927175922.rofwqmouuymi7nlc@google.com> References: <20210925004227.1829014-1-maskray@google.com> <877df2i32a.fsf@oldenburg.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <877df2i32a.fsf@oldenburg.str.redhat.com> X-Spam-Status: No, score=-16.9 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, FSL_HELO_FAKE, 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: 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: Mon, 27 Sep 2021 17:59:30 -0000 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 prevent 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-storage >> 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, and >> hard-coded TCB sizes. Currently this is somewhat robust for >> aarch64/powerpc64/x86-64 but is brittle for many other architectures. >> >> This patch implements __libc_get_static_tls_bounds@@GLIBC_PRIVATE which >> 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_PRIVATE >> 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 stored >in non-malloc'ed (mmap'ed) memory. The static TLS region is certainly 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 says "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/sanitizer_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 the 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 issues (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 some understanding about sanitizers' TLS usage.