From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32d.google.com (mail-ot1-x32d.google.com [IPv6:2607:f8b0:4864:20::32d]) by sourceware.org (Postfix) with ESMTPS id A67773890420 for ; Mon, 4 Jan 2021 19:34:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A67773890420 Received: by mail-ot1-x32d.google.com with SMTP id q25so27087055otn.10 for ; Mon, 04 Jan 2021 11:34:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gt1PZyJaaO945UpbsDSgY9nVdrDrwxfKVAEEQBSZvk8=; b=B3Wzx4ZRPBhMqz8zlD5h2neP7YCJqZUKKhIAdsT0f4vFsASeoW/RTUiF6Gp19T7OdY 1w/nqtmhtYLxgMIWjXtJL/+Fj0Hy7KCtcusUNpvQqn5g3+gJDpGOM/wDp6hxoTfPmh5B WfKsJ/36RqK3SWlhZz+5ATUet9I8pNKHCKbKSpEndkSqhaZBX2Xga1ad/BcUYLx7U5Rm FWMjojepeHJQyi+HuD1zOy582lpowTQ9bxm5aYnrtxOSeQ0QWgtLOiZA3j8pCqByqh/c B9CBjadBceGV6LbWbkaStDqRxZRdpHEYgfMCzR9wf/+QsIHPLaBbEBM+4AYnyANkYF9w 6pUQ== X-Gm-Message-State: AOAM532VEuKgg4hvfwczWa+Q2O/4Ui6xCb6cNmRxbggSnFEyYcMms0Oy XxOWjPzrqXVQjWrSYFo2UmO8CmL64FhRWfc+lUDK1HW573I= X-Google-Smtp-Source: ABdhPJz1IgcvKF2Dg4/8gzfeGSthpji2txeItm9m2KUXhUwrqV+e2D8V2XZpnn1GABSY34dn88MzivbXRarcdVosInE= X-Received: by 2002:a9d:6285:: with SMTP id x5mr53942930otk.179.1609788888845; Mon, 04 Jan 2021 11:34:48 -0800 (PST) MIME-Version: 1.0 References: <20201228141102.2562718-1-hjl.tools@gmail.com> <0d676184-2aee-6580-7281-dbbafbe671c4@redhat.com> In-Reply-To: <0d676184-2aee-6580-7281-dbbafbe671c4@redhat.com> From: "H.J. Lu" Date: Mon, 4 Jan 2021 11:34:12 -0800 Message-ID: Subject: V2 [PATCH] x86: Check IFUNC definition in unrelocated executable [BZ #20019] To: "Carlos O'Donell" Cc: GNU C Library Content-Type: multipart/mixed; boundary="000000000000e22f2105b8182e8e" X-Spam-Status: No, score=-3037.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, 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 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, 04 Jan 2021 19:34:51 -0000 --000000000000e22f2105b8182e8e Content-Type: text/plain; charset="UTF-8" On Mon, Jan 4, 2021 at 10:47 AM Carlos O'Donell wrote: > > On 12/28/20 9:11 AM, H.J. Lu via Libc-alpha wrote: > > Calling an IFUNC function defined in unrelocated executable may also > > lead to segfault. Issue an error message when calling IFUNC function > > defined in the unrelocated executable from a shared library. > > The logic here makes sense, but we need a stronger error message. > > Please review my understanding and suggested error message. > > Looking forward to v2. > > > --- > > sysdeps/i386/dl-machine.h | 15 ++++++++++----- > > sysdeps/x86_64/dl-machine.h | 15 ++++++++++----- > > 2 files changed, 20 insertions(+), 10 deletions(-) > > > > diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h > > index fea9e579ec..dedda484ba 100644 > > --- a/sysdeps/i386/dl-machine.h > > +++ b/sysdeps/i386/dl-machine.h > > @@ -337,16 +337,21 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc, > > { > > # ifndef RTLD_BOOTSTRAP > > OK. Logic is in the correct place in dl-machine.h for i386. > > > if (sym_map != map > > - && sym_map->l_type != lt_executable > > && !sym_map->l_relocated) > > { > > const char *strtab > > = (const char *) D_PTR (map, l_info[DT_STRTAB]); > > - _dl_error_printf ("\ > > + if (sym_map->l_type == lt_executable) > > + _dl_error_printf ("\ > > +%s: IFUNC symbol `%s' referenced in `%s' is defined in executable\n", > > + RTLD_PROGNAME, strtab + refsym->st_name, > > + map->l_name); > > + else > > + _dl_error_printf ("\ > > %s: Relink `%s' with `%s' for IFUNC symbol `%s'\n", > > - RTLD_PROGNAME, map->l_name, > > - sym_map->l_name, > > - strtab + refsym->st_name); > > + RTLD_PROGNAME, map->l_name, > > + sym_map->l_name, > > + strtab + refsym->st_name); > > } > > # endif > > value = ((Elf32_Addr (*) (void)) value) (); > > diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h > > index bb93c7c6ab..fc847f4bc2 100644 > > --- a/sysdeps/x86_64/dl-machine.h > > +++ b/sysdeps/x86_64/dl-machine.h > > @@ -314,16 +314,21 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc, > > { > > # ifndef RTLD_BOOTSTRAP > > OK. Logic is in the correct place in dl-machine.h for x86_64. > > > if (sym_map != map > > - && sym_map->l_type != lt_executable > > && !sym_map->l_relocated) > > { > > const char *strtab > > = (const char *) D_PTR (map, l_info[DT_STRTAB]); > > - _dl_error_printf ("\ > > + if (sym_map->l_type == lt_executable) > > + _dl_error_printf ("\ > > +%s: IFUNC symbol `%s' referenced in `%s' is defined in executable\n", > > The message should explain the error > e.g. "Such and such *must not* reference such and such." > > Or the message should explain how to fix the error (as the other does) > e.g. "Such and such must be relinked with such and such." > > We have made this a hard error. An executable with immediate binding > may not define an IFUNC resolver and implementation that is used from > a shared library since it creates an ordering issue with the dependent > libraries that use the resolution of the symbol i.e. you must initialize > the executable but to do that you must initialize the libraries, but to > do that you must initialize the executable etc. etc. > > In which case the error message could be: > > "%s: IFUNC symbol '%s' referenced in '%s' is defined in the executable > and creates an unsatisfiable circular dependency." Fixed. > Note: Use '' quotes not `' since the GNU Coding standards have changed. > https://www.gnu.org/prep/standards/standards.html#Quote-Characters > > > + RTLD_PROGNAME, strtab + refsym->st_name, > > + map->l_name); > > + else > > + _dl_error_printf ("\ > > %s: Relink `%s' with `%s' for IFUNC symbol `%s'\n", > > - RTLD_PROGNAME, map->l_name, > > - sym_map->l_name, > > - strtab + refsym->st_name); > > + RTLD_PROGNAME, map->l_name, > > + sym_map->l_name, > > + strtab + refsym->st_name); > > } > > # endif > > value = ((ElfW(Addr) (*) (void)) value) (); > > > > Here is the updated patch. Changes from V1: 1. Update the error message based on feedback from Carlos. 2. Make the error fatal instead of segfault later. OK for master? Thanks. -- H.J. --000000000000e22f2105b8182e8e Content-Type: text/x-patch; charset="US-ASCII"; name="0001-x86-Check-IFUNC-definition-in-unrelocated-executable.patch" Content-Disposition: attachment; filename="0001-x86-Check-IFUNC-definition-in-unrelocated-executable.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_kjiypldq0 RnJvbSA4NWZkNGYzNTQ3MTAzOGY3MzQ1MzJlZTkwMmZkMGI5OWE5YWExNmJhIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiAiSC5KLiBMdSIgPGhqbC50b29sc0BnbWFpbC5jb20+CkRhdGU6 IE1vbiwgMjggRGVjIDIwMjAgMDU6Mjg6NDkgLTA4MDAKU3ViamVjdDogW1BBVENIXSB4ODY6IENo ZWNrIElGVU5DIGRlZmluaXRpb24gaW4gdW5yZWxvY2F0ZWQgZXhlY3V0YWJsZSBbQloKICMyMDAx OV0KCkNhbGxpbmcgYW4gSUZVTkMgZnVuY3Rpb24gZGVmaW5lZCBpbiB1bnJlbG9jYXRlZCBleGVj dXRhYmxlIGFsc28gbGVhZHMgdG8Kc2VnZmF1bHQuICBJc3N1ZSBhIGZhdGFsIGVycm9yIG1lc3Nh Z2Ugd2hlbiBjYWxsaW5nIElGVU5DIGZ1bmN0aW9uIGRlZmluZWQKaW4gdGhlIHVucmVsb2NhdGVk IGV4ZWN1dGFibGUgZnJvbSBhIHNoYXJlZCBsaWJyYXJ5LgotLS0KIHN5c2RlcHMvaTM4Ni9kbC1t YWNoaW5lLmggICB8IDE2ICsrKysrKysrKysrLS0tLS0KIHN5c2RlcHMveDg2XzY0L2RsLW1hY2hp bmUuaCB8IDE2ICsrKysrKysrKysrLS0tLS0KIDIgZmlsZXMgY2hhbmdlZCwgMjIgaW5zZXJ0aW9u cygrKSwgMTAgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3lzZGVwcy9pMzg2L2RsLW1hY2hp bmUuaCBiL3N5c2RlcHMvaTM4Ni9kbC1tYWNoaW5lLmgKaW5kZXggNTA5NjA2MDVlNi4uMjNlOWNj M2JmYiAxMDA2NDQKLS0tIGEvc3lzZGVwcy9pMzg2L2RsLW1hY2hpbmUuaAorKysgYi9zeXNkZXBz L2kzODYvZGwtbWFjaGluZS5oCkBAIC0zMzcsMTYgKzMzNywyMiBAQCBlbGZfbWFjaGluZV9yZWwg KHN0cnVjdCBsaW5rX21hcCAqbWFwLCBjb25zdCBFbGYzMl9SZWwgKnJlbG9jLAogCXsKICMgaWZu ZGVmIFJUTERfQk9PVFNUUkFQCiAJICBpZiAoc3ltX21hcCAhPSBtYXAKLQkgICAgICAmJiBzeW1f bWFwLT5sX3R5cGUgIT0gbHRfZXhlY3V0YWJsZQogCSAgICAgICYmICFzeW1fbWFwLT5sX3JlbG9j YXRlZCkKIAkgICAgewogCSAgICAgIGNvbnN0IGNoYXIgKnN0cnRhYgogCQk9IChjb25zdCBjaGFy ICopIERfUFRSIChtYXAsIGxfaW5mb1tEVF9TVFJUQUJdKTsKLQkgICAgICBfZGxfZXJyb3JfcHJp bnRmICgiXAorCSAgICAgIGlmIChzeW1fbWFwLT5sX3R5cGUgPT0gbHRfZXhlY3V0YWJsZSkKKwkJ X2RsX2ZhdGFsX3ByaW50ZiAoIlwKKyVzOiBJRlVOQyBzeW1ib2wgJyVzJyByZWZlcmVuY2VkIGlu ICclcycgaXMgZGVmaW5lZCBpbiB0aGUgZXhlY3V0YWJsZSBcCithbmQgY3JlYXRlcyBhbiB1bnNh dGlzZmlhYmxlIGNpcmN1bGFyIGRlcGVuZGVuY3kuXG4iLAorCQkJCSAgUlRMRF9QUk9HTkFNRSwg c3RydGFiICsgcmVmc3ltLT5zdF9uYW1lLAorCQkJCSAgbWFwLT5sX25hbWUpOworCSAgICAgIGVs c2UKKwkJX2RsX2Vycm9yX3ByaW50ZiAoIlwKICVzOiBSZWxpbmsgYCVzJyB3aXRoIGAlcycgZm9y IElGVU5DIHN5bWJvbCBgJXMnXG4iLAotCQkJCVJUTERfUFJPR05BTUUsIG1hcC0+bF9uYW1lLAot CQkJCXN5bV9tYXAtPmxfbmFtZSwKLQkJCQlzdHJ0YWIgKyByZWZzeW0tPnN0X25hbWUpOworCQkJ CSAgUlRMRF9QUk9HTkFNRSwgbWFwLT5sX25hbWUsCisJCQkJICBzeW1fbWFwLT5sX25hbWUsCisJ CQkJICBzdHJ0YWIgKyByZWZzeW0tPnN0X25hbWUpOwogCSAgICB9CiAjIGVuZGlmCiAJICB2YWx1 ZSA9ICgoRWxmMzJfQWRkciAoKikgKHZvaWQpKSB2YWx1ZSkgKCk7CmRpZmYgLS1naXQgYS9zeXNk ZXBzL3g4Nl82NC9kbC1tYWNoaW5lLmggYi9zeXNkZXBzL3g4Nl82NC9kbC1tYWNoaW5lLmgKaW5k ZXggZjU4MmJlNTMyMC4uMTAzZWVlNmMzZiAxMDA2NDQKLS0tIGEvc3lzZGVwcy94ODZfNjQvZGwt bWFjaGluZS5oCisrKyBiL3N5c2RlcHMveDg2XzY0L2RsLW1hY2hpbmUuaApAQCAtMzE0LDE2ICsz MTQsMjIgQEAgZWxmX21hY2hpbmVfcmVsYSAoc3RydWN0IGxpbmtfbWFwICptYXAsIGNvbnN0IEVs ZlcoUmVsYSkgKnJlbG9jLAogCXsKICMgaWZuZGVmIFJUTERfQk9PVFNUUkFQCiAJICBpZiAoc3lt X21hcCAhPSBtYXAKLQkgICAgICAmJiBzeW1fbWFwLT5sX3R5cGUgIT0gbHRfZXhlY3V0YWJsZQog CSAgICAgICYmICFzeW1fbWFwLT5sX3JlbG9jYXRlZCkKIAkgICAgewogCSAgICAgIGNvbnN0IGNo YXIgKnN0cnRhYgogCQk9IChjb25zdCBjaGFyICopIERfUFRSIChtYXAsIGxfaW5mb1tEVF9TVFJU QUJdKTsKLQkgICAgICBfZGxfZXJyb3JfcHJpbnRmICgiXAorCSAgICAgIGlmIChzeW1fbWFwLT5s X3R5cGUgPT0gbHRfZXhlY3V0YWJsZSkKKwkJX2RsX2ZhdGFsX3ByaW50ZiAoIlwKKyVzOiBJRlVO QyBzeW1ib2wgJyVzJyByZWZlcmVuY2VkIGluICclcycgaXMgZGVmaW5lZCBpbiB0aGUgZXhlY3V0 YWJsZSBcCithbmQgY3JlYXRlcyBhbiB1bnNhdGlzZmlhYmxlIGNpcmN1bGFyIGRlcGVuZGVuY3ku XG4iLAorCQkJCSAgUlRMRF9QUk9HTkFNRSwgc3RydGFiICsgcmVmc3ltLT5zdF9uYW1lLAorCQkJ CSAgbWFwLT5sX25hbWUpOworCSAgICAgIGVsc2UKKwkJX2RsX2Vycm9yX3ByaW50ZiAoIlwKICVz OiBSZWxpbmsgYCVzJyB3aXRoIGAlcycgZm9yIElGVU5DIHN5bWJvbCBgJXMnXG4iLAotCQkJCVJU TERfUFJPR05BTUUsIG1hcC0+bF9uYW1lLAotCQkJCXN5bV9tYXAtPmxfbmFtZSwKLQkJCQlzdHJ0 YWIgKyByZWZzeW0tPnN0X25hbWUpOworCQkJCSAgUlRMRF9QUk9HTkFNRSwgbWFwLT5sX25hbWUs CisJCQkJICBzeW1fbWFwLT5sX25hbWUsCisJCQkJICBzdHJ0YWIgKyByZWZzeW0tPnN0X25hbWUp OwogCSAgICB9CiAjIGVuZGlmCiAJICB2YWx1ZSA9ICgoRWxmVyhBZGRyKSAoKikgKHZvaWQpKSB2 YWx1ZSkgKCk7Ci0tIAoyLjI5LjIKCg== --000000000000e22f2105b8182e8e--