| Message ID | 20200723162354.10301-1-szabolcs.nagy@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] aarch64: add PAC GNU property note to libgcc lse.S | expand |
Hi Szabolcs, > -----Original Message----- > From: Szabolcs Nagy <Szabolcs.Nagy@arm.com> > Sent: 23 July 2020 17:24 > To: gcc-patches@gcc.gnu.org > Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> > Subject: [PATCH 2/2] aarch64: add PAC-RET protection to libitm sjlj.S > > _ITM_beginTransaction is a 'returns_twice' function that saves x30 > on the stack as part of gtm_jmpbuf (that is passed down to > GTM_begin_transaction), but the saved x30 is also used for return. > > The return path should be protected so we don't leave an > ldp x29, x30, [sp] > ret > gadget in the code, so x30 is signed on function entry. This > exposes the signed address in the gtm_jmpbuf too. The jmpbuf does > not need a signed address since GTM_longjmp uses > ldp x29, x30, [x1] > br x30 > and with BTI there is a BTI j at the _ITM_beginTransaction call site > where this jump returns. Using PAC does not hurt: the gtm_jmpbuf is > internal to libitm and its layout is only used by sjlj.S so the > signed address does not escape. Saving signed x30 into gtm_jmpbuf > provides a bit of extra protection, but more importantly it allows > adding the PAC-RET support without changing the existing code much. > > In theory bti and pac-ret protection can be added unconditionally > since the instructions are in the nop space, in practice they > can cause trouble if some tooling does not understand the gnu > property note (e.g. old binutils) or some unwinder or debugger > does not understand the new dwarf op code used for pac-ret (e.g > old gdb). So the code is written to only support branch-protection > according to the code generation options. Ok. Thanks, Kyrill > > libitm/ChangeLog: > > * config/aarch64/sjlj.S: Add conditional pac-ret protection. > --- > libitm/config/aarch64/sjlj.S | 56 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 53 insertions(+), 3 deletions(-) > > diff --git a/libitm/config/aarch64/sjlj.S b/libitm/config/aarch64/sjlj.S > index e2093ca1a97..c84e98aecad 100644 > --- a/libitm/config/aarch64/sjlj.S > +++ b/libitm/config/aarch64/sjlj.S > @@ -25,6 +25,35 @@ > #include "asmcfi.h" > > #define BTI_C hint 34 > +#define PACIASP hint 25 > +#define AUTIASP hint 29 > +#define PACIBSP hint 27 > +#define AUTIBSP hint 31 > + > +#if defined(HAVE_AS_CFI_PSEUDO_OP) && > defined(__GCC_HAVE_DWARF2_CFI_ASM) > +# define cfi_window_save .cfi_window_save > +# define cfi_b_key_frame .cfi_b_key_frame > +#else > +# define cfi_window_save > +# define cfi_b_key_frame > +#endif > + > +#if __ARM_FEATURE_PAC_DEFAULT & 1 > +# define CFI_PAC_TOGGLE cfi_window_save > +# define CFI_PAC_KEY > +# define PAC_AND_BTI PACIASP > +# define AUT AUTIASP > +#elif __ARM_FEATURE_PAC_DEFAULT & 2 > +# define CFI_PAC_TOGGLE cfi_window_save > +# define CFI_PAC_KEY cfi_b_key_frame > +# define PAC_AND_BTI PACIBSP > +# define AUT AUTIBSP > +#else > +# define CFI_PAC_TOGGLE > +# define CFI_PAC_KEY > +# define PAC_AND_BTI BTI_C > +# define AUT > +#endif > > .text > .align 2 > @@ -33,7 +62,9 @@ > > _ITM_beginTransaction: > cfi_startproc > - BTI_C > + CFI_PAC_KEY > + PAC_AND_BTI > + CFI_PAC_TOGGLE > mov x1, sp > stp x29, x30, [sp, -11*16]! > cfi_adjust_cfa_offset(11*16) > @@ -60,6 +91,8 @@ _ITM_beginTransaction: > cfi_adjust_cfa_offset(-11*16) > cfi_restore(x29) > cfi_restore(x30) > + AUT > + CFI_PAC_TOGGLE > ret > cfi_endproc > .size _ITM_beginTransaction, . - _ITM_beginTransaction > @@ -73,6 +106,7 @@ GTM_longjmp: > /* The first parameter becomes the return value (x0). > The third parameter is ignored for now. */ > cfi_startproc > + CFI_PAC_KEY > BTI_C > ldp x19, x20, [x1, 1*16] > ldp x21, x22, [x1, 2*16] > @@ -86,7 +120,10 @@ GTM_longjmp: > ldr x3, [x1, 10*16] > ldp x29, x30, [x1] > cfi_def_cfa(x1, 0) > + CFI_PAC_TOGGLE > mov sp, x3 > + AUT > + CFI_PAC_TOGGLE > br x30 > cfi_endproc > .size GTM_longjmp, . - GTM_longjmp > @@ -96,6 +133,19 @@ GTM_longjmp: > #define FEATURE_1_BTI 1 > #define FEATURE_1_PAC 2 > > +/* Supported features based on the code generation options. */ > +#if defined(__ARM_FEATURE_BTI_DEFAULT) > +# define BTI_FLAG FEATURE_1_BTI > +#else > +# define BTI_FLAG 0 > +#endif > + > +#if __ARM_FEATURE_PAC_DEFAULT & 3 > +# define PAC_FLAG FEATURE_1_PAC > +#else > +# define PAC_FLAG 0 > +#endif > + > /* Add a NT_GNU_PROPERTY_TYPE_0 note. */ > #define GNU_PROPERTY(type, value) \ > .section .note.gnu.property, "a"; \ > @@ -113,7 +163,7 @@ GTM_longjmp: > .section .note.GNU-stack, "", %progbits > > /* Add GNU property note if built with branch protection. */ > -# ifdef __ARM_FEATURE_BTI_DEFAULT > -GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI) > +# if (BTI_FLAG|PAC_FLAG) != 0 > +GNU_PROPERTY (FEATURE_1_AND, BTI_FLAG|PAC_FLAG) > # endif > #endif > -- > 2.17.1
diff --git a/libitm/config/aarch64/sjlj.S b/libitm/config/aarch64/sjlj.S index e2093ca1a97..c84e98aecad 100644 --- a/libitm/config/aarch64/sjlj.S +++ b/libitm/config/aarch64/sjlj.S @@ -25,6 +25,35 @@ #include "asmcfi.h" #define BTI_C hint 34 +#define PACIASP hint 25 +#define AUTIASP hint 29 +#define PACIBSP hint 27 +#define AUTIBSP hint 31 + +#if defined(HAVE_AS_CFI_PSEUDO_OP) && defined(__GCC_HAVE_DWARF2_CFI_ASM) +# define cfi_window_save .cfi_window_save +# define cfi_b_key_frame .cfi_b_key_frame +#else +# define cfi_window_save +# define cfi_b_key_frame +#endif + +#if __ARM_FEATURE_PAC_DEFAULT & 1 +# define CFI_PAC_TOGGLE cfi_window_save +# define CFI_PAC_KEY +# define PAC_AND_BTI PACIASP +# define AUT AUTIASP +#elif __ARM_FEATURE_PAC_DEFAULT & 2 +# define CFI_PAC_TOGGLE cfi_window_save +# define CFI_PAC_KEY cfi_b_key_frame +# define PAC_AND_BTI PACIBSP +# define AUT AUTIBSP +#else +# define CFI_PAC_TOGGLE +# define CFI_PAC_KEY +# define PAC_AND_BTI BTI_C +# define AUT +#endif .text .align 2 @@ -33,7 +62,9 @@ _ITM_beginTransaction: cfi_startproc - BTI_C + CFI_PAC_KEY + PAC_AND_BTI + CFI_PAC_TOGGLE mov x1, sp stp x29, x30, [sp, -11*16]! cfi_adjust_cfa_offset(11*16) @@ -60,6 +91,8 @@ _ITM_beginTransaction: cfi_adjust_cfa_offset(-11*16) cfi_restore(x29) cfi_restore(x30) + AUT + CFI_PAC_TOGGLE ret cfi_endproc .size _ITM_beginTransaction, . - _ITM_beginTransaction @@ -73,6 +106,7 @@ GTM_longjmp: /* The first parameter becomes the return value (x0). The third parameter is ignored for now. */ cfi_startproc + CFI_PAC_KEY BTI_C ldp x19, x20, [x1, 1*16] ldp x21, x22, [x1, 2*16] @@ -86,7 +120,10 @@ GTM_longjmp: ldr x3, [x1, 10*16] ldp x29, x30, [x1] cfi_def_cfa(x1, 0) + CFI_PAC_TOGGLE mov sp, x3 + AUT + CFI_PAC_TOGGLE br x30 cfi_endproc .size GTM_longjmp, . - GTM_longjmp @@ -96,6 +133,19 @@ GTM_longjmp: #define FEATURE_1_BTI 1 #define FEATURE_1_PAC 2 +/* Supported features based on the code generation options. */ +#if defined(__ARM_FEATURE_BTI_DEFAULT) +# define BTI_FLAG FEATURE_1_BTI +#else +# define BTI_FLAG 0 +#endif + +#if __ARM_FEATURE_PAC_DEFAULT & 3 +# define PAC_FLAG FEATURE_1_PAC +#else +# define PAC_FLAG 0 +#endif + /* Add a NT_GNU_PROPERTY_TYPE_0 note. */ #define GNU_PROPERTY(type, value) \ .section .note.gnu.property, "a"; \ @@ -113,7 +163,7 @@ GTM_longjmp: .section .note.GNU-stack, "", %progbits /* Add GNU property note if built with branch protection. */ -# ifdef __ARM_FEATURE_BTI_DEFAULT -GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI) +# if (BTI_FLAG|PAC_FLAG) != 0 +GNU_PROPERTY (FEATURE_1_AND, BTI_FLAG|PAC_FLAG) # endif #endif
_ITM_beginTransaction is a 'returns_twice' function that saves x30 on the stack as part of gtm_jmpbuf (that is passed down to GTM_begin_transaction), but the saved x30 is also used for return. The return path should be protected so we don't leave an ldp x29, x30, [sp] ret gadget in the code, so x30 is signed on function entry. This exposes the signed address in the gtm_jmpbuf too. The jmpbuf does not need a signed address since GTM_longjmp uses ldp x29, x30, [x1] br x30 and with BTI there is a BTI j at the _ITM_beginTransaction call site where this jump returns. Using PAC does not hurt: the gtm_jmpbuf is internal to libitm and its layout is only used by sjlj.S so the signed address does not escape. Saving signed x30 into gtm_jmpbuf provides a bit of extra protection, but more importantly it allows adding the PAC-RET support without changing the existing code much. In theory bti and pac-ret protection can be added unconditionally since the instructions are in the nop space, in practice they can cause trouble if some tooling does not understand the gnu property note (e.g. old binutils) or some unwinder or debugger does not understand the new dwarf op code used for pac-ret (e.g old gdb). So the code is written to only support branch-protection according to the code generation options. libitm/ChangeLog: * config/aarch64/sjlj.S: Add conditional pac-ret protection. --- libitm/config/aarch64/sjlj.S | 56 ++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 3 deletions(-)