Message ID | 4224b7c0428492696fe6d6c01739adcf69fc677d.1610986541.git.szabolcs.nagy@arm.com |
---|---|
State | Superseded |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 56D61388C022; Mon, 18 Jan 2021 16:26:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 56D61388C022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610987183; bh=a8noJ/CxNzPAJZ7mQ3fA/tA9+XDOvYiygNvTFIWwnVE=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=P/sUuj+w015NAWeA8TJ/B553/7Esq2tNt6qTV8DFCfC0DSfpdcRcSVypMttSxCc9+ R7X1rr/F7ZnCJ+ziA+HXOLF4sCxFHivxuStRmp7ZtMG3vcE7TuoOXWQ6zbVYPs4TnT vkupnCIxM7+VvST33Vuj+3WZ8N5tvFweBNkG0fM8= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from EUR04-VI1-obe.outbound.protection.outlook.com (unknown [52.100.19.12]) by sourceware.org (Postfix) with ESMTPS id 38655388C01A for <libc-alpha@sourceware.org>; Mon, 18 Jan 2021 16:26:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 38655388C01A Received: from AM6PR01CA0062.eurprd01.prod.exchangelabs.com (2603:10a6:20b:e0::39) by VI1PR08MB3997.eurprd08.prod.outlook.com (2603:10a6:803:e0::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3763.10; Mon, 18 Jan 2021 16:26:18 +0000 Received: from AM5EUR03FT025.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:e0:cafe::96) by AM6PR01CA0062.outlook.office365.com (2603:10a6:20b:e0::39) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3763.9 via Frontend Transport; Mon, 18 Jan 2021 16:26:18 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; sourceware.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;sourceware.org; dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT025.mail.protection.outlook.com (10.152.16.157) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3763.12 via Frontend Transport; Mon, 18 Jan 2021 16:26:17 +0000 Received: ("Tessian outbound 4d8113405d55:v71"); Mon, 18 Jan 2021 16:26:17 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 42b85a8277846d30 X-CR-MTA-TID: 64aa7808 Received: from 3fd372ec92e3.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 28240887-0B7D-4CF7-8072-60DE2DEAB3CC.1; Mon, 18 Jan 2021 16:25:40 +0000 Received: from EUR01-VE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 3fd372ec92e3.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Mon, 18 Jan 2021 16:25:40 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ed21z6gl+FS9QZ8MPfe3V0LAk1MigNP4UeduZUNUUev2xF5hQy6Wagc0dz/xnfYmVylQRZIibTPhWflRWUMS143VNMMLiyItuLe1vCh5K4OjKYqcb5Ze/hjZ4MSW/N7cz7hSobSbwal5iPhiTzhAKJW+a+rgrNB4/7FkZALiFwkAgZD9IOszbSLUtwT1n6N+YrvPyZhPo24Q8wUHqgJexVmm9akrgsknqfgjeDR2I706Hp+dOb7ZkkOtWYTbRulinNXLQQuTQpnXch8X48hPl0VffkLWTq0EAWTTzOFO92OJM1oNTH150KjMQ7mqL6cWH0gavm6Gkf+YEi9GNWNnow== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=a8noJ/CxNzPAJZ7mQ3fA/tA9+XDOvYiygNvTFIWwnVE=; b=LSktN2iW5/YMwTq1fmoQuqltPxCuC1V6j1qtsAZ/6uYBugTottmGZCEkGTipSYEq3TTtBfRy9Mla7LfKem+8dydeL2cCmz01mgT1Wps2nKdbQuYkTWPuRFwCBSlkHoqbXiRWS0gOjLn0XJW1t2cUzesZoKypggZlelxqgTJ5iO1V9b+ij4TFNIO5HFnjo1xDEhK6Hr5dZyxhUAQ8B13V6/cQiLsbGVude28SCm5kMozcnucsRgQSFZOXfYjgHqARqZ30TmORjVlxzihI5IquLxrRRBbYkgxnIW+WQreD/PKQzX8b9Ek77em1HM6o2niDaJBDYo0g1oA3djF2X08GcQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none Authentication-Results-Original: sourceware.org; dkim=none (message not signed) header.d=none;sourceware.org; dmarc=none action=none header.from=arm.com; Received: from PA4PR08MB6320.eurprd08.prod.outlook.com (2603:10a6:102:e5::9) by PA4PR08MB6238.eurprd08.prod.outlook.com (2603:10a6:102:e8::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3763.11; Mon, 18 Jan 2021 16:25:39 +0000 Received: from PA4PR08MB6320.eurprd08.prod.outlook.com ([fe80::700f:ddbe:a347:ee4f]) by PA4PR08MB6320.eurprd08.prod.outlook.com ([fe80::700f:ddbe:a347:ee4f%7]) with mapi id 15.20.3763.014; Mon, 18 Jan 2021 16:25:39 +0000 To: libc-alpha@sourceware.org Subject: [PATCH v4 08/10] csu: Move static pie self relocation later [BZ #27072] Date: Mon, 18 Jan 2021 16:25:28 +0000 Message-Id: <4224b7c0428492696fe6d6c01739adcf69fc677d.1610986541.git.szabolcs.nagy@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <cover.1610986541.git.szabolcs.nagy@arm.com> References: <cover.1610986541.git.szabolcs.nagy@arm.com> Content-Type: text/plain X-Originating-IP: [217.140.106.54] X-ClientProxiedBy: SA0PR12CA0030.namprd12.prod.outlook.com (2603:10b6:806:6f::35) To PA4PR08MB6320.eurprd08.prod.outlook.com (2603:10a6:102:e5::9) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from localhost.localdomain (217.140.106.54) by SA0PR12CA0030.namprd12.prod.outlook.com (2603:10b6:806:6f::35) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3763.9 via Frontend Transport; Mon, 18 Jan 2021 16:25:38 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 0f748f70-bae7-4654-62f0-08d8bbcdc863 X-MS-TrafficTypeDiagnostic: PA4PR08MB6238:|VI1PR08MB3997: X-Microsoft-Antispam-PRVS: <VI1PR08MB39975A49F66954A25E7F5B4BEDA40@VI1PR08MB3997.eurprd08.prod.outlook.com> x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:10000;OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: KwEPsjn139mnnMBNcQwE1EHoikwisDMLUrGlxhJwxuoLbcbzPXJ0IdHTb4zkU5+sqXjphg2AqnyYMSkcLeiJs8JOx4nH/KuklEBrrnzafaSyN9IuJg86Vg7Vq3DBqnM7nF3FDpyOdwtlRufSv6lnX3xa+nIiCK8zWe0qmLicPcRgtrr9dix1HGvS8GnqV4M894WPp9wprsVaBCeaR59wxnJSUcqfytPXf8NXFZcGWJigVYJln6w25CdIN/u0cWxL0LKkLAhPiniLjZm4z7FflmW3dF569RxmXaI/p7qCd+du+6WvCxy4lLAV85FLQrRVtfcYieAg+ZFKDc5b9S/0PbhYT6rnXY/JOEDUNzs+JGy/mBKqVjpbpkr7qofMrB99JLRaACUjkxNiRk/CvaFCLBbDAca9hMFlnjxIXPidUQI4gK3cSvVfP/z2msPmn1MDsHL9YTdY4Dyk6SiqlsQzNpDmUUfcpicDStw1smJFxA0qMVXWGWX0ELMQQShPe5hXmNPCPsNsUba+M1prPuB1ts3oAsitIAlxidOtG5pJ+s374+afQVGzLASj7OdKJHwZNLJFNjPnpRuwc5U0TseABQ== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PA4PR08MB6320.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(39850400004)(376002)(396003)(136003)(346002)(83380400001)(6486002)(8676002)(69590400011)(478600001)(6666004)(52116002)(6916009)(316002)(8936002)(66476007)(66556008)(66946007)(5660300002)(86362001)(16526019)(26005)(6512007)(2906002)(186003)(6506007)(956004)(2616005)(36756003)(44832011); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData: BDfwOvgtqk2MzLjkYZY1I8Cwu+S/f5chtQXdqnc9DyEO1KVypO/PgtF91ERIgOtaHC3DofTqJ9b8PEgUitwJRF7ydtiq5/FQkruOs8WNEOeKFA/bJ3NYwyTZeZS5IcpXWk/o7MAYNMbW8bLsYDU8YIMV3N6IAkxcsWzLUmoka3R8e0PQtMYbeN0I5goiDsYo0ifD0NEzmH7DuimlTXrpv8+A3U7s+wrVMzHmXX8a+0WViM4DbKqTO8SFNMCxXJUHWTaINjt4ut7yz5eemirhWa5YbF3Xo7QQglzVx5uVgcQSG+nNOmI4/JtuvlWgzxzuLLOAEp4noMtvBO85UA+gLGR+8t3mLwlDSU9s3AT6MNyc5BJ9z4rDmDbENfkn/Gbpen/+7cfmA44aZslb/4cuf1SEX/xSvrwD/J0aqw9mY2q111m7+BevKI+J5faDNHGkIBvQrkJVVT/o2de/Q88i9XigVow2q1RfK/40V1PScUwMYQCCCLj+WMuFkSZApkKtPaeiCFdzRGSslsz8bMF2whePUKN7Am04VAtC5bDvgGepnjx9R+VxzggEJ2GmIA5bozBbz8FwbuV1wG73EVn/Rt1241l3FXsTMVJ7BTFNUsBYEYszr7kt6HI8aCiopsahozQnN1x0huRDhiG5RBa/q1i9Q4BnxjRJdviYU42sxmk5jnzTfMSKsYTUfNo1oe96xwr9so9tcYbE31DJbppZ/lglxkf+91xuCx7mhpD6UJjLnMZMf8wE0egDiXIcbvxxcWTPZvjSsuN5UyKw1xdCThSa1nYni+pE/O/0tbxAsB5+uHuUGuYlJIXQenOloUSMJxsMMH/4C/r0radT5m7t1OWRVpuT1tr9t8ckWDIRDTDTOEaXYmIFUVWzSnzaT+QdVr+dVxelZQnsFTezUN37cBUdl2q6wP9hM/XQU7i/gPXSCzAB9RI860a/OjO+1pNAZFN5ErtdVGpMYjLYJgDGJiojiXlXdVOP5B7WMakVliRYQox87EBVrneyVEFe1CCa X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR08MB6238 Original-Authentication-Results: sourceware.org; dkim=none (message not signed) header.d=none; sourceware.org; dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT025.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: 5e0829fe-2dfa-4099-c67a-08d8bbcdb165 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ifarzTZQmMCTt+Vy+1phlDci1ViChAypcx6UlKqGhtuFDkHK3g9msdjUVuWenipBpIsOVrNZFwlz6D4X6ysrTDxvK1WhRSwvDOOtEZ31SvbwLzwpjmvfiOIqjaw+X4Xn4MeUVW+O0SiNPezfUfbR+rwstDuTa3XbR9f4j1U1UMoxZT0QrGwWgEexCH34+JKq2yJzocoChiFUcmeFTJD1pkHoBacMVfC1t2u5YY7laInLtDVE3eZg7TiiJHSPs+WmYsfjvnIfRVUmzh6GS/azCQIDj+LAHx9OtBNlFIHnckkoWDFwy4z00SDQ61dcrCQfnu7DwZ9ugyj9H7oM4CEmfsXHsAj+M3sHsH3MzZwcA647pTA0q5HCYTCtzpr1FygrYh7jtg8SuXRkoIX7rqOwXbTybuyUcsG1A4pHn3QGzK82DEsIk2ZrTI/LvEgs5vOlG+BbB0N+iPGWj2fFpj/114hgPfXjyeFpGWju+GhOQBHsuUWN8TkD2UpIV7Mua29l X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFS:(4636009)(346002)(39860400002)(396003)(136003)(376002)(46966006)(70586007)(83380400001)(47076005)(8936002)(5660300002)(26005)(86362001)(69590400011)(70206006)(82310400003)(186003)(81166007)(34010700045)(356005)(336012)(16526019)(2906002)(316002)(478600001)(6666004)(6916009)(44832011)(2616005)(82740400003)(8676002)(6506007)(36756003)(956004)(6512007)(6486002); DIR:OUT; SFP:1501; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jan 2021 16:26:17.6761 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 0f748f70-bae7-4654-62f0-08d8bbcdc863 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT025.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB3997 X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, MSGID_FROM_MTA_HEADER, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP, UNPARSEABLE_RELAY 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 <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Szabolcs Nagy <szabolcs.nagy@arm.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
fix ifunc with static pie [BZ #27072]
|
|
Commit Message
Szabolcs Nagy
Jan. 18, 2021, 4:25 p.m. UTC
IFUNC resolvers may depend on tunables and cpu feature setup so move static pie self relocation after those. It is hard to guarantee that the ealy startup code does not rely on relocations so this is a bit fragile. It would be more robust to handle RELATIVE relocs early and only IRELATIVE relocs later, but the current relocation processing code cannot do that. The early startup code before relocation processing includes _dl_aux_init (auxvec); __libc_init_secure (); __tunables_init (__environ); ARCH_INIT_CPU_FEATURES (); These are simple enough that RELATIVE relocs can be avoided. __ehdr_start may require RELATIVE relocation so it was moved later, fortunately ehdr and phdr are not used in the early code. Fixes bug 27072. --- csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-)
Comments
On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: > IFUNC resolvers may depend on tunables and cpu feature setup so > move static pie self relocation after those. > > It is hard to guarantee that the ealy startup code does not rely > on relocations so this is a bit fragile. It would be more robust > to handle RELATIVE relocs early and only IRELATIVE relocs later, > but the current relocation processing code cannot do that. > > The early startup code before relocation processing includes > > _dl_aux_init (auxvec); > __libc_init_secure (); > __tunables_init (__environ); > ARCH_INIT_CPU_FEATURES (); > > These are simple enough that RELATIVE relocs can be avoided. > > __ehdr_start may require RELATIVE relocation so it was moved > later, fortunately ehdr and phdr are not used in the early code. > > Fixes bug 27072. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > 1 file changed, 25 insertions(+), 19 deletions(-) > > diff --git a/csu/libc-start.c b/csu/libc-start.c > index 1e90dcb0a7..c2b59431a3 100644 > --- a/csu/libc-start.c > +++ b/csu/libc-start.c > @@ -146,8 +146,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > int result; > > #ifndef SHARED > - _dl_relocate_static_pie (); > - > char **ev = &argv[argc + 1]; > > __environ = ev; Ok. > @@ -169,24 +167,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > } > # endif > _dl_aux_init (auxvec); > - if (GL(dl_phdr) == NULL) > # endif > - { > - /* Starting from binutils-2.23, the linker will define the > - magic symbol __ehdr_start to point to our own ELF header > - if it is visible in a segment that also includes the phdrs. > - So we can set up _dl_phdr and _dl_phnum even without any > - information from auxv. */ > - > - extern const ElfW(Ehdr) __ehdr_start > - __attribute__ ((weak, visibility ("hidden"))); > - if (&__ehdr_start != NULL) > - { > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > - GL(dl_phnum) = __ehdr_start.e_phnum; > - } > - } > > /* Initialize very early so that tunables can use it. */ > __libc_init_secure (); Ok. > @@ -195,6 +176,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > ARCH_INIT_CPU_FEATURES (); > > + /* Do static pie self relocation after tunables and cpu features > + are setup for ifunc resolvers. Before this point relocations > + must be avoided. */ > + _dl_relocate_static_pie (); > + > /* Perform IREL{,A} relocations. */ > ARCH_SETUP_IREL (); > Ok. > @@ -206,6 +192,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > hwcap and platform fields available in the TCB. */ > ARCH_APPLY_IREL (); > > +# ifdef HAVE_AUX_VECTOR > + if (GL(dl_phdr) == NULL) > +# endif > + { > + /* Starting from binutils-2.23, the linker will define the > + magic symbol __ehdr_start to point to our own ELF header > + if it is visible in a segment that also includes the phdrs. > + So we can set up _dl_phdr and _dl_phnum even without any > + information from auxv. */ > + > + extern const ElfW(Ehdr) __ehdr_start > + __attribute__ ((weak, visibility ("hidden"))); > + if (&__ehdr_start != NULL) > + { > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > + GL(dl_phnum) = __ehdr_start.e_phnum; > + } > + } > + > /* Set up the stack checker's canary. */ > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > # ifdef THREAD_SET_STACK_GUARD > Ok.
The 01/19/2021 11:07, Adhemerval Zanella wrote: > On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: > > IFUNC resolvers may depend on tunables and cpu feature setup so > > move static pie self relocation after those. > > > > It is hard to guarantee that the ealy startup code does not rely > > on relocations so this is a bit fragile. It would be more robust > > to handle RELATIVE relocs early and only IRELATIVE relocs later, > > but the current relocation processing code cannot do that. > > > > The early startup code before relocation processing includes > > > > _dl_aux_init (auxvec); > > __libc_init_secure (); > > __tunables_init (__environ); > > ARCH_INIT_CPU_FEATURES (); > > > > These are simple enough that RELATIVE relocs can be avoided. > > > > __ehdr_start may require RELATIVE relocation so it was moved > > later, fortunately ehdr and phdr are not used in the early code. > > > > Fixes bug 27072. > > LGTM, thanks. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> sigh, this is an old version of this patch, i made a mistake putting the series together. the problem is that _dl_phdr is used in ARCH_SETUP_TLS (to get the tls program headers) so the __ehdr_start magic should be before that (this only matters if auxv lacks AT_PHDR for some reason, which should not happen normally on linux, so testing won't show the problem) i'm trying to figure out where i lost the new version.. thanks for the reviews. > > > --- > > csu/libc-start.c | 44 +++++++++++++++++++++++++------------------- > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > index 1e90dcb0a7..c2b59431a3 100644 > > --- a/csu/libc-start.c > > +++ b/csu/libc-start.c > > @@ -146,8 +146,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > int result; > > > > #ifndef SHARED > > - _dl_relocate_static_pie (); > > - > > char **ev = &argv[argc + 1]; > > > > __environ = ev; > Ok. > > > > @@ -169,24 +167,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > } > > # endif > > _dl_aux_init (auxvec); > > - if (GL(dl_phdr) == NULL) > > # endif > > - { > > - /* Starting from binutils-2.23, the linker will define the > > - magic symbol __ehdr_start to point to our own ELF header > > - if it is visible in a segment that also includes the phdrs. > > - So we can set up _dl_phdr and _dl_phnum even without any > > - information from auxv. */ > > - > > - extern const ElfW(Ehdr) __ehdr_start > > - __attribute__ ((weak, visibility ("hidden"))); > > - if (&__ehdr_start != NULL) > > - { > > - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > - GL(dl_phnum) = __ehdr_start.e_phnum; > > - } > > - } > > > > /* Initialize very early so that tunables can use it. */ > > __libc_init_secure (); > > Ok. > > > @@ -195,6 +176,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > > > ARCH_INIT_CPU_FEATURES (); > > > > + /* Do static pie self relocation after tunables and cpu features > > + are setup for ifunc resolvers. Before this point relocations > > + must be avoided. */ > > + _dl_relocate_static_pie (); > > + > > /* Perform IREL{,A} relocations. */ > > ARCH_SETUP_IREL (); > > > > Ok. > > > @@ -206,6 +192,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > hwcap and platform fields available in the TCB. */ > > ARCH_APPLY_IREL (); > > > > +# ifdef HAVE_AUX_VECTOR > > + if (GL(dl_phdr) == NULL) > > +# endif > > + { > > + /* Starting from binutils-2.23, the linker will define the > > + magic symbol __ehdr_start to point to our own ELF header > > + if it is visible in a segment that also includes the phdrs. > > + So we can set up _dl_phdr and _dl_phnum even without any > > + information from auxv. */ > > + > > + extern const ElfW(Ehdr) __ehdr_start > > + __attribute__ ((weak, visibility ("hidden"))); > > + if (&__ehdr_start != NULL) > > + { > > + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); > > + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; > > + GL(dl_phnum) = __ehdr_start.e_phnum; > > + } > > + } > > + > > /* Set up the stack checker's canary. */ > > uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); > > # ifdef THREAD_SET_STACK_GUARD > > > > Ok.
On 19/01/2021 11:35, Szabolcs Nagy wrote: > The 01/19/2021 11:07, Adhemerval Zanella wrote: >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: >>> IFUNC resolvers may depend on tunables and cpu feature setup so >>> move static pie self relocation after those. >>> >>> It is hard to guarantee that the ealy startup code does not rely >>> on relocations so this is a bit fragile. It would be more robust >>> to handle RELATIVE relocs early and only IRELATIVE relocs later, >>> but the current relocation processing code cannot do that. >>> >>> The early startup code before relocation processing includes >>> >>> _dl_aux_init (auxvec); >>> __libc_init_secure (); >>> __tunables_init (__environ); >>> ARCH_INIT_CPU_FEATURES (); >>> >>> These are simple enough that RELATIVE relocs can be avoided. >>> >>> __ehdr_start may require RELATIVE relocation so it was moved >>> later, fortunately ehdr and phdr are not used in the early code. >>> >>> Fixes bug 27072. >> >> LGTM, thanks. >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > sigh, this is an old version of this patch, i made a > mistake putting the series together. > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > (to get the tls program headers) so the __ehdr_start > magic should be before that (this only matters if auxv > lacks AT_PHDR for some reason, which should not happen > normally on linux, so testing won't show the problem) By normally do you mean it might happen on a specific kernel version or is it architecture specific?
On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > The 01/19/2021 11:07, Adhemerval Zanella wrote: > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: > >>> IFUNC resolvers may depend on tunables and cpu feature setup so > >>> move static pie self relocation after those. > >>> > >>> It is hard to guarantee that the ealy startup code does not rely > >>> on relocations so this is a bit fragile. It would be more robust > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later, > >>> but the current relocation processing code cannot do that. > >>> > >>> The early startup code before relocation processing includes > >>> > >>> _dl_aux_init (auxvec); > >>> __libc_init_secure (); > >>> __tunables_init (__environ); > >>> ARCH_INIT_CPU_FEATURES (); > >>> > >>> These are simple enough that RELATIVE relocs can be avoided. > >>> > >>> __ehdr_start may require RELATIVE relocation so it was moved > >>> later, fortunately ehdr and phdr are not used in the early code. > >>> > >>> Fixes bug 27072. > >> > >> LGTM, thanks. > >> > >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > > > > sigh, this is an old version of this patch, i made a > > mistake putting the series together. > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > (to get the tls program headers) so the __ehdr_start > > magic should be before that (this only matters if auxv > > lacks AT_PHDR for some reason, which should not happen > > normally on linux, so testing won't show the problem) > > By normally do you mean it might happen on a specific kernel version > or is it architecture specific? I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE relocation. I verified it by adding -Wl,-z,report-relative-reloc when building elf/sln on x86.
The 01/19/2021 06:48, H.J. Lu wrote: > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > <libc-alpha@sourceware.org> wrote: > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > The 01/19/2021 11:07, Adhemerval Zanella wrote: > > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: > > >>> IFUNC resolvers may depend on tunables and cpu feature setup so > > >>> move static pie self relocation after those. > > >>> > > >>> It is hard to guarantee that the ealy startup code does not rely > > >>> on relocations so this is a bit fragile. It would be more robust > > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later, > > >>> but the current relocation processing code cannot do that. > > >>> > > >>> The early startup code before relocation processing includes > > >>> > > >>> _dl_aux_init (auxvec); > > >>> __libc_init_secure (); > > >>> __tunables_init (__environ); > > >>> ARCH_INIT_CPU_FEATURES (); > > >>> > > >>> These are simple enough that RELATIVE relocs can be avoided. > > >>> > > >>> __ehdr_start may require RELATIVE relocation so it was moved > > >>> later, fortunately ehdr and phdr are not used in the early code. > > >>> > > >>> Fixes bug 27072. > > >> > > >> LGTM, thanks. > > >> > > >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > > > > > > > sigh, this is an old version of this patch, i made a > > > mistake putting the series together. > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > (to get the tls program headers) so the __ehdr_start > > > magic should be before that (this only matters if auxv > > > lacks AT_PHDR for some reason, which should not happen > > > normally on linux, so testing won't show the problem) > > > > By normally do you mean it might happen on a specific kernel version > > or is it architecture specific? i guess __ehdr_start symbol can be useful and with it glibc does not have to depend on auxv (which an elf loader like valgrind/qemu-user may get wrong) however it is only used as a fallback and on linux AT_PHDR is always expected to be present. (i don't know if this ever triggers) > > I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE > relocation. I verified it by adding -Wl,-z,report-relative-reloc when building > elf/sln on x86. it needs relative reloc on aarch64: it can be an undefined weak symbol and that must be 0. a pc relative address computation cannot give 0 (unless linker does some instruction rewriting, but on aarch64 the address computation is multiple instructions that can be spread far apart). so yeah it needs a GOT entry and that will be either 0 or needs a RELATIVE reloc.
On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/19/2021 06:48, H.J. Lu wrote: > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > The 01/19/2021 11:07, Adhemerval Zanella wrote: > > > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: > > > >>> IFUNC resolvers may depend on tunables and cpu feature setup so > > > >>> move static pie self relocation after those. > > > >>> > > > >>> It is hard to guarantee that the ealy startup code does not rely > > > >>> on relocations so this is a bit fragile. It would be more robust > > > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later, > > > >>> but the current relocation processing code cannot do that. > > > >>> > > > >>> The early startup code before relocation processing includes > > > >>> > > > >>> _dl_aux_init (auxvec); > > > >>> __libc_init_secure (); > > > >>> __tunables_init (__environ); > > > >>> ARCH_INIT_CPU_FEATURES (); > > > >>> > > > >>> These are simple enough that RELATIVE relocs can be avoided. > > > >>> > > > >>> __ehdr_start may require RELATIVE relocation so it was moved > > > >>> later, fortunately ehdr and phdr are not used in the early code. > > > >>> > > > >>> Fixes bug 27072. > > > >> > > > >> LGTM, thanks. > > > >> > > > >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > > > > > > > > > > sigh, this is an old version of this patch, i made a > > > > mistake putting the series together. > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > (to get the tls program headers) so the __ehdr_start > > > > magic should be before that (this only matters if auxv > > > > lacks AT_PHDR for some reason, which should not happen > > > > normally on linux, so testing won't show the problem) > > > > > > By normally do you mean it might happen on a specific kernel version > > > or is it architecture specific? > > i guess __ehdr_start symbol can be useful and with it > glibc does not have to depend on auxv (which an elf > loader like valgrind/qemu-user may get wrong) > > however it is only used as a fallback and on linux > AT_PHDR is always expected to be present. (i don't > know if this ever triggers) Only used on Hurd? > > > > I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE > > relocation. I verified it by adding -Wl,-z,report-relative-reloc when building > > elf/sln on x86. > > it needs relative reloc on aarch64: it can be an undefined weak > symbol and that must be 0. a pc relative address computation > cannot give 0 (unless linker does some instruction rewriting, > but on aarch64 the address computation is multiple instructions > that can be spread far apart). so yeah it needs a GOT entry and > that will be either 0 or needs a RELATIVE reloc. On x86, I converted load from GOT to simple LEA without RELATIVE in this case. But this is an x86 specific optimization.
On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > The 01/19/2021 11:07, Adhemerval Zanella wrote: > > > > >> On 18/01/2021 13:25, Szabolcs Nagy via Libc-alpha wrote: > > > > >>> IFUNC resolvers may depend on tunables and cpu feature setup so > > > > >>> move static pie self relocation after those. > > > > >>> > > > > >>> It is hard to guarantee that the ealy startup code does not rely > > > > >>> on relocations so this is a bit fragile. It would be more robust > > > > >>> to handle RELATIVE relocs early and only IRELATIVE relocs later, > > > > >>> but the current relocation processing code cannot do that. > > > > >>> > > > > >>> The early startup code before relocation processing includes > > > > >>> > > > > >>> _dl_aux_init (auxvec); > > > > >>> __libc_init_secure (); > > > > >>> __tunables_init (__environ); > > > > >>> ARCH_INIT_CPU_FEATURES (); > > > > >>> > > > > >>> These are simple enough that RELATIVE relocs can be avoided. > > > > >>> > > > > >>> __ehdr_start may require RELATIVE relocation so it was moved > > > > >>> later, fortunately ehdr and phdr are not used in the early code. > > > > >>> > > > > >>> Fixes bug 27072. > > > > >> > > > > >> LGTM, thanks. > > > > >> > > > > >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > > > > > > > > > > > > > sigh, this is an old version of this patch, i made a > > > > > mistake putting the series together. > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > (to get the tls program headers) so the __ehdr_start > > > > > magic should be before that (this only matters if auxv > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > normally on linux, so testing won't show the problem) > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > or is it architecture specific? > > > > i guess __ehdr_start symbol can be useful and with it > > glibc does not have to depend on auxv (which an elf > > loader like valgrind/qemu-user may get wrong) > > > > however it is only used as a fallback and on linux > > AT_PHDR is always expected to be present. (i don't > > know if this ever triggers) > > Only used on Hurd? Does arm64 linker always define __ehdr_start? If yes, can you drop "weak," to see if RELATIVE goes away? > > > > > > I think we can leave __ehdr_start ASIS since it doesn't need RELATIVE > > > relocation. I verified it by adding -Wl,-z,report-relative-reloc when building > > > elf/sln on x86. > > > > it needs relative reloc on aarch64: it can be an undefined weak > > symbol and that must be 0. a pc relative address computation > > cannot give 0 (unless linker does some instruction rewriting, > > but on aarch64 the address computation is multiple instructions > > that can be spread far apart). so yeah it needs a GOT entry and > > that will be either 0 or needs a RELATIVE reloc. > > On x86, I converted load from GOT to simple LEA without RELATIVE > in this case. But this is an x86 specific optimization. > > -- > H.J.
The 01/19/2021 08:47, H.J. Lu wrote: > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > <libc-alpha@sourceware.org> wrote: > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > magic should be before that (this only matters if auxv > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > or is it architecture specific? > > > > > > i guess __ehdr_start symbol can be useful and with it > > > glibc does not have to depend on auxv (which an elf > > > loader like valgrind/qemu-user may get wrong) > > > > > > however it is only used as a fallback and on linux > > > AT_PHDR is always expected to be present. (i don't > > > know if this ever triggers) > > > > Only used on Hurd? > > Does arm64 linker always define __ehdr_start? If yes, can you drop > "weak," to see if RELATIVE goes away? __ehdr_start support was added in binutils 2.23 so i guess all supported binutils has it which means we can make it non-weak indeed. good idea. (we can also ignore auxv and rely on __ehdr_start only, but for now just making it non-weak should be fine.)
On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/19/2021 08:47, H.J. Lu wrote: > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > magic should be before that (this only matters if auxv > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > or is it architecture specific? > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > glibc does not have to depend on auxv (which an elf > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > however it is only used as a fallback and on linux > > > > AT_PHDR is always expected to be present. (i don't > > > > know if this ever triggers) > > > > > > Only used on Hurd? > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > "weak," to see if RELATIVE goes away? > > __ehdr_start support was added in binutils 2.23 We may assume binutils >= 2.33 when building for static PIE since all static PIE linkers should define __ehdr_start. Does lld define __ehdr_start? > so i guess all supported binutils has it which > means we can make it non-weak indeed. > > good idea. > > (we can also ignore auxv and rely on __ehdr_start only, > but for now just making it non-weak should be fine.) >
On Tue, Jan 19, 2021 at 9:11 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > or is it architecture specific? > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > glibc does not have to depend on auxv (which an elf > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > however it is only used as a fallback and on linux > > > > > AT_PHDR is always expected to be present. (i don't > > > > > know if this ever triggers) > > > > > > > > Only used on Hurd? > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > "weak," to see if RELATIVE goes away? > > > > __ehdr_start support was added in binutils 2.23 > > We may assume binutils >= 2.33 when building for static PIE > since all static PIE linkers should define __ehdr_start. > > Does lld define __ehdr_start? LLD defines __ehdr_start as hidden if it is not a defined symbol in -no-pie/-pie/-shared modes. The behavior seems to match gold. GNU ld seems to use STB_LOCAL STV_DEFAULT but the exterior behavior should be the same. > > so i guess all supported binutils has it which > > means we can make it non-weak indeed. > > > > good idea. > > > > (we can also ignore auxv and rely on __ehdr_start only, > > but for now just making it non-weak should be fine.) > > > > > -- > H.J.
On Tue, Jan 19, 2021 at 9:26 AM Fāng-ruì Sòng <maskray@google.com> wrote: > > On Tue, Jan 19, 2021 at 9:11 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > > or is it architecture specific? > > > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > > glibc does not have to depend on auxv (which an elf > > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > > > however it is only used as a fallback and on linux > > > > > > AT_PHDR is always expected to be present. (i don't > > > > > > know if this ever triggers) > > > > > > > > > > Only used on Hurd? > > > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > > "weak," to see if RELATIVE goes away? > > > > > > __ehdr_start support was added in binutils 2.23 > > > > We may assume binutils >= 2.33 when building for static PIE > > since all static PIE linkers should define __ehdr_start. > > > > Does lld define __ehdr_start? > > LLD defines __ehdr_start as hidden if it is not a defined symbol in > -no-pie/-pie/-shared modes. > The behavior seems to match gold. GNU ld seems to use STB_LOCAL > STV_DEFAULT but the exterior behavior should be the same. I am not sure where you got such incorrect info. Both ld and gold were changed to STV_HIDDEN in 2013 by the same commit: ommit cde7cb0129a884a060b99c7c83e8f5c9af728b0a Author: Maciej W. Rozycki <macro@linux-mips.org> Date: Fri May 3 15:19:27 2013 +0000 gold/ PR ld/15365 * layout.cc (Layout::finalize): Make __ehdr_start STV_HIDDEN. ld/ PR ld/15365 * emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation): Restrict __ehdr_start's export class to no less than STV_HIDDEN.
The 01/19/2021 09:10, H.J. Lu wrote: > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > or is it architecture specific? > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > glibc does not have to depend on auxv (which an elf > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > however it is only used as a fallback and on linux > > > > > AT_PHDR is always expected to be present. (i don't > > > > > know if this ever triggers) > > > > > > > > Only used on Hurd? > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > "weak," to see if RELATIVE goes away? > > > > __ehdr_start support was added in binutils 2.23 > > We may assume binutils >= 2.33 when building for static PIE > since all static PIE linkers should define __ehdr_start. this piece of code is used for both static PIE and non-PIE, but we already require binutils >= 2.25 for building glibc, dropping weak should be fine. i will resend the series with this. > > Does lld define __ehdr_start? > > > so i guess all supported binutils has it which > > means we can make it non-weak indeed. > > > > good idea. > > > > (we can also ignore auxv and rely on __ehdr_start only, > > but for now just making it non-weak should be fine.) > > > > > -- > H.J.
On Tue, Jan 19, 2021 at 9:34 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 19, 2021 at 9:26 AM Fāng-ruì Sòng <maskray@google.com> wrote: > > > > On Tue, Jan 19, 2021 at 9:11 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > > > or is it architecture specific? > > > > > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > > > glibc does not have to depend on auxv (which an elf > > > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > > > > > however it is only used as a fallback and on linux > > > > > > > AT_PHDR is always expected to be present. (i don't > > > > > > > know if this ever triggers) > > > > > > > > > > > > Only used on Hurd? > > > > > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > > > "weak," to see if RELATIVE goes away? > > > > > > > > __ehdr_start support was added in binutils 2.23 > > > > > > We may assume binutils >= 2.33 when building for static PIE > > > since all static PIE linkers should define __ehdr_start. > > > > > > Does lld define __ehdr_start? > > > > LLD defines __ehdr_start as hidden if it is not a defined symbol in > > -no-pie/-pie/-shared modes. > > The behavior seems to match gold. GNU ld seems to use STB_LOCAL > > STV_DEFAULT but the exterior behavior should be the same. > > I am not sure where you got such incorrect info. Both ld and gold were changed > to STV_HIDDEN in 2013 by the same commit: > > ommit cde7cb0129a884a060b99c7c83e8f5c9af728b0a > Author: Maciej W. Rozycki <macro@linux-mips.org> > Date: Fri May 3 15:19:27 2013 +0000 > > gold/ > PR ld/15365 > * layout.cc (Layout::finalize): Make __ehdr_start STV_HIDDEN. > > ld/ > PR ld/15365 > * emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation): > Restrict __ehdr_start's export class to no less than STV_HIDDEN. > > -- > H.J. % cat a.s .global _start, __ehdr_start _start: leaq __ehdr_start(%rip), %rax % gcc -fuse-ld=bfd -nostdlib a.s -o a.so % readelf -Ws a.so ... 9: 0000000000000000 0 NOTYPE LOCAL DEFAULT 1 __ehdr_start When the binding is STB_LOCAL, the visibility does not really matter. ELF spec: "A symbol with STB_LOCAL binding may not have STV_PROTECTED visibility. 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."
On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/19/2021 09:10, H.J. Lu wrote: > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > > or is it architecture specific? > > > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > > glibc does not have to depend on auxv (which an elf > > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > > > however it is only used as a fallback and on linux > > > > > > AT_PHDR is always expected to be present. (i don't > > > > > > know if this ever triggers) > > > > > > > > > > Only used on Hurd? > > > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > > "weak," to see if RELATIVE goes away? > > > > > > __ehdr_start support was added in binutils 2.23 > > > > We may assume binutils >= 2.33 when building for static PIE > > since all static PIE linkers should define __ehdr_start. > > this piece of code is used for both static PIE and non-PIE, > but we already require binutils >= 2.25 for building glibc, > dropping weak should be fine. > It is safer to check BUILD_PIE_DEFAULT when dropping weak.
The 01/19/2021 09:42, H.J. Lu wrote: > On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/19/2021 09:10, H.J. Lu wrote: > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > > > or is it architecture specific? > > > > > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > > > glibc does not have to depend on auxv (which an elf > > > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > > > > > however it is only used as a fallback and on linux > > > > > > > AT_PHDR is always expected to be present. (i don't > > > > > > > know if this ever triggers) > > > > > > > > > > > > Only used on Hurd? > > > > > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > > > "weak," to see if RELATIVE goes away? > > > > > > > > __ehdr_start support was added in binutils 2.23 > > > > > > We may assume binutils >= 2.33 when building for static PIE > > > since all static PIE linkers should define __ehdr_start. > > > > this piece of code is used for both static PIE and non-PIE, > > but we already require binutils >= 2.25 for building glibc, > > dropping weak should be fine. > > > > It is safer to check BUILD_PIE_DEFAULT when dropping > weak. ok. does static linking have weaker linker version requirement than building glibc?
On Tue, Jan 19, 2021 at 9:47 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/19/2021 09:42, H.J. Lu wrote: > > On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > The 01/19/2021 09:10, H.J. Lu wrote: > > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > > > > or is it architecture specific? > > > > > > > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > > > > glibc does not have to depend on auxv (which an elf > > > > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > > > > > > > however it is only used as a fallback and on linux > > > > > > > > AT_PHDR is always expected to be present. (i don't > > > > > > > > know if this ever triggers) > > > > > > > > > > > > > > Only used on Hurd? > > > > > > > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > > > > "weak," to see if RELATIVE goes away? > > > > > > > > > > __ehdr_start support was added in binutils 2.23 > > > > > > > > We may assume binutils >= 2.33 when building for static PIE > > > > since all static PIE linkers should define __ehdr_start. > > > > > > this piece of code is used for both static PIE and non-PIE, > > > but we already require binutils >= 2.25 for building glibc, > > > dropping weak should be fine. > > > > > > > It is safer to check BUILD_PIE_DEFAULT when dropping > > weak. > > ok. > > does static linking have weaker linker version requirement > than building glibc? Very unlikely. But one may be forced to use the older linker for some reason.
On Tue, Jan 19, 2021 at 9:53 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 19, 2021 at 9:47 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > The 01/19/2021 09:42, H.J. Lu wrote: > > > On Tue, Jan 19, 2021 at 9:38 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > The 01/19/2021 09:10, H.J. Lu wrote: > > > > > On Tue, Jan 19, 2021 at 9:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > > > > > The 01/19/2021 08:47, H.J. Lu wrote: > > > > > > > On Tue, Jan 19, 2021 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > On Tue, Jan 19, 2021 at 7:24 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > > > > > > The 01/19/2021 06:48, H.J. Lu wrote: > > > > > > > > > > On Tue, Jan 19, 2021 at 6:37 AM Adhemerval Zanella via Libc-alpha > > > > > > > > > > <libc-alpha@sourceware.org> wrote: > > > > > > > > > > > On 19/01/2021 11:35, Szabolcs Nagy wrote: > > > > > > > > > > > > the problem is that _dl_phdr is used in ARCH_SETUP_TLS > > > > > > > > > > > > (to get the tls program headers) so the __ehdr_start > > > > > > > > > > > > magic should be before that (this only matters if auxv > > > > > > > > > > > > lacks AT_PHDR for some reason, which should not happen > > > > > > > > > > > > normally on linux, so testing won't show the problem) > > > > > > > > > > > > > > > > > > > > > > By normally do you mean it might happen on a specific kernel version > > > > > > > > > > > or is it architecture specific? > > > > > > > > > > > > > > > > > > i guess __ehdr_start symbol can be useful and with it > > > > > > > > > glibc does not have to depend on auxv (which an elf > > > > > > > > > loader like valgrind/qemu-user may get wrong) > > > > > > > > > > > > > > > > > > however it is only used as a fallback and on linux > > > > > > > > > AT_PHDR is always expected to be present. (i don't > > > > > > > > > know if this ever triggers) > > > > > > > > > > > > > > > > Only used on Hurd? > > > > > > > > > > > > > > Does arm64 linker always define __ehdr_start? If yes, can you drop > > > > > > > "weak," to see if RELATIVE goes away? > > > > > > > > > > > > __ehdr_start support was added in binutils 2.23 > > > > > > > > > > We may assume binutils >= 2.33 when building for static PIE > > > > > since all static PIE linkers should define __ehdr_start. > > > > > > > > this piece of code is used for both static PIE and non-PIE, > > > > but we already require binutils >= 2.25 for building glibc, > > > > dropping weak should be fine. > > > > > > > > > > It is safer to check BUILD_PIE_DEFAULT when dropping > > > weak. > > > > ok. > > > > does static linking have weaker linker version requirement > > than building glibc? > > Very unlikely. But one may be forced to use the older linker > for some reason. > BTW, I pushed commit 22b79ed7f413cd980a7af0cf258da5bf82b6d5e5 Author: H.J. Lu <hjl.tools@gmail.com> Date: Fri Jan 15 06:46:12 2021 -0800 Use <startup.h> in __libc_init_secure to master. You need to rebase.
diff --git a/csu/libc-start.c b/csu/libc-start.c index 1e90dcb0a7..c2b59431a3 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -146,8 +146,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), int result; #ifndef SHARED - _dl_relocate_static_pie (); - char **ev = &argv[argc + 1]; __environ = ev; @@ -169,24 +167,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), } # endif _dl_aux_init (auxvec); - if (GL(dl_phdr) == NULL) # endif - { - /* Starting from binutils-2.23, the linker will define the - magic symbol __ehdr_start to point to our own ELF header - if it is visible in a segment that also includes the phdrs. - So we can set up _dl_phdr and _dl_phnum even without any - information from auxv. */ - - extern const ElfW(Ehdr) __ehdr_start - __attribute__ ((weak, visibility ("hidden"))); - if (&__ehdr_start != NULL) - { - assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); - GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; - GL(dl_phnum) = __ehdr_start.e_phnum; - } - } /* Initialize very early so that tunables can use it. */ __libc_init_secure (); @@ -195,6 +176,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), ARCH_INIT_CPU_FEATURES (); + /* Do static pie self relocation after tunables and cpu features + are setup for ifunc resolvers. Before this point relocations + must be avoided. */ + _dl_relocate_static_pie (); + /* Perform IREL{,A} relocations. */ ARCH_SETUP_IREL (); @@ -206,6 +192,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), hwcap and platform fields available in the TCB. */ ARCH_APPLY_IREL (); +# ifdef HAVE_AUX_VECTOR + if (GL(dl_phdr) == NULL) +# endif + { + /* Starting from binutils-2.23, the linker will define the + magic symbol __ehdr_start to point to our own ELF header + if it is visible in a segment that also includes the phdrs. + So we can set up _dl_phdr and _dl_phnum even without any + information from auxv. */ + + extern const ElfW(Ehdr) __ehdr_start + __attribute__ ((weak, visibility ("hidden"))); + if (&__ehdr_start != NULL) + { + assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr)); + GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff; + GL(dl_phnum) = __ehdr_start.e_phnum; + } + } + /* Set up the stack checker's canary. */ uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); # ifdef THREAD_SET_STACK_GUARD