From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 93CEF3858407 for ; Thu, 4 Aug 2022 15:19:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 93CEF3858407 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=foss.arm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=foss.arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E541D113E; Thu, 4 Aug 2022 08:19:24 -0700 (PDT) Received: from [10.57.13.163] (unknown [10.57.13.163]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 90B783F73B; Thu, 4 Aug 2022 08:19:23 -0700 (PDT) Message-ID: <72ab6827-13eb-a2ed-668c-d6b53a83e9ca@foss.arm.com> Date: Thu, 4 Aug 2022 16:19:21 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v2 1/8] newlib: libc: define M-profile PACBTI-enablement macros Content-Language: en-GB To: Victor Do Nascimento , newlib@sourceware.org Cc: richard.earnshaw@arm.com References: <20220803153524.20631-1-victor.donascimento@arm.com> <20220803153524.20631-2-victor.donascimento@arm.com> From: Richard Earnshaw In-Reply-To: <20220803153524.20631-2-victor.donascimento@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3496.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP 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: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Aug 2022 15:19:26 -0000 On 03/08/2022 16:35, Victor Do Nascimento wrote: > Create an assembly header file that conditionally defines fuction > prologues/epilogues depending on the compile-time mbranch-protection > argument values. > > * newlib/libc/machine/arm/pacbti.h: New. > --- > newlib/libc/machine/arm/pacbti.h | 64 ++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > create mode 100644 newlib/libc/machine/arm/pacbti.h > > diff --git a/newlib/libc/machine/arm/pacbti.h b/newlib/libc/machine/arm/pacbti.h > new file mode 100644 > index 000000000..4c31d00bc > --- /dev/null > +++ b/newlib/libc/machine/arm/pacbti.h > @@ -0,0 +1,64 @@ > +/* > + * Copyright (c) 2022 Arm Ltd > + * All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * 3. The name of the company may not be used to endorse or promote > + * products derived from this software without specific prior written > + * permission. > + * > + * THIS SOFTWARE IS PROVIDED BY ARM LTD ``AS IS'' AND ANY EXPRESS OR IMPLIED > + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. > + * IN NO EVENT SHALL ARM LTD BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED > + * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR > + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING > + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > + > +/* Checki whether leaf function PAC signing has been requested > + in the -mbranch-protect compile-time option. */ > +#define LEAF_PROTECT_BIT 2 > +#define __HAVE_PAC_LEAF \ > + __ARM_FEATURE_PAC_DEFAULT & (1 << LEAF_PROTECT_BIT) Either this header needs to avoid polluting the user namespace, or it doesn't. If it does, then LEAF_PROTECT_BIT fails to do that. If it doesn't then __HAVE_PAC_LEAF should really be HAVE_PAC_LEAF (to avoid polluting the reserved namespace. I suspect this header is private to newlib (won't be exported to users), so should not be prefixing names with __. > + > +/* Macro to handle function entry depending on branch-protection > + schemes. */ > + .macro pacbti_prologue > +#if __HAVE_PAC_LEAF > +#if __ARM_FEATURE_BTI_DEFAULT > + pacbti ip, lr, sp > +#else > + pac ip, lr, sp > +#endif /* __ARM_FEATURE_BTI_DEFAULT */ > + .cfi_register 143, 12 > + str ip, [sp, #-4]! This causes the stack to lose 8-byte alignment. Whilst for leaf functions that's probably not a problem, the macro should have an option where the user can ask for alignment to be preserved. But there should also be an option to not save IP on the stack at all (for when the user does not modify IP in the function body). > + .save {ra_auth_code} > + .cfi_def_cfa_offset 4 > + .cfi_offset 143, -4 > +#elif __ARM_FEATURE_BTI_DEFAULT > + bti > +#endif /* __HAVE_PAC_LEAF */ > + .endm > + > +/* Macro to handle different branch exchange cases depending on > + branch-protection schemes. */ > + .macro pacbti_epilogue > +#if __HAVE_PAC_LEAF > + ldr ip, [sp], #4 > + .cfi_restore 143 > + .cfi_def_cfa_offset 0 > + aut ip, lr, sp > +#endif /* __HAVE_PAC_LEAF */ > + bx lr > + .endm I think these macros are really misnamed, firstly, they're only for leaf functions and secondly, they (particularly the epilogue) does something even if PAC is not needed. So I think they should be renamed as 'leaf_prologue' and 'leaf_epilogue' respectively. In consequence, I think this file should really be merged into the existing arm_asm.h, then we don't need yet another header file. Finally, the header needs to define a (C) macro that defines how much to adjust the CFI offset by for various scenarios: - When PAC is not used - When PAC is used with no alignment - When PAC is used and asked for 8-byte alignment to be preserved. R.