diff mbox series

[v7,14/14] aarch64: add NEWS entry about branch protection support

Message ID 90d6dfb05675c96e1f4cd35187d2fb783fc00204.1594209990.git.szabolcs.nagy@arm.com
State New
Headers show
Series aarch64: branch protection support | expand

Commit Message

Szabolcs Nagy July 8, 2020, 12:14 p.m. UTC 
This is a new security feature that relies on architecture
extensions and needs glibc to be built with a gcc configured
with branch protection.
---
 NEWS | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Adhemerval Zanella Netto July 8, 2020, 1:48 p.m. UTC | #1 
On 08/07/2020 09:14, Szabolcs Nagy wrote:
> This is a new security feature that relies on architecture
> extensions and needs glibc to be built with a gcc configured
> with branch protection.

LGTM, thanks. I think I have acked patches, is there still a missing one?
Otherwise I think the patchset it ok to be pushed upstream.

> ---
>  NEWS | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/NEWS b/NEWS
> index d7282b4ad5..5083f5eacf 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -67,6 +67,17 @@ Major new features:
>    They should be used instead of sys_errlist and sys_nerr, both are
>    thread and async-signal safe.  These functions are GNU extensions.
>  
> +* AArch64 now supports standard branch protection security hardening
> +  in glibc when it is built with a GCC that is configured with
> +  --enable-standard-branch-protection.  This includes branch target
> +  identification (BTI) and pointer authentication for return addresses
> +  (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> +  extensions respectively for the protection to be effective,
> +  otherwise the used instructions are nops.  User code can use PAC-RET
> +  without libc support, but BTI requires a libc that is built with BTI
> +  support, otherwise runtime objects linked into user code will not be
> +  BTI compatible.
> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * The deprecated <sys/sysctl.h> header and the sysctl function have been
> 

Ok.
Szabolcs Nagy July 8, 2020, 2:03 p.m. UTC | #2 
The 07/08/2020 10:48, Adhemerval Zanella wrote:
> On 08/07/2020 09:14, Szabolcs Nagy wrote:
> > This is a new security feature that relies on architecture
> > extensions and needs glibc to be built with a gcc configured
> > with branch protection.
> 
> LGTM, thanks. I think I have acked patches, is there still a missing one?
> Otherwise I think the patchset it ok to be pushed upstream.

thanks, pushed the series.
Florian Weimer July 24, 2020, 7:19 a.m. UTC | #3 
* Szabolcs Nagy:

> +* AArch64 now supports standard branch protection security hardening
> +  in glibc when it is built with a GCC that is configured with
> +  --enable-standard-branch-protection.  This includes branch target
> +  identification (BTI) and pointer authentication for return addresses
> +  (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> +  extensions respectively for the protection to be effective,
> +  otherwise the used instructions are nops.  User code can use PAC-RET
> +  without libc support, but BTI requires a libc that is built with BTI
> +  support, otherwise runtime objects linked into user code will not be
> +  BTI compatible.

How can I test whether GCC has been built with
--enable-standard-branch-protection?

We have a Fedora change for this:

  <https://fedoraproject.org/wiki/Changes/Aarch64_PointerAuthentication>

But no GCC update has been submitted for it, and we have not adjust our
glibc build accordingly.

It also doesn't look like libc_nonshared.a is built correctly for this.
In particular, __libc_csu_init (which is linked statically into every
program) does not have any BTI+PAC marker instructions, as far as I can
see:

0000000000000000 <__libc_csu_init>:
   0:   stp     x29, x30, [sp, #-64]!
   4:   mov     x29, sp
   8:   stp     x19, x20, [sp, #16]
   c:   adrp    x20, 0 <__init_array_end>
                        c: R_AARCH64_ADR_PREL_PG_HI21   __init_array_end
  10:   add     x20, x20, #0x0
                        10: R_AARCH64_ADD_ABS_LO12_NC   __init_array_end
  14:   stp     x21, x22, [sp, #32]
  18:   adrp    x21, 0 <__init_array_start>
                        18: R_AARCH64_ADR_PREL_PG_HI21  __init_array_start
  1c:   add     x21, x21, #0x0
                        1c: R_AARCH64_ADD_ABS_LO12_NC   __init_array_start
  20:   sub     x20, x20, x21
  24:   mov     w22, w0
  28:   stp     x23, x24, [sp, #48]
  2c:   mov     x23, x1
  30:   mov     x24, x2
  34:   asr     x20, x20, #3
  38:   bl      0 <_init>
                        38: R_AARCH64_CALL26    _init
  3c:   cbz     x20, 68 <__libc_csu_init+0x68>
  40:   mov     x19, #0x0                       // #0
  44:   nop
  48:   ldr     x3, [x21, x19, lsl #3]
  4c:   mov     x2, x24
  50:   add     x19, x19, #0x1
  54:   mov     x1, x23
  58:   mov     w0, w22
  5c:   blr     x3
  60:   cmp     x20, x19
  64:   b.ne    48 <__libc_csu_init+0x48>  // b.any
  68:   ldp     x19, x20, [sp, #16]
  6c:   ldp     x21, x22, [sp, #32]
  70:   ldp     x23, x24, [sp, #48]
  74:   ldp     x29, x30, [sp], #64
  78:   ret
  7c:   nop

Is there an alternative to enabling this for glibc (and elsewhere)
without a special build of GCC?  Or will this still not work because
without --enable-standard-branch-protection for GCC, libgcc.a is not
ready?

Thanks,
Florian
Szabolcs Nagy July 24, 2020, 7:50 a.m. UTC | #4 
The 07/24/2020 09:19, Florian Weimer wrote:
> * Szabolcs Nagy:
> > +* AArch64 now supports standard branch protection security hardening
> > +  in glibc when it is built with a GCC that is configured with
> > +  --enable-standard-branch-protection.  This includes branch target
> > +  identification (BTI) and pointer authentication for return addresses
> > +  (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> > +  extensions respectively for the protection to be effective,
> > +  otherwise the used instructions are nops.  User code can use PAC-RET
> > +  without libc support, but BTI requires a libc that is built with BTI
> > +  support, otherwise runtime objects linked into user code will not be
> > +  BTI compatible.
> 
> How can I test whether GCC has been built with
> --enable-standard-branch-protection?

unfortunately in gcc-10.1 and <= gcc-9.3 there was no easy
check for this, that's why i have complicated configure
tests for it in glibc: right now you need to link some
program that uses gcc target objects and see if it has
the property notes, in gcc-10.2 i added the acle macros

https://developer.arm.com/documentation/101028/0011/Feature-test-macros?lang=en

__ARM_FEATURE_DEFAULT_BTI
__ARM_FEATURE_DEFAULT_PAC

which are set when the code is compiled with

 -mbranch-protection=bti|pac-ret|standard

so at least the code generation can be checked.

> 
> We have a Fedora change for this:
> 
>   <https://fedoraproject.org/wiki/Changes/Aarch64_PointerAuthentication>
> 
> But no GCC update has been submitted for it, and we have not adjust our
> glibc build accordingly.
> 
> It also doesn't look like libc_nonshared.a is built correctly for this.
> In particular, __libc_csu_init (which is linked statically into every
> program) does not have any BTI+PAC marker instructions, as far as I can
> see:
> 
> 0000000000000000 <__libc_csu_init>:
>    0:   stp     x29, x30, [sp, #-64]!
>    4:   mov     x29, sp
>    8:   stp     x19, x20, [sp, #16]
>    c:   adrp    x20, 0 <__init_array_end>
>                         c: R_AARCH64_ADR_PREL_PG_HI21   __init_array_end
>   10:   add     x20, x20, #0x0
>                         10: R_AARCH64_ADD_ABS_LO12_NC   __init_array_end
>   14:   stp     x21, x22, [sp, #32]
>   18:   adrp    x21, 0 <__init_array_start>
>                         18: R_AARCH64_ADR_PREL_PG_HI21  __init_array_start
>   1c:   add     x21, x21, #0x0
>                         1c: R_AARCH64_ADD_ABS_LO12_NC   __init_array_start
>   20:   sub     x20, x20, x21
>   24:   mov     w22, w0
>   28:   stp     x23, x24, [sp, #48]
>   2c:   mov     x23, x1
>   30:   mov     x24, x2
>   34:   asr     x20, x20, #3
>   38:   bl      0 <_init>
>                         38: R_AARCH64_CALL26    _init
>   3c:   cbz     x20, 68 <__libc_csu_init+0x68>
>   40:   mov     x19, #0x0                       // #0
>   44:   nop
>   48:   ldr     x3, [x21, x19, lsl #3]
>   4c:   mov     x2, x24
>   50:   add     x19, x19, #0x1
>   54:   mov     x1, x23
>   58:   mov     w0, w22
>   5c:   blr     x3
>   60:   cmp     x20, x19
>   64:   b.ne    48 <__libc_csu_init+0x48>  // b.any
>   68:   ldp     x19, x20, [sp, #16]
>   6c:   ldp     x21, x22, [sp, #32]
>   70:   ldp     x23, x24, [sp, #48]
>   74:   ldp     x29, x30, [sp], #64
>   78:   ret
>   7c:   nop

gcc-10.1 with --enable-standard-branch-protection gives me

0000000000000000 <__libc_csu_init>:
   0:   d503233f        paciasp
   4:   a9bc7bfd        stp     x29, x30, [sp, #-64]!
   8:   910003fd        mov     x29, sp
   c:   a90153f3        stp     x19, x20, [sp, #16]
  10:   90000014        adrp    x20, 0 <__init_array_end>       10: R_AARCH64_ADR_PREL_PG_HI21  __init_array_end
  14:   91000294        add     x20, x20, #0x0  14: R_AARCH64_ADD_ABS_LO12_NC   __init_array_end
  18:   a9025bf5        stp     x21, x22, [sp, #32]
  1c:   90000015        adrp    x21, 0 <__init_array_start>     1c: R_AARCH64_ADR_PREL_PG_HI21  __init_array_start
  20:   910002b5        add     x21, x21, #0x0  20: R_AARCH64_ADD_ABS_LO12_NC   __init_array_start
  24:   cb150294        sub     x20, x20, x21
  28:   2a0003f6        mov     w22, w0
  2c:   a90363f7        stp     x23, x24, [sp, #48]
  30:   aa0103f7        mov     x23, x1
  34:   aa0203f8        mov     x24, x2
  38:   9343fe94        asr     x20, x20, #3
  3c:   94000000        bl      0 <_init>       3c: R_AARCH64_CALL26    _init
  40:   b4000154        cbz     x20, 68 <__libc_csu_init+0x68>
  44:   d2800013        mov     x19, #0x0                       // #0
  48:   f8737aa3        ldr     x3, [x21, x19, lsl #3]
  4c:   aa1803e2        mov     x2, x24
  50:   91000673        add     x19, x19, #0x1
  54:   aa1703e1        mov     x1, x23
  58:   2a1603e0        mov     w0, w22
  5c:   d63f0060        blr     x3
  60:   eb13029f        cmp     x20, x19
  64:   54ffff21        b.ne    48 <__libc_csu_init+0x48>  // b.any
  68:   a94153f3        ldp     x19, x20, [sp, #16]
  6c:   a9425bf5        ldp     x21, x22, [sp, #32]
  70:   a94363f7        ldp     x23, x24, [sp, #48]
  74:   a8c47bfd        ldp     x29, x30, [sp], #64
  78:   d50323bf        autiasp
  7c:   d65f03c0        ret

> 
> Is there an alternative to enabling this for glibc (and elsewhere)
> without a special build of GCC?  Or will this still not work because
> without --enable-standard-branch-protection for GCC, libgcc.a is not
> ready?

i require --enable-standard-branch-protection exactly
because gcc target libs and crt files are not branch
protected by default, this can change later (always
build target libs with branch protection), but i
consider it risky given that the pac-ret instructions
can be mishandled by custom unwinders who don't know
the new dwarf op code for it or tools that don't
understand the gnu property notes (note handling in
binutils can be checked at gcc build time, but other
things cannot be, so there will be some risk when using
a gcc with branch-protection and it will take some time
to find the issues given the lack of hardware)
Florian Weimer July 24, 2020, 8:06 a.m. UTC | #5 
* Szabolcs Nagy:

> i require --enable-standard-branch-protection exactly
> because gcc target libs and crt files are not branch
> protected by default, this can change later (always
> build target libs with branch protection), but i
> consider it risky given that the pac-ret instructions
> can be mishandled by custom unwinders who don't know
> the new dwarf op code for it or tools that don't
> understand the gnu property notes (note handling in
> binutils can be checked at gcc build time, but other
> things cannot be, so there will be some risk when using
> a gcc with branch-protection and it will take some time
> to find the issues given the lack of hardware)

Okay, then the NEWS entry is appriate, and we did not follow the
required steps for enabling this feature in Fedora 33. 8-(

Thanks,
Florian
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index d7282b4ad5..5083f5eacf 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,17 @@  Major new features:
   They should be used instead of sys_errlist and sys_nerr, both are
   thread and async-signal safe.  These functions are GNU extensions.
 
+* AArch64 now supports standard branch protection security hardening
+  in glibc when it is built with a GCC that is configured with
+  --enable-standard-branch-protection.  This includes branch target
+  identification (BTI) and pointer authentication for return addresses
+  (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
+  extensions respectively for the protection to be effective,
+  otherwise the used instructions are nops.  User code can use PAC-RET
+  without libc support, but BTI requires a libc that is built with BTI
+  support, otherwise runtime objects linked into user code will not be
+  BTI compatible.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * The deprecated <sys/sysctl.h> header and the sysctl function have been