From patchwork Sat May 7 01:29:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 619516 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3r1rgz6Br0z9t5c for ; Sat, 7 May 2016 11:30:55 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=jrSzu3Al; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id :mime-version:content-type; q=dns; s=default; b=J0gCOUgvU1IvE40Q JQdamB16TtSx88fP/GbUSLvSJ0zjAn7/63y4vZirEkRb1wYgS4xic2KO1X0cEJAs rC9Mj4Vc7uCk+l7fhVxaU+PTA01zZ+t4yfP0Q00iJQgOHsq9n9n9Iir+vCrfXhBH 91WQ6BbHKPCshHX9Q9w14Of8uFo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id :mime-version:content-type; s=default; bh=RQI9w0LHxCj0VTVrw1kZJn nB0bc=; b=jrSzu3Alq2XgxiE/aaYWJ368ACpEbUBSGPSqNQWggk4ROJn4MJmMV/ 1W6lPrRYFIbK3da15nL6VHQN47Zo7QGbkgWTBjl6SK8Xum9NqzjDU3bWKFKSYoBW C5fR7nbR8z3rM8tbDzkk3MiyjvyJngS7N2DH7hjIk8BdZAz6AU3WQ= Received: (qmail 50480 invoked by alias); 7 May 2016 01:30:26 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 50375 invoked by uid 89); 7 May 2016 01:30:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=2016-05-07, santa, Santa, gabi X-Spam-User: qpsmtpd, 2 recipients X-HELO: mailapp01.imgtec.com Date: Sat, 7 May 2016 02:29:54 +0100 From: "Maciej W. Rozycki" To: CC: Subject: [PATCH v2] Treat STV_HIDDEN and STV_INTERNAL symbols as STB_LOCAL Message-ID: User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 In a reference to PR ld/19908 make ld.so respect symbol export classes aka visibility and treat STV_HIDDEN and STV_INTERNAL symbols as local, preventing such symbols from preempting exported symbols. According to the ELF gABI[1] neither STV_HIDDEN nor STV_INTERNAL symbols are supposed to be present in linked binaries: "A hidden symbol contained in a relocatable object must be either removed or converted to STB_LOCAL binding by the link-editor when the relocatable object is included in an executable file or shared object." "An internal symbol contained in a relocatable object must be either removed or converted to STB_LOCAL binding by the link-editor when the relocatable object is included in an executable file or shared object." however some GNU binutils versions produce such symbols in some cases. PR ld/19908 is one and we also have this note in scripts/abilist.awk: # binutils versions up through at least 2.23 have some bugs that # caused STV_HIDDEN symbols to appear in .dynsym, though that is useless. so clearly there is linked code out there which contains such symbols which is prone to symbol table misinterpretation, and it'll be more productive if we handle this gracefully, under the Robustness Principle: "be liberal in what you accept, and conservative in what you produce", especially as this is a simple (STV_HIDDEN|STV_INTERNAL) => STB_LOCAL mapping. References: [1] "System V Application Binary Interface - DRAFT - 24 April 2001", The Santa Cruz Operation, Inc., "Symbol Table", 2016-05-07 Maciej W. Rozycki * sysdeps/generic/ldsodefs.h (dl_symbol_visibility_binds_local_p): New inline function. * elf/dl-addr.c (determine_info): Treat hidden and internal symbols as local. * elf/dl-lookup.c (do_lookup_x): Likewise. * elf/dl-reloc.c (RESOLVE_MAP): Likewise. --- Hi, This is an updated version taking Roland's suggestions into account. Rather than using a macro I decided to go for an inline function instead, as I have a preference for them in code which can assume modern tools. Among the reasons are type checking and single evaluation of the parameters. I hope this choice is OK, we have numerous inline functions throughout already. This has passed testing with the mips-linux-gnu target, little-endian, o32 ABI, with no regressions. OK to apply? Maciej Changes from v1: - factor out checks to `dl_symbol_visibility_binds_local_p', - update the formatting of `ELFW' macro calls. glibc-elf-export-class-local.diff Index: glibc/elf/dl-addr.c =================================================================== --- glibc.orig/elf/dl-addr.c 2016-05-05 19:36:17.395247035 +0100 +++ glibc/elf/dl-addr.c 2016-05-06 04:37:39.262222495 +0100 @@ -88,6 +88,7 @@ determine_info (const ElfW(Addr) addr, s for (; (void *) symtab < (void *) symtabend; ++symtab) if ((ELFW(ST_BIND) (symtab->st_info) == STB_GLOBAL || ELFW(ST_BIND) (symtab->st_info) == STB_WEAK) + && __glibc_likely (!dl_symbol_visibility_binds_local_p (symtab)) && ELFW(ST_TYPE) (symtab->st_info) != STT_TLS && (symtab->st_shndx != SHN_UNDEF || symtab->st_value != 0) Index: glibc/elf/dl-lookup.c =================================================================== --- glibc.orig/elf/dl-lookup.c 2016-05-05 20:13:10.146644695 +0100 +++ glibc/elf/dl-lookup.c 2016-05-06 04:40:30.236831042 +0100 @@ -516,6 +516,10 @@ do_lookup_x (const char *undef_name, uin #endif } + /* Hidden and internal symbols are local, ignore them. */ + if (__glibc_unlikely (dl_symbol_visibility_binds_local_p (sym))) + goto skip; + switch (ELFW(ST_BIND) (sym->st_info)) { case STB_WEAK: Index: glibc/elf/dl-reloc.c =================================================================== --- glibc.orig/elf/dl-reloc.c 2016-05-05 19:36:17.418334585 +0100 +++ glibc/elf/dl-reloc.c 2016-05-06 04:41:12.649078174 +0100 @@ -233,7 +233,8 @@ _dl_relocate_object (struct link_map *l, /* This macro is used as a callback from the ELF_DYNAMIC_RELOCATE code. */ #define RESOLVE_MAP(ref, version, r_type) \ - (ELFW(ST_BIND) ((*ref)->st_info) != STB_LOCAL \ + ((ELFW (ST_BIND) ((*ref)->st_info) != STB_LOCAL \ + && __glibc_likely (!dl_symbol_visibility_binds_local_p (*ref))) \ ? ((__builtin_expect ((*ref) == l->l_lookup_cache.sym, 0) \ && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class) \ ? (bump_num_cache_relocations (), \ Index: glibc/sysdeps/generic/ldsodefs.h =================================================================== --- glibc.orig/sysdeps/generic/ldsodefs.h 2016-05-05 20:13:20.000000000 +0100 +++ glibc/sysdeps/generic/ldsodefs.h 2016-05-06 07:23:22.874587470 +0100 @@ -88,6 +88,19 @@ typedef struct link_map *lookup_t; || (ADDR) < (L)->l_addr + (SYM)->st_value + (SYM)->st_size) \ && ((MATCHSYM) == NULL || (MATCHSYM)->st_value < (SYM)->st_value)) +/* According to the ELF gABI no STV_HIDDEN or STV_INTERNAL symbols are + expected to be present in dynamic symbol tables as they should have + been either removed or converted to STB_LOCAL binding by the static + linker. However some GNU binutils versions produce such symbols in + some cases. To prevent such symbols present in a buggy binary from + preempting global symbols we filter them out with this predicate. */ +static __always_inline bool +dl_symbol_visibility_binds_local_p (const ElfW(Sym) *sym) +{ + return (ELFW(ST_VISIBILITY) (sym->st_other) == STV_HIDDEN + || ELFW(ST_VISIBILITY) (sym->st_other) == STV_INTERNAL); +} + /* Unmap a loaded object, called by _dl_close (). */ #ifndef DL_UNMAP_IS_SPECIAL # define DL_UNMAP(map) _dl_unmap_segments (map)