[v3,05/13] aarch64: Add BTI support to assembly files

Message ID 64e9a422a1ef370334b90a591229f7f4e293bf8c.1589552054.git.szabolcs.nagy@arm.com
State Superseded
Headers
Series aarch64: branch protection support |

Commit Message

Szabolcs Nagy May 15, 2020, 2:40 p.m. UTC
  From: Sudakshina Das <sudi.das@arm.com>

To enable building glibc with branch protection, assembly code
needs BTI landing pads and ELF object file markings in the form
of a GNU property note.

The landing pads are unconditionally added to all functions that
may be indirectly called. When the code segment is not mapped
with PROT_BTI these instructions are nops. They are kept in the
code when BTI is not supported so that the layout of performance
critical code is unchanged across configurations.

The GNU property notes are only added when there is support for
BTI in the toolchain, because old binutils does not handle the
notes right. (Does not know how to merge them nor to put them in
PT_GNU_PROPERTY segment instead of PT_NOTE, and some versions
of binutils emit warnings about the unknown GNU property. In
such cases the produced libc binaries would not have valid
ELF marking so BTI would not be enabled.)

Note: functions using ENTRY or ENTRY_ALIGN now start with an
additional BTI c, so alignment of the following code changes,
but ENTRY_ALIGN_AND_PAD was fixed so there is no change to the
existing code layout. Some string functions may need to be
tuned for optimal performance after this commit.

Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
 sysdeps/aarch64/crti.S          |  2 ++
 sysdeps/aarch64/crtn.S          |  2 ++
 sysdeps/aarch64/dl-tlsdesc.S    |  3 +++
 sysdeps/aarch64/dl-trampoline.S |  2 ++
 sysdeps/aarch64/start.S         |  1 +
 sysdeps/aarch64/sysdep.h        | 34 ++++++++++++++++++++++++++++++++-
 6 files changed, 43 insertions(+), 1 deletion(-)
  

Comments

Adhemerval Zanella May 25, 2020, 6:49 p.m. UTC | #1
On 15/05/2020 11:40, Szabolcs Nagy wrote:
> From: Sudakshina Das <sudi.das@arm.com>
> 
> To enable building glibc with branch protection, assembly code
> needs BTI landing pads and ELF object file markings in the form
> of a GNU property note.
> 
> The landing pads are unconditionally added to all functions that
> may be indirectly called. When the code segment is not mapped
> with PROT_BTI these instructions are nops. They are kept in the
> code when BTI is not supported so that the layout of performance
> critical code is unchanged across configurations.
> 
> The GNU property notes are only added when there is support for
> BTI in the toolchain, because old binutils does not handle the
> notes right. (Does not know how to merge them nor to put them in
> PT_GNU_PROPERTY segment instead of PT_NOTE, and some versions
> of binutils emit warnings about the unknown GNU property. In
> such cases the produced libc binaries would not have valid
> ELF marking so BTI would not be enabled.)
> 
> Note: functions using ENTRY or ENTRY_ALIGN now start with an
> additional BTI c, so alignment of the following code changes,
> but ENTRY_ALIGN_AND_PAD was fixed so there is no change to the
> existing code layout. Some string functions may need to be
> tuned for optimal performance after this commit.
> 
> Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>

LGTM with a small change with depends on previous commit change
(with I also added a comment).

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

> ---
>  sysdeps/aarch64/crti.S          |  2 ++
>  sysdeps/aarch64/crtn.S          |  2 ++
>  sysdeps/aarch64/dl-tlsdesc.S    |  3 +++
>  sysdeps/aarch64/dl-trampoline.S |  2 ++
>  sysdeps/aarch64/start.S         |  1 +
>  sysdeps/aarch64/sysdep.h        | 34 ++++++++++++++++++++++++++++++++-
>  6 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
> index 1728eac37a..c346bcad72 100644
> --- a/sysdeps/aarch64/crti.S
> +++ b/sysdeps/aarch64/crti.S
> @@ -75,6 +75,7 @@ call_weak_fn:
>  	.hidden	_init
>  	.type	_init, %function
>  _init:
> +	BTI_C
>  	stp	x29, x30, [sp, -16]!
>  	mov	x29, sp
>  #if PREINIT_FUNCTION_WEAK
> @@ -89,5 +90,6 @@ _init:
>  	.hidden	_fini
>  	.type	_fini, %function
>  _fini:
> +	BTI_C
>  	stp	x29, x30, [sp, -16]!
>  	mov	x29, sp

Ok.

> diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
> index c3e97cc449..0c1ef112c2 100644
> --- a/sysdeps/aarch64/crtn.S
> +++ b/sysdeps/aarch64/crtn.S
> @@ -37,6 +37,8 @@
>  /* crtn.S puts function epilogues in the .init and .fini sections
>     corresponding to the prologues in crti.S. */
>  
> +#include <sysdep.h>
> +
>  	.section .init,"ax",%progbits
>  	ldp	x29, x30, [sp], 16
>  	RET

Ok.

> diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
> index 557ad1d505..9d96c8632a 100644
> --- a/sysdeps/aarch64/dl-tlsdesc.S
> +++ b/sysdeps/aarch64/dl-tlsdesc.S
> @@ -74,6 +74,7 @@
>  	cfi_startproc
>  	.align 2
>  _dl_tlsdesc_return:
> +	BTI_C
>  	DELOUSE (0)
>  	ldr	PTR_REG (0), [x0, #PTR_SIZE]
>  	RET
> @@ -95,6 +96,7 @@ _dl_tlsdesc_return:
>  	cfi_startproc
>  	.align  2
>  _dl_tlsdesc_undefweak:
> +	BTI_C
>  	str	x1, [sp, #-16]!
>  	cfi_adjust_cfa_offset (16)
>  	DELOUSE (0)
> @@ -142,6 +144,7 @@ _dl_tlsdesc_undefweak:
>  	cfi_startproc
>  	.align 2
>  _dl_tlsdesc_dynamic:
> +	BTI_C
>  	DELOUSE (0)
>  
>  	/* Save just enough registers to support fast path, if we fall

Ok.

> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
> index 94e965c096..2cbfa81434 100644
> --- a/sysdeps/aarch64/dl-trampoline.S
> +++ b/sysdeps/aarch64/dl-trampoline.S
> @@ -35,6 +35,7 @@
>  	cfi_startproc
>  	.align 2
>  _dl_runtime_resolve:
> +	BTI_C
>  	/* AArch64 we get called with:
>  	   ip0		&PLTGOT[2]
>  	   ip1		temp(dl resolver entry point)
> @@ -126,6 +127,7 @@ _dl_runtime_resolve:
>  	cfi_startproc
>  	.align 2
>  _dl_runtime_profile:
> +	BTI_C
>  	/* AArch64 we get called with:
>  	   ip0		&PLTGOT[2]
>  	   ip1		temp(dl resolver entry point)

OK.

> diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> index d96cf57e2d..75393e1c18 100644
> --- a/sysdeps/aarch64/start.S
> +++ b/sysdeps/aarch64/start.S
> @@ -46,6 +46,7 @@
>  	.globl _start
>  	.type _start,#function
>  _start:
> +	BTI_C
>  	/* Create an initial frame with 0 LR and FP */
>  	mov	x29, #0
>  	mov	x30, #0

OK.

> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 604c489170..086fc84b53 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -41,12 +41,42 @@
>  
>  #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
>  
> +/* Branch Target Identitication support.  */
> +#define BTI_C		hint	34
> +#define BTI_J		hint	36
> +
> +/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code.  */
> +#define FEATURE_1_AND 0xc0000000
> +#define FEATURE_1_BTI 1
> +#define FEATURE_1_PAC 2
> +
> +/* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
> +#define GNU_PROPERTY(type, value)	\
> +  .section .note.gnu.property, "a";	\
> +  .p2align 3;				\
> +  .word 4;				\
> +  .word 16;				\
> +  .word 5;				\
> +  .asciz "GNU";				\
> +  .word type;				\
> +  .word 4;				\
> +  .word value;				\
> +  .word 0;				\
> +  .text
> +
> +/* Add GNU property note with the supported features to all asm code
> +   where sysdep.h is included.  */
> +#if defined HAVE_AARCH64_BTI
> +GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
> +#endif

By defining the default value as 0 I think you can check with

  #if HAVE_AARCH64_BTI

> +
>  /* Define an entry point visible from C.  */
>  #define ENTRY(name)						\
>    .globl C_SYMBOL_NAME(name);					\
>    .type C_SYMBOL_NAME(name),%function;				\
>    .align 4;							\
>    C_LABEL(name)							\
> +  BTI_C;							\
>    cfi_startproc;						\
>    CALL_MCOUNT
>  
> @@ -56,6 +86,7 @@
>    .type C_SYMBOL_NAME(name),%function;				\
>    .p2align align;						\
>    C_LABEL(name)							\
> +  BTI_C;							\
>    cfi_startproc;						\
>    CALL_MCOUNT
>  
> @@ -68,10 +99,11 @@
>    .globl C_SYMBOL_NAME(name);					\
>    .type C_SYMBOL_NAME(name),%function;				\
>    .p2align align;						\
> -  .rep padding;							\
> +  .rep padding - 1; /* -1 for bti c.  */			\
>    nop;								\
>    .endr;							\
>    C_LABEL(name)							\
> +  BTI_C;							\
>    cfi_startproc;						\
>    CALL_MCOUNT
>  
> 

Ok.
  

Patch

diff --git a/sysdeps/aarch64/crti.S b/sysdeps/aarch64/crti.S
index 1728eac37a..c346bcad72 100644
--- a/sysdeps/aarch64/crti.S
+++ b/sysdeps/aarch64/crti.S
@@ -75,6 +75,7 @@  call_weak_fn:
 	.hidden	_init
 	.type	_init, %function
 _init:
+	BTI_C
 	stp	x29, x30, [sp, -16]!
 	mov	x29, sp
 #if PREINIT_FUNCTION_WEAK
@@ -89,5 +90,6 @@  _init:
 	.hidden	_fini
 	.type	_fini, %function
 _fini:
+	BTI_C
 	stp	x29, x30, [sp, -16]!
 	mov	x29, sp
diff --git a/sysdeps/aarch64/crtn.S b/sysdeps/aarch64/crtn.S
index c3e97cc449..0c1ef112c2 100644
--- a/sysdeps/aarch64/crtn.S
+++ b/sysdeps/aarch64/crtn.S
@@ -37,6 +37,8 @@ 
 /* crtn.S puts function epilogues in the .init and .fini sections
    corresponding to the prologues in crti.S. */
 
+#include <sysdep.h>
+
 	.section .init,"ax",%progbits
 	ldp	x29, x30, [sp], 16
 	RET
diff --git a/sysdeps/aarch64/dl-tlsdesc.S b/sysdeps/aarch64/dl-tlsdesc.S
index 557ad1d505..9d96c8632a 100644
--- a/sysdeps/aarch64/dl-tlsdesc.S
+++ b/sysdeps/aarch64/dl-tlsdesc.S
@@ -74,6 +74,7 @@ 
 	cfi_startproc
 	.align 2
 _dl_tlsdesc_return:
+	BTI_C
 	DELOUSE (0)
 	ldr	PTR_REG (0), [x0, #PTR_SIZE]
 	RET
@@ -95,6 +96,7 @@  _dl_tlsdesc_return:
 	cfi_startproc
 	.align  2
 _dl_tlsdesc_undefweak:
+	BTI_C
 	str	x1, [sp, #-16]!
 	cfi_adjust_cfa_offset (16)
 	DELOUSE (0)
@@ -142,6 +144,7 @@  _dl_tlsdesc_undefweak:
 	cfi_startproc
 	.align 2
 _dl_tlsdesc_dynamic:
+	BTI_C
 	DELOUSE (0)
 
 	/* Save just enough registers to support fast path, if we fall
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 94e965c096..2cbfa81434 100644
--- a/sysdeps/aarch64/dl-trampoline.S
+++ b/sysdeps/aarch64/dl-trampoline.S
@@ -35,6 +35,7 @@ 
 	cfi_startproc
 	.align 2
 _dl_runtime_resolve:
+	BTI_C
 	/* AArch64 we get called with:
 	   ip0		&PLTGOT[2]
 	   ip1		temp(dl resolver entry point)
@@ -126,6 +127,7 @@  _dl_runtime_resolve:
 	cfi_startproc
 	.align 2
 _dl_runtime_profile:
+	BTI_C
 	/* AArch64 we get called with:
 	   ip0		&PLTGOT[2]
 	   ip1		temp(dl resolver entry point)
diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
index d96cf57e2d..75393e1c18 100644
--- a/sysdeps/aarch64/start.S
+++ b/sysdeps/aarch64/start.S
@@ -46,6 +46,7 @@ 
 	.globl _start
 	.type _start,#function
 _start:
+	BTI_C
 	/* Create an initial frame with 0 LR and FP */
 	mov	x29, #0
 	mov	x30, #0
diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 604c489170..086fc84b53 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -41,12 +41,42 @@ 
 
 #define ASM_SIZE_DIRECTIVE(name) .size name,.-name
 
+/* Branch Target Identitication support.  */
+#define BTI_C		hint	34
+#define BTI_J		hint	36
+
+/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code.  */
+#define FEATURE_1_AND 0xc0000000
+#define FEATURE_1_BTI 1
+#define FEATURE_1_PAC 2
+
+/* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
+#define GNU_PROPERTY(type, value)	\
+  .section .note.gnu.property, "a";	\
+  .p2align 3;				\
+  .word 4;				\
+  .word 16;				\
+  .word 5;				\
+  .asciz "GNU";				\
+  .word type;				\
+  .word 4;				\
+  .word value;				\
+  .word 0;				\
+  .text
+
+/* Add GNU property note with the supported features to all asm code
+   where sysdep.h is included.  */
+#if defined HAVE_AARCH64_BTI
+GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
+#endif
+
 /* Define an entry point visible from C.  */
 #define ENTRY(name)						\
   .globl C_SYMBOL_NAME(name);					\
   .type C_SYMBOL_NAME(name),%function;				\
   .align 4;							\
   C_LABEL(name)							\
+  BTI_C;							\
   cfi_startproc;						\
   CALL_MCOUNT
 
@@ -56,6 +86,7 @@ 
   .type C_SYMBOL_NAME(name),%function;				\
   .p2align align;						\
   C_LABEL(name)							\
+  BTI_C;							\
   cfi_startproc;						\
   CALL_MCOUNT
 
@@ -68,10 +99,11 @@ 
   .globl C_SYMBOL_NAME(name);					\
   .type C_SYMBOL_NAME(name),%function;				\
   .p2align align;						\
-  .rep padding;							\
+  .rep padding - 1; /* -1 for bti c.  */			\
   nop;								\
   .endr;							\
   C_LABEL(name)							\
+  BTI_C;							\
   cfi_startproc;						\
   CALL_MCOUNT