Message ID | 7b008fd34f802456db3731a043ff56683b569ff7.1604393169.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 49675398303D; Tue, 3 Nov 2020 10:27:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 49675398303D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1604399235; bh=b2LGHS8npIN3aGe1xT1kK15btQCbisIo7d+nrOmvXU0=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=wCl4vNpp9QRnJkGi/b4tm+n3RRImxeBeF88GlfT13sZEAXprbJWnd+iL3oZ8w/iut Qya5MM2Ml0DbDzBBY6Pa/jBN7oJgAd2+cgR8Zcbxso714y7Ek0JvDjV+qkXMYVTMt7 j0IZm5Ecwot+lSe6M/atyNr+kTqk3PAxSVc1fVlc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2074.outbound.protection.outlook.com [40.107.20.74]) by sourceware.org (Postfix) with ESMTPS id AF3DE3983011 for <libc-alpha@sourceware.org>; Tue, 3 Nov 2020 10:27:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AF3DE3983011 Received: from AM6PR08CA0006.eurprd08.prod.outlook.com (2603:10a6:20b:b2::18) by AM4PR0802MB2290.eurprd08.prod.outlook.com (2603:10a6:200:5c::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.27; Tue, 3 Nov 2020 10:27:10 +0000 Received: from AM5EUR03FT018.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:b2:cafe::ca) by AM6PR08CA0006.outlook.office365.com (2603:10a6:20b:b2::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18 via Frontend Transport; Tue, 3 Nov 2020 10:27:10 +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 AM5EUR03FT018.mail.protection.outlook.com (10.152.16.114) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3520.15 via Frontend Transport; Tue, 3 Nov 2020 10:27:10 +0000 Received: ("Tessian outbound c189680f801b:v64"); Tue, 03 Nov 2020 10:27:09 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: e0d9a8a02e872d3c X-CR-MTA-TID: 64aa7808 Received: from f565e3fc67ad.3 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 02DAA500-AAB5-4231-B5C5-DC82B94A7B7D.1; Tue, 03 Nov 2020 10:26:36 +0000 Received: from EUR02-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id f565e3fc67ad.3 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Tue, 03 Nov 2020 10:26:36 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FtiHxDxxfzlr3GOl7v7ohQxZsttt8jyMnW2iPZo+swkeyJLYuABDoPECRMJvMxx+b8D3VfhSi0VVAVHcpePw8ts/FARFieIdXAdyxXaK2BGFXaHr40DzyZn0/AT7MtOJ0yOM5c8Jx5sTwyB+ZfNJzORBnyWQCZIeN0tpfbbv/Dj873e8fSD4rwlUn7WvzL2t3DtJ9nmToXLWlZ2xxtXOYdvaXHTjInLXf7Tr+VgFqH61EMR7VAc+P+HAmtFCE2ewlrrH7WMIOi/XTEOely+TuVWxH5/0kE6KqUSAqRERaf+6Hl0DFbbvqi0PvVnPH1JO0DvmV6WggYs3dwv65MvIvw== 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=b2LGHS8npIN3aGe1xT1kK15btQCbisIo7d+nrOmvXU0=; b=JXLlGTTIQsLfRgj2xwkObOaTy3ON/PNARxRB1jdYLzxVJbpIsnUT0aRf5WGeZ7BKuqPg3i2I9WvDs+wVg3ZL/zpacIEct7RAJb8UuI/oBCJ8A7U4f6RmPqX8iyWt50Ml8TLNQjm1p/osQ4QcGQMaUrEK2V0UMcM2gwOjkpAXy3N2wQaLLsvxE/u4FJPS/09gyJP1Yy5IeuSR6CwYC1TO53qcSrfeKXNkgpNJNkmE911BtRX78JFDG6rjnUVl7IA8UiWC3X0THxWqVKNwx9akmgTAz9nFr7KBWclV/EY6ut1Ot0ygBTo1GoFzricQdfwVP2W2djNWKDKfjBV9dETc1w== 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 PR3PR08MB5564.eurprd08.prod.outlook.com (2603:10a6:102:87::18) by PA4PR08MB6285.eurprd08.prod.outlook.com (2603:10a6:102:e0::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18; Tue, 3 Nov 2020 10:26:24 +0000 Received: from PR3PR08MB5564.eurprd08.prod.outlook.com ([fe80::2904:edcf:b299:f792]) by PR3PR08MB5564.eurprd08.prod.outlook.com ([fe80::2904:edcf:b299:f792%3]) with mapi id 15.20.3499.032; Tue, 3 Nov 2020 10:26:24 +0000 To: libc-alpha@sourceware.org Subject: [PATCH 2/4] elf: Move note processing after l_phdr is updated [BZ #26831] Date: Tue, 3 Nov 2020 10:26:18 +0000 Message-Id: <7b008fd34f802456db3731a043ff56683b569ff7.1604393169.git.szabolcs.nagy@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <cover.1604393169.git.szabolcs.nagy@arm.com> References: <cover.1604393169.git.szabolcs.nagy@arm.com> Content-Type: text/plain X-Originating-IP: [217.140.106.54] X-ClientProxiedBy: LO2P265CA0022.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:62::34) To PR3PR08MB5564.eurprd08.prod.outlook.com (2603:10a6:102:87::18) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from localhost.localdomain (217.140.106.54) by LO2P265CA0022.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:62::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18 via Frontend Transport; Tue, 3 Nov 2020 10:26:23 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: a440b8b5-2a39-43ce-e822-08d87fe305a4 X-MS-TrafficTypeDiagnostic: PA4PR08MB6285:|AM4PR0802MB2290: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: <AM4PR0802MB22909A65E5B694C55AEAF4ADED110@AM4PR0802MB2290.eurprd08.prod.outlook.com> x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:7691;OLM:7691; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: mlsiUh61bx5Crih3vA2Z4JW+a2wt3QsdhbSpnZqOyjqsM63KVmxW5XOr1X2TO3Dg1A9F/A/uNYels5NAdSLyJluaktIENrR8gRfaBbqFCjaXvg3Pr5iC9e1MFZzuDweG77qkYBvFpBgkhUuhUgmBaXdZVVG72PYO/tGE/mH+Ylhfe1YQcx1NTEsXSEGjFF5W65uAE3NjKpyWd2AY/T5lqqC+hdmimbYhOzU+uGgyPPFHS5MpKCl/vzUZ0TL1VGoR2vL2LZ+gu7viXGw9n3Dv1bx4tMYcrBFXB8lM92XEKjgA/g7mC2kQzxq4N7Swh6qgb3PwXYuRYp6ztJKEOlrv52Fke7/qLOZNWoPac2G0n1Gf34OgqdO8G6whwM1MgiDG X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PR3PR08MB5564.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(376002)(346002)(39860400002)(136003)(396003)(366004)(6506007)(36756003)(186003)(54906003)(478600001)(8936002)(6916009)(52116002)(316002)(4326008)(16526019)(26005)(69590400008)(86362001)(5660300002)(2616005)(44832011)(7416002)(956004)(8676002)(66476007)(66556008)(6512007)(83380400001)(6486002)(2906002)(6666004)(66946007); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData: m4/1TbY0scBRHWKeDmS23Ck/0jdSOWYCIpsJxujIy6Qhy5erw+FUqpuGY6k6Fqu8Z3r7jDCYMIhiJMlAZujBIULCskECo5/w5OxdqeHez9ZOFIrOWgJ1WcLJ4UxkK/8pHeE+HXr5xX4gJuoyOfhMNksHGc7kq2kENAuW+hThVJVx9HtsNa2V5MwT7m8I9JB03lS5o2ch7TM8TeYANislqEWiGxevhdEflqSTa7qqm9XgyjbFFMo29TZO/UG7cqoyXB08Q2nLLNqMIKdTyRQdpBAlyL1n1mCi3gxvXc1/cXmx8j5aBJHvFCuNrxAMqws4FAyHERpLdAXNtPwSGOAezm9WpTnuA0nawC/SWVR/fBLVt9BobCUCEK2wyqKDG5z0xbHWFcpLFOMfweXNoV3vkO1JCQHad2kbnmqTmFX03jyzKBC/2LK7X/EOuHXEk2zFoHWVPAxy99VHwM5i0j45YmA9m82ZdYmbxOLdWvlBtPa/SbuGJAiRhyxImq2X6bmkMHNjKG6O7m3Q/n0hMX5vsA/xIERm9cSk8gWUKQbaMhDEtn2Y9oBCzg0iqOWlyr9K3dDoAgG09PDcMG+uaMtq+wyGDP3FlBoj9DXHYvTRgFBTITKiyRAKR0lR7JoRnTYz7i//qDcF3Vt6153BFqUFcg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR08MB6285 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: AM5EUR03FT018.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: a88b4fed-4a14-4294-4b4f-08d87fe2ea23 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: WOzvcEnX48LtOBg64MlkkuAvFrEKjUvrJTtTDW/3POtIurSvyppoCq6SHzL8Xj/Cwjx+VUVIPFjzzd5PHKmBrINpHia0ni+HOCvXcK8uMAG8EpIjjOlwIpWsNDvWXcLKnVxTBDFdPCcxuYkC/tptSa8Z0b0y6ep4r4eZ+2HzJfIZhZ/g978/64OPAyobeIj8rgwLfhDRDkp7CObI7nsIgBsxVIS2XAw6it5mWnnQk4ZlvCWZQP5qBwRYWNSsFk2XT9VvBbiWqa7ZzLAv6jWTkOzHHaEzKumxzuHBu/kCWp93mD448KCKZ6kHukx7QxrG44rKww7Sjx6y5MooEBpzjcdV+jt/6Q0MPP6MblljdfwzFjFTpAswNFbcFeo91i+9Pi+OmOWW7VbfyDHuP4MLBc2HYEs3XV2AAbGpwuaSqo8= 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)(136003)(376002)(39860400002)(396003)(346002)(46966005)(36756003)(6916009)(86362001)(107886003)(6486002)(478600001)(6512007)(4326008)(356005)(83380400001)(54906003)(70206006)(2616005)(316002)(82310400003)(69590400008)(186003)(8676002)(5660300002)(336012)(47076004)(2906002)(6666004)(26005)(6506007)(82740400003)(70586007)(16526019)(44832011)(956004)(36906005)(8936002)(81166007); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2020 10:27:10.1002 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: a440b8b5-2a39-43ce-e822-08d87fe305a4 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: AM5EUR03FT018.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0802MB2290 X-Spam-Status: No, score=-14.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, MSGID_FROM_MTA_HEADER, RCVD_IN_DNSWL_LOW, 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> Cc: Mark Rutland <mark.rutland@arm.com>, Florian Weimer <fweimer@redhat.com>, Kees Cook <keescook@chromium.org>, kernel-hardening@lists.openwall.com, Salvatore Mesoraca <s.mesoraca16@gmail.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, linux-kernel@vger.kernel.org, Jeremy Linton <jeremy.linton@arm.com>, Mark Brown <broonie@kernel.org>, Lennart Poettering <mzxreary@0pointer.de>, linux-hardening@vger.kernel.org, Topi Miettinen <toiwoton@gmail.com>, linux-arm-kernel@lists.infradead.org Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831]
|
|
Commit Message
Szabolcs Nagy
Nov. 3, 2020, 10:26 a.m. UTC
Program headers are processed in two pass: after the first pass load segments are mmapped so in the second pass target specific note processing logic can access the notes. The second pass is moved later so various link_map fields are set up that may be useful for note processing such as l_phdr. --- elf/dl-load.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Comments
* Szabolcs Nagy: > Program headers are processed in two pass: after the first pass > load segments are mmapped so in the second pass target specific > note processing logic can access the notes. > > The second pass is moved later so various link_map fields are > set up that may be useful for note processing such as l_phdr. > --- > elf/dl-load.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/elf/dl-load.c b/elf/dl-load.c > index ceaab7f18e..673cf960a0 100644 > --- a/elf/dl-load.c > +++ b/elf/dl-load.c > @@ -1259,21 +1259,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > maplength, has_holes, loader); > if (__glibc_unlikely (errstring != NULL)) > goto call_lose; > - > - /* Process program headers again after load segments are mapped in > - case processing requires accessing those segments. Scan program > - headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > - exits. */ > - for (ph = &phdr[l->l_phnum]; ph != phdr; --ph) > - switch (ph[-1].p_type) > - { > - case PT_NOTE: > - _dl_process_pt_note (l, fd, &ph[-1]); > - break; > - case PT_GNU_PROPERTY: > - _dl_process_pt_gnu_property (l, fd, &ph[-1]); > - break; > - } > } > > if (l->l_ld == 0) > @@ -1481,6 +1466,21 @@ cannot enable executable stack as shared object requires"); > /* Assign the next available module ID. */ > l->l_tls_modid = _dl_next_tls_modid (); > > + /* Process program headers again after load segments are mapped in > + case processing requires accessing those segments. Scan program > + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > + exits. */ > + for (ph = &l->l_phdr[l->l_phnum]; ph != l->l_phdr; --ph) > + switch (ph[-1].p_type) > + { > + case PT_NOTE: > + _dl_process_pt_note (l, fd, &ph[-1]); > + break; > + case PT_GNU_PROPERTY: > + _dl_process_pt_gnu_property (l, fd, &ph[-1]); > + break; > + } > + > #ifdef DL_AFTER_LOAD > DL_AFTER_LOAD (l); > #endif Is this still compatible with the CET requirements? I hope it is because the CET magic happens in _dl_open_check, so after the the code in elf/dl-load.c has run. Thanks, Florian
On Tue, Nov 3, 2020 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote: > > * Szabolcs Nagy: > > > Program headers are processed in two pass: after the first pass > > load segments are mmapped so in the second pass target specific > > note processing logic can access the notes. > > > > The second pass is moved later so various link_map fields are > > set up that may be useful for note processing such as l_phdr. > > --- > > elf/dl-load.c | 30 +++++++++++++++--------------- > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > diff --git a/elf/dl-load.c b/elf/dl-load.c > > index ceaab7f18e..673cf960a0 100644 > > --- a/elf/dl-load.c > > +++ b/elf/dl-load.c > > @@ -1259,21 +1259,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > > maplength, has_holes, loader); > > if (__glibc_unlikely (errstring != NULL)) > > goto call_lose; > > - > > - /* Process program headers again after load segments are mapped in > > - case processing requires accessing those segments. Scan program > > - headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > - exits. */ > > - for (ph = &phdr[l->l_phnum]; ph != phdr; --ph) > > - switch (ph[-1].p_type) > > - { > > - case PT_NOTE: > > - _dl_process_pt_note (l, fd, &ph[-1]); > > - break; > > - case PT_GNU_PROPERTY: > > - _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > - break; > > - } > > } > > > > if (l->l_ld == 0) > > @@ -1481,6 +1466,21 @@ cannot enable executable stack as shared object requires"); > > /* Assign the next available module ID. */ > > l->l_tls_modid = _dl_next_tls_modid (); > > > > + /* Process program headers again after load segments are mapped in > > + case processing requires accessing those segments. Scan program > > + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > + exits. */ > > + for (ph = &l->l_phdr[l->l_phnum]; ph != l->l_phdr; --ph) > > + switch (ph[-1].p_type) > > + { > > + case PT_NOTE: > > + _dl_process_pt_note (l, fd, &ph[-1]); > > + break; > > + case PT_GNU_PROPERTY: > > + _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > + break; > > + } > > + > > #ifdef DL_AFTER_LOAD > > DL_AFTER_LOAD (l); > > #endif > > Is this still compatible with the CET requirements? > > I hope it is because the CET magic happens in _dl_open_check, so after > the the code in elf/dl-load.c has run. > > _dl_process_pt_note and _dl_process_pt_gnu_property may call _dl_signal_error. Are we prepared to clean more things up when it happens? I am investigating: https://sourceware.org/bugzilla/show_bug.cgi?id=26825 I don't think cleanup of _dl_process_pt_gnu_property failure is done properly.
The 11/03/2020 04:36, H.J. Lu wrote: > On Tue, Nov 3, 2020 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote: > > * Szabolcs Nagy: > > > > > Program headers are processed in two pass: after the first pass > > > load segments are mmapped so in the second pass target specific > > > note processing logic can access the notes. > > > > > > The second pass is moved later so various link_map fields are > > > set up that may be useful for note processing such as l_phdr. > > > --- > > > elf/dl-load.c | 30 +++++++++++++++--------------- > > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > > > diff --git a/elf/dl-load.c b/elf/dl-load.c > > > index ceaab7f18e..673cf960a0 100644 > > > --- a/elf/dl-load.c > > > +++ b/elf/dl-load.c > > > @@ -1259,21 +1259,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > > > maplength, has_holes, loader); > > > if (__glibc_unlikely (errstring != NULL)) > > > goto call_lose; > > > - > > > - /* Process program headers again after load segments are mapped in > > > - case processing requires accessing those segments. Scan program > > > - headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > > - exits. */ > > > - for (ph = &phdr[l->l_phnum]; ph != phdr; --ph) > > > - switch (ph[-1].p_type) > > > - { > > > - case PT_NOTE: > > > - _dl_process_pt_note (l, fd, &ph[-1]); > > > - break; > > > - case PT_GNU_PROPERTY: > > > - _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > > - break; > > > - } > > > } > > > > > > if (l->l_ld == 0) > > > @@ -1481,6 +1466,21 @@ cannot enable executable stack as shared object requires"); > > > /* Assign the next available module ID. */ > > > l->l_tls_modid = _dl_next_tls_modid (); > > > > > > + /* Process program headers again after load segments are mapped in > > > + case processing requires accessing those segments. Scan program > > > + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > > + exits. */ > > > + for (ph = &l->l_phdr[l->l_phnum]; ph != l->l_phdr; --ph) > > > + switch (ph[-1].p_type) > > > + { > > > + case PT_NOTE: > > > + _dl_process_pt_note (l, fd, &ph[-1]); > > > + break; > > > + case PT_GNU_PROPERTY: > > > + _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > > + break; > > > + } > > > + > > > #ifdef DL_AFTER_LOAD > > > DL_AFTER_LOAD (l); > > > #endif > > > > Is this still compatible with the CET requirements? > > > > I hope it is because the CET magic happens in _dl_open_check, so after > > the the code in elf/dl-load.c has run. i believe the note processing and later cet magic are not affected by this code move. but i did not test this with cet. > > _dl_process_pt_note and _dl_process_pt_gnu_property may call > _dl_signal_error. Are we prepared to clean more things up when it > happens? I am investigating: yeah, this is difficult to reason about. it seems to me that after _dl_map_object returns there may be _dl_map_object_deps which can fail in a way that all of dlopen has to be rolled back, so if i move things around in _dl_map_object that should not introduce new issues. but it is not clear to me how robust the dlopen code is against arbitrary failure in dl_open_worker. > > https://sourceware.org/bugzilla/show_bug.cgi?id=26825 > > I don't think cleanup of _dl_process_pt_gnu_property failure is done > properly. > > -- > H.J. --
On Tue, Nov 3, 2020 at 7:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 11/03/2020 04:36, H.J. Lu wrote: > > On Tue, Nov 3, 2020 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote: > > > * Szabolcs Nagy: > > > > > > > Program headers are processed in two pass: after the first pass > > > > load segments are mmapped so in the second pass target specific > > > > note processing logic can access the notes. > > > > > > > > The second pass is moved later so various link_map fields are > > > > set up that may be useful for note processing such as l_phdr. > > > > --- > > > > elf/dl-load.c | 30 +++++++++++++++--------------- > > > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/elf/dl-load.c b/elf/dl-load.c > > > > index ceaab7f18e..673cf960a0 100644 > > > > --- a/elf/dl-load.c > > > > +++ b/elf/dl-load.c > > > > @@ -1259,21 +1259,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > > > > maplength, has_holes, loader); > > > > if (__glibc_unlikely (errstring != NULL)) > > > > goto call_lose; > > > > - > > > > - /* Process program headers again after load segments are mapped in > > > > - case processing requires accessing those segments. Scan program > > > > - headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > > > - exits. */ > > > > - for (ph = &phdr[l->l_phnum]; ph != phdr; --ph) > > > > - switch (ph[-1].p_type) > > > > - { > > > > - case PT_NOTE: > > > > - _dl_process_pt_note (l, fd, &ph[-1]); > > > > - break; > > > > - case PT_GNU_PROPERTY: > > > > - _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > > > - break; > > > > - } > > > > } > > > > > > > > if (l->l_ld == 0) > > > > @@ -1481,6 +1466,21 @@ cannot enable executable stack as shared object requires"); > > > > /* Assign the next available module ID. */ > > > > l->l_tls_modid = _dl_next_tls_modid (); > > > > > > > > + /* Process program headers again after load segments are mapped in > > > > + case processing requires accessing those segments. Scan program > > > > + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > > > + exits. */ > > > > + for (ph = &l->l_phdr[l->l_phnum]; ph != l->l_phdr; --ph) > > > > + switch (ph[-1].p_type) > > > > + { > > > > + case PT_NOTE: > > > > + _dl_process_pt_note (l, fd, &ph[-1]); > > > > + break; > > > > + case PT_GNU_PROPERTY: > > > > + _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > > > + break; > > > > + } > > > > + > > > > #ifdef DL_AFTER_LOAD > > > > DL_AFTER_LOAD (l); > > > > #endif > > > > > > Is this still compatible with the CET requirements? > > > > > > I hope it is because the CET magic happens in _dl_open_check, so after > > > the the code in elf/dl-load.c has run. > > i believe the note processing and later cet magic > are not affected by this code move. > > but i did not test this with cet. > > > > > _dl_process_pt_note and _dl_process_pt_gnu_property may call > > _dl_signal_error. Are we prepared to clean more things up when it > > happens? I am investigating: > > yeah, this is difficult to reason about. > > it seems to me that after _dl_map_object returns there > may be _dl_map_object_deps which can fail in a way that > all of dlopen has to be rolled back, so if i move things > around in _dl_map_object that should not introduce new > issues. I haven't investigated it in detail. But there are 1314 if (l->l_phdr == NULL) 1315 { 1316 /* The program header is not contained in any of the segments. 1317 We have to allocate memory ourself and copy it over from out 1318 temporary place. */ 1319 ElfW(Phdr) *newp = (ElfW(Phdr) *) malloc (header->e_phnum 1320 * sizeof (ElfW(Phdr))); 1321 if (newp == NULL) 1322 { 1323 errstring = N_("cannot allocate memory for program header"); 1324 goto call_lose_errno; 1325 } 1326 1327 l->l_phdr = memcpy (newp, phdr, 1328 (header->e_phnum * sizeof (ElfW(Phdr)))); 1329 l->l_phdr_allocated = 1; 1330 } When _dl_process_pt_gnu_property is moved after it, will l_phdr be free on _dl_signal_error? > but it is not clear to me how robust the dlopen code is > against arbitrary failure in dl_open_worker. I think we are mostly OK, except for some corner cases. Delay _dl_process_pt_gnu_property may introduce more corner cases. > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=26825 > > > > I don't think cleanup of _dl_process_pt_gnu_property failure is done > > properly. > > > > -- > > H.J. > > --
The 11/03/2020 04:36, H.J. Lu wrote: > On Tue, Nov 3, 2020 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote: > > Is this still compatible with the CET requirements? > > > > I hope it is because the CET magic happens in _dl_open_check, so after > > the the code in elf/dl-load.c has run. > > _dl_process_pt_note and _dl_process_pt_gnu_property may call > _dl_signal_error. Are we prepared to clean more things up when it > happens? I am investigating: > > https://sourceware.org/bugzilla/show_bug.cgi?id=26825 > > I don't think cleanup of _dl_process_pt_gnu_property failure is done > properly. (removed linux devs from cc) indeed failures are not handled right, the call tree is dl_open_worker _dl_map_object _dl_map_object_from_fd _dl_process_pt_gnu_property record bti/cet marking _dl_map_object_deps catch(_dl_map_object) ... record bti/cet marking // maping is complete dl_open_check handle bti/cet _dl_relocate_object // point of no return activate_nodelete catch(dl_init) dl_main parse phdr or _dl_map_object if ld.so mapped ... record bti/cet marking _dl_map_object_deps ... record bti/cet marking rtld_main_check handle bti/cet _dl_relocate_object the issues: - dl_open_check is only applied to the dlopened lib not to its dependencies. does cet check the deps? for bti this means protection is missed on deps, i opened bug 26926. - _dl_map_object_from_fd has special failure handling (via "lose") because there is local clean ups to be done (e.g. open fd, mallocs), so _dl_process_pt_gnu_property cannot use _dl_signal_error. however i need to do bti mmap there while the fd is available. my plan for bti: do mmap in _dl_process_pt_gnu_property and record failures in the linkmap, then in dl_open_check walk dependencies and if any of them failed then signal the error from there. (this fixes both issues on aarch64.) if this works out i'll resend the patches and ask linux to add bti protection to the main exe.
diff --git a/elf/dl-load.c b/elf/dl-load.c index ceaab7f18e..673cf960a0 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1259,21 +1259,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, maplength, has_holes, loader); if (__glibc_unlikely (errstring != NULL)) goto call_lose; - - /* Process program headers again after load segments are mapped in - case processing requires accessing those segments. Scan program - headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY - exits. */ - for (ph = &phdr[l->l_phnum]; ph != phdr; --ph) - switch (ph[-1].p_type) - { - case PT_NOTE: - _dl_process_pt_note (l, fd, &ph[-1]); - break; - case PT_GNU_PROPERTY: - _dl_process_pt_gnu_property (l, fd, &ph[-1]); - break; - } } if (l->l_ld == 0) @@ -1481,6 +1466,21 @@ cannot enable executable stack as shared object requires"); /* Assign the next available module ID. */ l->l_tls_modid = _dl_next_tls_modid (); + /* Process program headers again after load segments are mapped in + case processing requires accessing those segments. Scan program + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY + exits. */ + for (ph = &l->l_phdr[l->l_phnum]; ph != l->l_phdr; --ph) + switch (ph[-1].p_type) + { + case PT_NOTE: + _dl_process_pt_note (l, fd, &ph[-1]); + break; + case PT_GNU_PROPERTY: + _dl_process_pt_gnu_property (l, fd, &ph[-1]); + break; + } + #ifdef DL_AFTER_LOAD DL_AFTER_LOAD (l); #endif