[Patch 6/8 V2] Arm: Add pointer authentication for stack-unwinding runtime.

Richard Earnshaw Richard.Earnshaw@foss.arm.com
Fri Dec 10 12:15:23 GMT 2021



On 09/12/2021 17:36, Andrea Corallo via Gcc-patches wrote:
> Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> 
>> On 28/10/2021 12:43, Tejas Belagod via Gcc-patches wrote:
>>>
>>>> -----Original Message-----
>>>> From: Gcc-patches <gcc-patches-
>>>> bounces+belagod=gcc.gnu.org@gcc.gnu.org> On Behalf Of Tejas Belagod via
>>>> Gcc-patches
>>>> Sent: Friday, October 8, 2021 1:18 PM
>>>> To: gcc-patches@gcc.gnu.org
>>>> Subject: [Patch 5/7, Arm. GCC] Add pointer authentication for stack-
>>>> unwinding runtime.
>>>>
>>>> Hi,
>>>>
>>>> This patch adds authentication for when the stack is unwound when an
>>>> exception is taken.  All the changes here are done to the runtime code in
>>>> libgcc's unwinder code for Arm target. All the changes are guarded under
>>>> defined (__ARM_FEATURE_PAC_DEFAULT) and activates only if the +pacbti
>>>> feature is switched on for the architecture. This means that switching on the
>>>> target feature via -march or -mcpu is sufficient and -mbranch-protection
>>>> need not be enabled. This ensures that the unwinder is authenticated only if
>>>> the PACBTI instructions are available in the non-NOP space as it uses AUTG.
>>>> Just generating PAC/AUT instructions using -mbranch-protection will not
>>>> enable authentication on the unwinder.
>>>>
>>>> Tested on arm-none-eabi. OK for trunk?
>>>>
>>>> 2021-10-04  Tejas Belagod  <tbelagod@arm.com>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 	* ginclude/unwind-arm-common.h (_Unwind_VRS_RegClass):
>>>> Introduce
>>>> 	new pseudo register class _UVRSC_PAC.
>>>> 	* libgcc/config/arm/pr-support.c (__gnu_unwind_execute): Decode
>>>> 	exception opcode (0xb4) for saving RA_AUTH_CODE and
>>>> authenticate
>>>> 	with AUTG if found.
>>>> 	* libgcc/config/arm/unwind-arm.c (struct pseudo_regs): New.
>>>> 	(phase1_vrs): Introduce new field to store pseudo-reg state.
>>>> 	(phase2_vrs): Likewise.
>>>> 	(_Unwind_VRS_Get): Load pseudo register state from virtual reg set.
>>>> 	(_Unwind_VRS_Set): Store pseudo register state to virtual reg set.
>>>> 	(_Unwind_VRS_Pop): Load pseudo register value from stack into
>>>> VRS.
>>> Rebased and respin based on reviews for previous patches.
>>> This patch adds authentication for when the stack is unwound when
>>> an exception is taken.  All the changes here are done to the runtime
>>> code in libgcc's unwinder code for Arm target. All the changes are
>>> guarded under defined (__ARM_FEATURE_PAUTH) and activates only
>>> if the +pacbti feature is switched on for the architecture. This means
>>> that switching on the target feature via -march or -mcpu is sufficient
>>> and -mbranch-protection need not be enabled. This ensures that the
>>> unwinder is authenticated only if the PACBTI instructions are available
>>> in the non-NOP space as it uses AUTG. Just generating PAC/AUT instructions
>>> using -mbranch-protection will not enable authentication on the unwinder.
>>> 2021-10-25  Tejas Belagod  <tbelagod@arm.com>
>>> gcc/ChangeLog:
>>> 	* ginclude/unwind-arm-common.h (_Unwind_VRS_RegClass):
>>> Introduce
>>> 	new pseudo register class _UVRSC_PAC.
>>> 	* libgcc/config/arm/pr-support.c (__gnu_unwind_execute): Decode
>>> 	exception opcode (0xb4) for saving RA_AUTH_CODE and authenticate
>>> 	with AUTG if found.
>>> 	* libgcc/config/arm/unwind-arm.c (struct pseudo_regs): New.
>>> 	(phase1_vrs): Introduce new field to store pseudo-reg state.
>>> 	(phase2_vrs): Likewise.
>>> 	(_Unwind_VRS_Get): Load pseudo register state from virtual reg set.
>>> 	(_Unwind_VRS_Set): Store pseudo register state to virtual reg set.
>>> 	(_Unwind_VRS_Pop): Load pseudo register value from stack into VRS.
>>> Tested the following configurations, OK for trunk?
>>> -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
>>> -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
>>> mcmodel=small and tiny
>>> aarch64-none-linux-gnu native test and bootstrap
>>> Thanks,
>>> Tejas.
>>>
> 
>> I'd like to try to get rid of most of the ifdefs from this patch; at
>> least, it shouldn't be using the ACLE PAUTH feature.  The unwinder
>> should be able to cope with any unwind sequence thrown at it.
>>
>> Things are a little more complicated for pointer authentication,
>> though, because some operations in the main code constructing the
>> frame may be using architectural NOP instructions, while the unwinder
>> cannot do the validation using only the architectural NOPs.
>>
>> So we need a fall-back: if the unwinder is built without the PAUTH
>> feature it needs to unwind the pauth frames without the additional
>> validation (but it still needs to be able to handle them).
>>
>> So the only remaining question is whether the additional support
>> should only be enabled for M-profile targets, or whether we should
>> just put this code into all builds of the unwinder.  I'm not sure I
>> have a complete answer to that.  My inclination is to put it in
>> unconditionally - we haven't had conditionals for any other optional
>> architecture feature before.  If something similar is added for
>> A/R-profiles, then either we will share the code exactly, or we'll end
>> up with a different unwind code to use as a suitable discriminator.
>>
>> R.
> 
> Hi Richard,
> 
> thanks for reviewing.
> 
> The attached patch implements what I think we want.  That unwinders is
> always able to unwind the stack but will perform authentication only if
> built with PACBTI support.
> 
> WDYT?
> 
> Thanks
> 
>    Andrea
> 


@@ -114,6 +115,22 @@ __gnu_unwind_execute (_Unwind_Context * context, 
__gnu_unwind_state * uws)
        op = next_unwind_byte (uws);
        if (op == CODE_FINISH)
  	{
+	  /* When we reach end, we have to authenticate R12 we just popped 
earlier.  */
+	  if (set_pac)
+	    {
+#if defined(TARGET_HAVE_PACBTI)
+	      _uw sp;
+	      _uw lr;
+	      _uw pac;
+	      _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, &sp);
+	      _Unwind_VRS_Get (context, _UVRSC_CORE, R_LR, _UVRSD_UINT32, &lr);
+	      _Unwind_VRS_Get (context, _UVRSC_PAC, R_IP,
+			       _UVRSD_UINT32, &pac);
+	      __asm__ __volatile__
+		("autg %0, %1, %2" : : "r"(pac), "r"(lr), "r"(sp) ;
+#endif
+	    }
+

You would be better moving the ifdef outside of the 'if (set_pac)' 
clause, which becomes empty otherwise.  Also, I think a comment here is 
warranted that, while the check provides additional security against a 
corrupted unwind chain, it isn't essential for correct unwinding of an 
uncorrupted chain.

Otherwise, this is ok.

R.


More information about the Gcc-patches mailing list