From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 8BD2A3890434 for ; Mon, 4 Jan 2021 19:50:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8BD2A3890434 Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-293-xhk2aJZdMLmEtD3VB6PmXw-1; Mon, 04 Jan 2021 14:50:24 -0500 X-MC-Unique: xhk2aJZdMLmEtD3VB6PmXw-1 Received: by mail-qt1-f197.google.com with SMTP id z43so18183134qtb.0 for ; Mon, 04 Jan 2021 11:50:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=KIGkPpSE5mndJ0j5WmNb2RnL+AZNmG67FSMX2poeVGM=; b=HDhEpWciNXDlGHz0x2APfkVo/DuAwVfn2xTZahPrxgSZgGFXYr8rOrxOQy4w1g5Lxb xxxFpoudA3ugHeo/v/xrwIPu5cOxUtrUEdKxTuV+4mXJHvnc8RPSlaaptSaCVewX3YZY rjWdHlqBD9uiyEO9lKzIACWNLkOznBf+LeufCtuamWGVN5elN++SnUC5soW/7Z3EOzUy YEQGYt7Bdn7j24+iNzFrPZDUezBXss84xiqxI4KjLNc/WY51BgM/n9MiJI9GMjAao5vz PaJvQuBXNDAYaspR49l9FwJ6b9In4mUu/GGoEjFglTlIhtbvb4934AFkfXui5tJ/IJHg 2VHg== X-Gm-Message-State: AOAM530f3Ydvv6og3WhnO7Qp+GkcAuxkvAth2S/h+8y1/Hl3YbMcKw3x H5m5rUIydvzPZAPgAXuW9bEMdUJO1KhnbX3r8B/B4IConf8hVsglXc47BZepjLalXthtqjKIBSz /N5QCHcEINGHhIiBju66k9JGs5UHPqvoVv533fDFhRrJnHc12xSwPP8deLkGZtJy0YY4j5A== X-Received: by 2002:a0c:a5a5:: with SMTP id z34mr77521265qvz.59.1609789823519; Mon, 04 Jan 2021 11:50:23 -0800 (PST) X-Google-Smtp-Source: ABdhPJzXm1Mp5LCUN6Vrmij+A81XCLdGqs8axzfZ+QfVemUVWVkzp2gmlLIu1XjoIitpT2Q6sGt06w== X-Received: by 2002:a0c:a5a5:: with SMTP id z34mr77521237qvz.59.1609789823206; Mon, 04 Jan 2021 11:50:23 -0800 (PST) Received: from [192.168.1.16] (198-84-214-74.cpe.teksavvy.com. [198.84.214.74]) by smtp.gmail.com with ESMTPSA id i27sm32348415qkk.15.2021.01.04.11.50.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Jan 2021 11:50:22 -0800 (PST) Subject: Re: V2 [PATCH] x86: Check IFUNC definition in unrelocated executable [BZ #20019] To: "H.J. Lu" Cc: GNU C Library References: <20201228141102.2562718-1-hjl.tools@gmail.com> <0d676184-2aee-6580-7281-dbbafbe671c4@redhat.com> From: Carlos O'Donell Organization: Red Hat Message-ID: <4058143e-9b84-f8f5-1361-420a5b3aa808@redhat.com> Date: Mon, 4 Jan 2021 14:50:21 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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:50:28 -0000 On 1/4/21 2:34 PM, H.J. Lu wrote: > 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? Could binutils have given the user a better warnings? OK for master. Reviewed-by: Carlos O'Donell > From 85fd4f35471038f734532ee902fd0b99a9aa16ba Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" > Date: Mon, 28 Dec 2020 05:28:49 -0800 > Subject: [PATCH] x86: Check IFUNC definition in unrelocated executable [BZ > #20019] > > Calling an IFUNC function defined in unrelocated executable also leads to > segfault. Issue a fatal error message when calling IFUNC function defined > in the unrelocated executable from a shared library. > --- > sysdeps/i386/dl-machine.h | 16 +++++++++++----- > sysdeps/x86_64/dl-machine.h | 16 +++++++++++----- > 2 files changed, 22 insertions(+), 10 deletions(-) > > diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h > index 50960605e6..23e9cc3bfb 100644 > --- a/sysdeps/i386/dl-machine.h > +++ b/sysdeps/i386/dl-machine.h > @@ -337,16 +337,22 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc, > { > # ifndef RTLD_BOOTSTRAP > 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_fatal_printf ("\ > +%s: IFUNC symbol '%s' referenced in '%s' is defined in the executable \ > +and creates an unsatisfiable circular dependency.\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 f582be5320..103eee6c3f 100644 > --- a/sysdeps/x86_64/dl-machine.h > +++ b/sysdeps/x86_64/dl-machine.h > @@ -314,16 +314,22 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc, > { > # ifndef RTLD_BOOTSTRAP > 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_fatal_printf ("\ > +%s: IFUNC symbol '%s' referenced in '%s' is defined in the executable \ > +and creates an unsatisfiable circular dependency.\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 = ((ElfW(Addr) (*) (void)) value) (); > -- > 2.29.2 > -- Cheers, Carlos.