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....@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 <tbela...@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 <tbela...@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.
