diff mbox series

[v6,12/14] aarch64: fix pac-ret support in _mcount

Message ID a688767ffbe00df6bac7e2f2d38801aace571927.1593612309.git.szabolcs.nagy@arm.com
State Committed
Headers
Series aarch64: branch protection support |

Commit Message

Szabolcs Nagy July 1, 2020, 2:40 p.m. UTC 
  Currently gcc -pg -mbranch-protection=pac-ret passes signed return
address to _mcount, so _mcount now has to always strip pac from the
frompc since that's from user code that may be built with pac-ret.

This is gcc PR target/94791: signed pointers should not escape and get
passed across extern call boundaries, since that's an ABI break, but
because existing gcc has this issue we work it around in glibc until
that is resolved. This is compatible with a fixed gcc and it is a nop
on systems without PAuth support. The bug was introduced in gcc-7 with
-msign-return-address=non-leaf|all support which in gcc-9 got renamed
to -mbranch-protection=pac-ret|pac-ret+leaf|standard.

strip_pac uses inline asm instead of __builtin_aarch64_xpaclri since
that is not a documented api and not available in all supported gccs.
---
 sysdeps/aarch64/machine-gmon.h |  3 ++-
 sysdeps/aarch64/sysdep.h       | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Adhemerval Zanella Netto July 6, 2020, 6:33 p.m. UTC | #1 
On 01/07/2020 11:40, Szabolcs Nagy wrote:
> Currently gcc -pg -mbranch-protection=pac-ret passes signed return
> address to _mcount, so _mcount now has to always strip pac from the
> frompc since that's from user code that may be built with pac-ret.
> 
> This is gcc PR target/94791: signed pointers should not escape and get
> passed across extern call boundaries, since that's an ABI break, but
> because existing gcc has this issue we work it around in glibc until
> that is resolved. This is compatible with a fixed gcc and it is a nop
> on systems without PAuth support. The bug was introduced in gcc-7 with
> -msign-return-address=non-leaf|all support which in gcc-9 got renamed
> to -mbranch-protection=pac-ret|pac-ret+leaf|standard.
> 
> strip_pac uses inline asm instead of __builtin_aarch64_xpaclri since
> that is not a documented api and not available in all supported gccs.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/aarch64/machine-gmon.h |  3 ++-
>  sysdeps/aarch64/sysdep.h       | 11 +++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
> index 730a23b781..a687298b1c 100644
> --- a/sysdeps/aarch64/machine-gmon.h
> +++ b/sysdeps/aarch64/machine-gmon.h
> @@ -27,8 +27,9 @@ static void mcount_internal (u_long frompc, u_long selfpc);
>  #define _MCOUNT_DECL(frompc, selfpc) \
>  static inline void mcount_internal (u_long frompc, u_long selfpc)
>  
> +/* Note: strip_pac is needed for frompc because of gcc PR target/94791.  */
>  #define MCOUNT                                                    \
>  void __mcount (void *frompc)                                      \
>  {                                                                 \
> -  mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
> +  mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
>  }

Ok.

> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 500c272745..2879aeaa5c 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -35,6 +35,17 @@
>  
>  #define PTR_SIZE	(1<<PTR_LOG_SIZE)
>  
> +#ifndef __ASSEMBLER__
> +/* Strip pointer authentication code from pointer p.  */
> +static inline void *
> +strip_pac (void *p)
> +{
> +  register void *ra asm ("x30") = (p);
> +  asm ("hint 7 // xpaclri" : "+r"(ra));
> +  return ra;
> +}
> +#endif
> +
>  #ifdef	__ASSEMBLER__
>  
>  /* Syntactic details of assembler.  */
> 

Ok.
diff mbox series

Patch

diff --git a/sysdeps/aarch64/machine-gmon.h b/sysdeps/aarch64/machine-gmon.h
index 730a23b781..a687298b1c 100644
--- a/sysdeps/aarch64/machine-gmon.h
+++ b/sysdeps/aarch64/machine-gmon.h
@@ -27,8 +27,9 @@  static void mcount_internal (u_long frompc, u_long selfpc);
 #define _MCOUNT_DECL(frompc, selfpc) \
 static inline void mcount_internal (u_long frompc, u_long selfpc)
 
+/* Note: strip_pac is needed for frompc because of gcc PR target/94791.  */
 #define MCOUNT                                                    \
 void __mcount (void *frompc)                                      \
 {                                                                 \
-  mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
+  mcount_internal ((u_long) strip_pac (frompc), (u_long) RETURN_ADDRESS (0)); \
 }
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 500c272745..2879aeaa5c 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -35,6 +35,17 @@ 
 
 #define PTR_SIZE	(1<<PTR_LOG_SIZE)
 
+#ifndef __ASSEMBLER__
+/* Strip pointer authentication code from pointer p.  */
+static inline void *
+strip_pac (void *p)
+{
+  register void *ra asm ("x30") = (p);
+  asm ("hint 7 // xpaclri" : "+r"(ra));
+  return ra;
+}
+#endif
+
 #ifdef	__ASSEMBLER__
 
 /* Syntactic details of assembler.  */