[arm-embedded][committed][PATCH 4/7] ARMv8-M Security Extension's cmse_nonsecure_entry: clear registers

["Andre Vieira (lists)" <Andre dot SimoesDiasVieira at arm dot com>]

On 30/11/16 17:22, Kyrill Tkachov wrote:
> 
> On 30/11/16 15:32, Andre Vieira (lists) wrote:
>> On 23/11/16 11:52, Andre Vieira (lists) wrote:
>>> Hi,
>>>
>>> After some extra testing I realized there was an issue with the way we
>>> were clearing registers when returning from a cmse_nonsecure_entry
>>> function for ARMv8-M.Baseline.  This patch fixes that and changes the
>>> testcase to catch the issue.
>>>
>>> The problem was I was always using LR to clear the registers, however,
>>> due to the way the Thumb-1 backend works, we can't guarantee LR will
>>> contain the address to which we will be returning at the time of
>>> clearing. Instead we use r0 to clear r1-r3 and IP. If the function does
>>> not use r0 to return a value, we clear r0 with 0 before using it to
>>> clear everything else. As for LR, we move the value of the register used
>>> to return into it prior to returning.
>>>
>>> This satisfies the requirements of not leaking secure information since
>>> all registers hold either:
>>> - values to return
>>> - 0
>>> - return address
>>>
>>> No changes to ChangeLog.
>>>
>>> Cheers,
>>> Andre
>>>
>> Hi,
>>
>> So I seemed to have forgotten to address two of your comments earlier,
>> done in this version.
>>
>> To reiterate:
>> After some extra testing I realized there was an issue with the way we
>> were clearing registers when returning from a cmse_nonsecure_entry
>> function for ARMv8-M Baseline.  This patch fixes that and changes the
>> testcase to catch the issue.
>>
>> The problem was I was always using LR to clear the registers, however,
>> due to the way the Thumb-1 backend works, we can't guarantee LR will
>> contain the address to which we will be returning at the time of
>> clearing. Instead we use r0 to clear r1-r3 and IP. If the function does
>> not use r0 to return a value, we clear r0 with 0 before using it to
>> clear everything else. As for LR, we move the value of the register used
>> to return into it prior to returning.
>>
>> This satisfies the requirements of not leaking secure information since
>> all registers hold either:
>> - values to return
>> - 0
>> - return address
>>
>> *** gcc/ChangeLog ***
>> 2016-11-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>               Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>           * 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_cmse_entry_return): Duplicate
>>           thumb2_return pattern for cmse_nonsecure_entry functions.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-11-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>               Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>           * 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.
> 
> Ok, thanks for addressing the issues.
> Kyrill
> 
>> Cheers,
>> Andre
> 
Hi,

Backported this to the embedded-6-branch in revision r243250.

Cheers,
Andre

gcc/ChangeLog.arm:
2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	Backport from mainline
	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Thomas Preud'homme	<thomas.preudhomme@arm.com>

	* 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): Disable for
	cmse_nonsecure_entry functions.
	(*thumb2_cmse_entry_return): Duplicate thumb2_return pattern for
	cmse_nonsecure_entry functions.

gcc/testsuite/ChangeLog.arm:
2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	Backport from mainline
	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Thomas Preud'homme	<thomas.preudhomme@arm.com>

	* 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: New.
	* gcc.target/arm/cmse/baseline/softfp.c: New.
	* 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.

Attachment: diff4
Description: Text document