From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5144 invoked by alias); 26 Oct 2016 14:14:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 5044 invoked by uid 89); 26 Oct 2016 14:14:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.4 required=5.0 tests=BAYES_50,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy=retaining, UINT32_MAX, uint32_max, aapcs X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Oct 2016 14:14:26 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0000A29; Wed, 26 Oct 2016 07:14:24 -0700 (PDT) Received: from [10.2.207.77] (e100706-lin.cambridge.arm.com [10.2.207.77]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 834AF3F218; Wed, 26 Oct 2016 07:14:24 -0700 (PDT) Message-ID: <5810BA3F.1060907@foss.arm.com> Date: Wed, 26 Oct 2016 14:14:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: "Andre Vieira (lists)" , gcc-patches@gcc.gnu.org Subject: Re: [PATCHv2 4/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_entry: clear registers References: <5796116C.6010100@arm.com> <579612EE.3050606@arm.com> <57BD7E8B.4000108@arm.com> <580F885D.6030107@arm.com> In-Reply-To: <580F885D.6030107@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg02128.txt.bz2 Hi Andre, On 25/10/16 17:29, Andre Vieira (lists) wrote: > On 24/08/16 12:01, Andre Vieira (lists) wrote: >> On 25/07/16 14:23, Andre Vieira (lists) wrote: >>> This patch extends support for the ARMv8-M Security Extensions >>> 'cmse_nonsecure_entry' attribute to safeguard against leak of >>> information through unbanked registers. >>> >>> When returning from a nonsecure entry function we clear all caller-saved >>> registers that are not used to pass return values, by writing either the >>> LR, in case of general purpose registers, or the value 0, in case of FP >>> registers. We use the LR to write to APSR and FPSCR too. We currently do >>> not support entry functions that pass arguments or return variables on >>> the stack and we diagnose this. This patch relies on the existing code >>> to make sure callee-saved registers used in cmse_nonsecure_entry >>> functions are saved and restored thus retaining their nonsecure mode >>> value, this should be happening already as it is required by AAPCS. >>> >>> This patch also clears padding bits for cmse_nonsecure_entry functions >>> with struct and union return types. For unions a bit is only considered >>> a padding bit if it is an unused bit in every field of that union. The >>> function that calculates these is used in a later patch to do the same >>> for arguments of cmse_nonsecure_call's. >>> >>> *** gcc/ChangeLog *** >>> 2016-07-25 Andre Vieira >>> Thomas Preud'homme >>> >>> * config/arm/arm.c (output_return_instruction): Clear >>> registers. >>> (thumb2_expand_return): Likewise. >>> (thumb1_expand_epilogue): Likewise. >>> (thumb_exit): Likewise. >>> (arm_expand_epilogue): Likewise. >>> (cmse_nonsecure_entry_clear_before_return): New. >>> (comp_not_to_clear_mask_str_un): New. >>> (compute_not_to_clear_mask): New. >>> * config/arm/thumb1.md (*epilogue_insns): Change length attribute. >>> * config/arm/thumb2.md (*thumb2_return): Likewise. >>> >>> *** gcc/testsuite/ChangeLog *** >>> 2016-07-25 Andre Vieira >>> Thomas Preud'homme >>> >>> * gcc.target/arm/cmse/cmse.exp: Test different multilibs separate. >>> * gcc.target/arm/cmse/struct-1.c: New. >>> * gcc.target/arm/cmse/bitfield-1.c: New. >>> * gcc.target/arm/cmse/bitfield-2.c: New. >>> * gcc.target/arm/cmse/bitfield-3.c: New. >>> * gcc.target/arm/cmse/baseline/cmse-2.c: Test that registers are >>> cleared. >>> * gcc.target/arm/cmse/mainline/soft/cmse-5.c: New. >>> * gcc.target/arm/cmse/mainline/hard/cmse-5.c: New. >>> * gcc.target/arm/cmse/mainline/hard-sp/cmse-5.c: New. >>> * gcc.target/arm/cmse/mainline/softfp/cmse-5.c: New. >>> * gcc.target/arm/cmse/mainline/softfp-sp/cmse-5.c: New. >>> >> Updated this patch to correctly clear only the cumulative >> exception-status (0-4,7) and the condition code bits (28-31) of the >> FPSCR. I also adapted the code to be handle the bigger floating point >> register files. >> >> ---- >> >> This patch extends support for the ARMv8-M Security Extensions >> 'cmse_nonsecure_entry' attribute to safeguard against leak of >> information through unbanked registers. >> >> When returning from a nonsecure entry function we clear all caller-saved >> registers that are not used to pass return values, by writing either the >> LR, in case of general purpose registers, or the value 0, in case of FP >> registers. We use the LR to write to APSR. For FPSCR we clear only the >> cumulative exception-status (0-4, 7) and the condition code bits >> (28-31). We currently do not support entry functions that pass arguments >> or return variables on the stack and we diagnose this. This patch relies >> on the existing code to make sure callee-saved registers used in >> cmse_nonsecure_entry functions are saved and restored thus retaining >> their nonsecure mode value, this should be happening already as it is >> required by AAPCS. >> >> This patch also clears padding bits for cmse_nonsecure_entry functions >> with struct and union return types. For unions a bit is only considered >> a padding bit if it is an unused bit in every field of that union. The >> function that calculates these is used in a later patch to do the same >> for arguments of cmse_nonsecure_call's. >> >> *** gcc/ChangeLog *** >> 2016-07-xx Andre Vieira >> Thomas Preud'homme >> >> * config/arm/arm.c (output_return_instruction): Clear >> registers. >> (thumb2_expand_return): Likewise. >> (thumb1_expand_epilogue): Likewise. >> (thumb_exit): Likewise. >> (arm_expand_epilogue): Likewise. >> (cmse_nonsecure_entry_clear_before_return): New. >> (comp_not_to_clear_mask_str_un): New. >> (compute_not_to_clear_mask): New. >> * config/arm/thumb1.md (*epilogue_insns): Change length attribute. >> * config/arm/thumb2.md (*thumb2_return): Duplicate pattern for >> cmse_nonsecure_entry functions. >> >> *** gcc/testsuite/ChangeLog *** >> 2016-07-xx Andre Vieira >> Thomas Preud'homme >> >> * gcc.target/arm/cmse/cmse.exp: Test different multilibs separate. >> * gcc.target/arm/cmse/struct-1.c: New. >> * gcc.target/arm/cmse/bitfield-1.c: New. >> * gcc.target/arm/cmse/bitfield-2.c: New. >> * gcc.target/arm/cmse/bitfield-3.c: New. >> * gcc.target/arm/cmse/baseline/cmse-2.c: Test that registers are >> cleared. >> * gcc.target/arm/cmse/mainline/soft/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/hard/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/hard-sp/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/softfp/cmse-5.c: New. >> * gcc.target/arm/cmse/mainline/softfp-sp/cmse-5.c: New. >> > Hi, > > Rebased previous patch on top of trunk as requested. No changes to > ChangeLog. > > Cheers, > Andre + for (i = *regno; i < regno_t; i++) + { + /* For all but the last register used by this field only keep the + padding bits that were padding bits in this field. */ + padding_bits_to_clear_res[i] &= padding_bits_to_clear_t[i]; + } + + /* For the last register, keep all padding bits that were padding + bits in this field and any padding bits that are still valid + as padding bits but fall outside of this field's size. */ + mask = (UINT32_MAX - ((uint32_t) 1 << last_used_bit_t)) + 1; + padding_bits_to_clear_res[regno_t] + &= padding_bits_to_clear_t[regno_t] | mask; I tried this on a clean trunk (without any of my patches) and I'm getting a build error about UINT32_MAX not being defined. Did you have some other patches in your tree that masked this? I think avoiding UINT32_MAX would be sensible here. I don't see it used anywhere else in gcc. Perhaps just using (uint32_t)-1 would work? Or reuse the 64-bit HOST_WIDE_INT_M1U to get a mask of all ones. Kyrill