From patchwork Mon Jan 15 21:09:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Jeanson X-Patchwork-Id: 84141 Return-Path: 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 4C429385AC1D for ; Mon, 15 Jan 2024 21:10:22 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from smtpout.efficios.com (unknown [IPv6:2607:5300:203:b2ee::31e5]) by sourceware.org (Postfix) with ESMTPS id 0C8E43857BB2 for ; Mon, 15 Jan 2024 21:09:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0C8E43857BB2 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=efficios.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0C8E43857BB2 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:5300:203:b2ee::31e5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705352998; cv=none; b=IESBLW9byQ00WfcSk/mEJI9DUerCeRVQ0rM3+pLOzh5SM5GtS0qe/+PWeIovik6x/2a6DevVr88WmUxVODzXro/dMqWNu+JDQ0iUGCL/Y73JhjUFydFDkSQEYyRiw3qmQ4UYzuvzbheAZGem/itFe0rBS5HvXULKn6xOw5h7feE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705352998; c=relaxed/simple; bh=Jft/Vh5I9eHSbroOVwbBSt/Jgm9EiEPDzNK26zUvtw0=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=gbiJRl0bQs+1WJ3D8LJct8Zwvl8ehUCPK5i8xLp01lq72lQTEJVYd06kLodb5oO9TOdJMPBNTAZYlG5r7H7J0NBckPepqITbqpo6ndSs1Vy41geIDooXrRAHkjkxmLVWFjpzw4b3R6EWb3Yt+QkhCn8FmJ7ITnqM+MLtAP/vQhw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=efficios.com; s=smtpout1; t=1705352993; bh=Jft/Vh5I9eHSbroOVwbBSt/Jgm9EiEPDzNK26zUvtw0=; h=From:To:Cc:Subject:Date:From; b=ZMXgqKtajG2u0kzZ7f243Y1B26DXAQD92t9F93nScGQHl0Kn+95PhHYaVyUp0AtVr KPCNtjC6zn3xcLx1GR6DzLsGkkLH7F/HLhYjLLHz3RfNEnG1p5OX2UwF7Qc6XViKXa B6bWDFhx851mvWcC1Pun3D3Hiu8X94JhzsXgYxjZl9h3pZT4jvbl32PujJxd7iBPwK UAzYt//y4eBfSf8uOFPjioJI8GDLDm+4Q3QSFFf6melcUcjOwJa2Zfi6vPy7tlGuMB s9L7wz2jph05shJE3fqNC3gdxAYmmdKedaBtZZyqcqea0oKaz0EBZsHI9+cxmAevI0 NGl6eXc86+z4g== Received: from laptop-mjeanson.internal.efficios.com (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) by smtpout.efficios.com (Postfix) with ESMTPSA id 4TDPss4WlLzKmw; Mon, 15 Jan 2024 16:09:53 -0500 (EST) From: Michael Jeanson To: libc-alpha@sourceware.org Cc: Michael Jeanson , Mathieu Desnoyers Subject: [PATCH] nptl: fix potential merge of __rseq_* relro symbols Date: Mon, 15 Jan 2024 16:09:31 -0500 Message-Id: <20240115210931.1267987-1-mjeanson@efficios.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org While working on a patch to add support for the extensible rseq ABI, we came across an issue where a new 'const' variable would be merged with the existing '__rseq_size' variable. We tracked this to the use of '-fmerge-all-constants' which allows the compiler to merge identical constant variables. This means that all 'const' variables in a compile unit that are of the same size and are initialized to the same value can be merged. In this specific case, on 32 bit systems 'unsigned int' and 'ptrdiff_t' are both 4 bytes and initialized to 0 which should trigger the merge. However for reasons we haven't delved into when the attribute 'section (".data.rel.ro")' is added to the mix, only variables of the same exact types are merged. As far as we know this behavior is not specified anywhere and could change with a new compiler version, hence this patch. Declare both variables as regular static variables and add 'extern const' aliases for the ABI. This has the added bonus of removing the asm workaround to set the values on rseq registration. Tested on Debian 12 with GCC 12.2. Signed-off-by: Michael Jeanson Reviewed-by: Mathieu Desnoyers --- sysdeps/nptl/dl-tls_init_tp.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c index 092c274f36..7f66f6de32 100644 --- a/sysdeps/nptl/dl-tls_init_tp.c +++ b/sysdeps/nptl/dl-tls_init_tp.c @@ -45,8 +45,15 @@ rtld_mutex_dummy (pthread_mutex_t *lock) #endif const unsigned int __rseq_flags; -const unsigned int __rseq_size attribute_relro; -const ptrdiff_t __rseq_offset attribute_relro; + +/* Due to the use of '-fmerge-all-constants' the compiler is allowed to merge + all 'const' variables of the same size that are initialized to the same + value. To work around this, declare them as regular static variable and use + an alias that is 'const'. */ +static unsigned int rseq_size attribute_relro; +extern const unsigned int __attribute__ ((alias ("rseq_size"))) __rseq_size; +static ptrdiff_t rseq_offset attribute_relro; +extern const ptrdiff_t __attribute__ ((alias ("rseq_offset"))) __rseq_offset; void __tls_pre_init_tp (void) @@ -105,10 +112,8 @@ __tls_init_tp (void) do_rseq = TUNABLE_GET (rseq, int, NULL); if (rseq_register_current_thread (pd, do_rseq)) { - /* We need a writable view of the variables. They are in - .data.relro and are not yet write-protected. */ - extern unsigned int size __asm__ ("__rseq_size"); - size = sizeof (pd->rseq_area); + /* The variables are in .data.relro but are not yet write-protected. */ + rseq_size = sizeof (pd->rseq_area); } #ifdef RSEQ_SIG @@ -117,8 +122,7 @@ __tls_init_tp (void) all targets support __thread_pointer, so set __rseq_offset only if the rseq registration may have happened because RSEQ_SIG is defined. */ - extern ptrdiff_t offset __asm__ ("__rseq_offset"); - offset = (char *) &pd->rseq_area - (char *) __thread_pointer (); + rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer (); #endif }