From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14571 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] make relocation time symbol lookup and dlsym consistent Date: Tue, 13 Aug 2019 11:01:02 -0400 Message-ID: <20190813150102.GE9017@brightrain.aerifal.cx> References: <20190810231811.GK22009@port70.net> <20190813083844.GL22009@port70.net> <20190813142424.GD9017@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="220228"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14587-gllmg-musl=m.gmane.org@lists.openwall.com Tue Aug 13 17:01:18 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1hxYIY-000vCQ-0h for gllmg-musl@m.gmane.org; Tue, 13 Aug 2019 17:01:18 +0200 Original-Received: (qmail 23687 invoked by uid 550); 13 Aug 2019 15:01:15 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 23662 invoked from network); 13 Aug 2019 15:01:14 -0000 Content-Disposition: inline In-Reply-To: <20190813142424.GD9017@brightrain.aerifal.cx> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14571 Archived-At: On Tue, Aug 13, 2019 at 10:24:24AM -0400, Rich Felker wrote: > On Tue, Aug 13, 2019 at 10:38:44AM +0200, Szabolcs Nagy wrote: > > * Szabolcs Nagy [2019-08-11 01:18:11 +0200]: > > > static void *do_dlsym(struct dso *p, const char *s, void *ra) > > > { > > > - size_t i; > > > - uint32_t h = 0, gh = 0, *ght; > > > - Sym *sym; > > > - if (p == head || p == RTLD_DEFAULT || p == RTLD_NEXT) { > > > - if (p == RTLD_DEFAULT) { > > > - p = head; > > > - } else if (p == RTLD_NEXT) { > > > - p = addr2dso((size_t)ra); > > > - if (!p) p=head; > > > - p = p->next; > > > - } > > > - struct symdef def = find_sym(p, s, 0); > > > - if (!def.sym) goto failed; > > > - if ((def.sym->st_info&0xf) == STT_TLS) > > > - return __tls_get_addr((tls_mod_off_t []){def.dso->tls_id, def.sym->st_value-DTP_OFFSET}); > > > - if (DL_FDPIC && (def.sym->st_info&0xf) == STT_FUNC) > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > - return def.dso->funcdescs + (def.sym - def.dso->syms); > > > - return laddr(def.dso, def.sym->st_value); > > > - } > > > - if (__dl_invalid_handle(p)) > > > + int use_deps = 0; > > > + if (p == head || p == RTLD_DEFAULT) { > > > + p = head; > > > + } else if (p == RTLD_NEXT) { > > > + p = addr2dso((size_t)ra); > > > + if (!p) p=head; > > > + p = p->next; > > > + } else if (__dl_invalid_handle(p)) { > > > return 0; > > > - if ((ght = p->ghashtab)) { > > > - gh = gnu_hash(s); > > > - sym = gnu_lookup(gh, ght, p, s); > > > - } else { > > > - h = sysv_hash(s); > > > - sym = sysv_lookup(s, h, p); > > > - } > > > - if (sym && (sym->st_info&0xf) == STT_TLS) > > > - return __tls_get_addr((tls_mod_off_t []){p->tls_id, sym->st_value-DTP_OFFSET}); > > > - if (DL_FDPIC && sym && sym->st_shndx && (sym->st_info&0xf) == STT_FUNC) > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > - return p->funcdescs + (sym - p->syms); > > > - if (sym && sym->st_value && (1<<(sym->st_info&0xf) & OK_TYPES)) > > > - return laddr(p, sym->st_value); > > > - for (i=0; p->deps[i]; i++) { > > > - if ((ght = p->deps[i]->ghashtab)) { > > > - if (!gh) gh = gnu_hash(s); > > > - sym = gnu_lookup(gh, ght, p->deps[i], s); > > > - } else { > > > - if (!h) h = sysv_hash(s); > > > - sym = sysv_lookup(s, h, p->deps[i]); > > > - } > > > - if (sym && (sym->st_info&0xf) == STT_TLS) > > > - return __tls_get_addr((tls_mod_off_t []){p->deps[i]->tls_id, sym->st_value-DTP_OFFSET}); > > > - if (DL_FDPIC && sym && sym->st_shndx && (sym->st_info&0xf) == STT_FUNC) > > > - return p->deps[i]->funcdescs + (sym - p->deps[i]->syms); > > > - if (sym && sym->st_value && (1<<(sym->st_info&0xf) & OK_TYPES)) > > > - return laddr(p->deps[i], sym->st_value); > > > - } > > > -failed: > > > - error("Symbol not found: %s", s); > > > - return 0; > > > + } else > > > + use_deps = 1; > > > + struct symdef def = find_sym2(p, s, 0, use_deps); > > > + if (!def.sym) { > > > + error("Symbol not found: %s", s); > > > + return 0; > > > + } > > > + if ((def.sym->st_info&0xf) == STT_TLS) > > > + return __tls_get_addr((tls_mod_off_t []){def.dso->tls_id, def.sym->st_value-DTP_OFFSET}); > > > + if (DL_FDPIC && (def.sym->st_info&0xf) == STT_FUNC) > > > + return def.dso->funcdescs + (def.sym - def.dso->syms); > > > > there is another behaviour change i did not notice before: > > > > st_shndx is no longer checked in DL_FDPIC case here, i assumed > > find_sym did that, but there is no fdpic specific logic there. > > > > the old code was inconsistent in the RTLD_DEFAULT vs shared lib > > dlsym case. > > > > i dont know if this is relevant for fdpic, i didnt test that case. > > Thanks for catching. I'll take a look at this. commit d47d9a50f2568927af51e21b2f2120409db1ab44 documented that the logic probably isn't entirely correct. It looks to me like st_shndx being 0/undef here was being used as a proxy for the symbol being completely undefined (just a reference), which is normally (on non-fdpic) determined by the value being 0; nonzero value but 0/undef section means PLT thunk or copy relocation. The code in question only applies to functions, not data, and fdpic does not have PLT thunks that provide definitions since the function definition is always the *descriptor*, whose existence ldso is responsible for managing. So, I think your new code is okay as-is. Let me know if any of this sounds wrong or even dubious. Rich