[RFC,AARCH64] Machine descriptions to support stack smashing protection

Hi Maintainers,

This is RFC patch that adds machine descriptions to support stack
smashing protection in AArch64.

I have written a very simple patch that prints "stack set" and "stack
test" as template of instructions.

I had 2 assumptions.

1) For "stack_protect_set" and "stack_protect_test", I
used "memory_operand" as predicate.

GCC pushes the memory operand in a register much
earlier during expand phase before these patterns are invoked.

So assuming that I will get a memory operand "__stack_chk_gaurd" in a
register when we are not using TLS based stack guard.

2) For the TLS case, assuming stack guard value will be stored at "-8"
offset from "tp" GCC generates below code for stack set.


        mrs     x0, tpidr_el0
        ldr     x1, [x0,-8]
        str     x1, [x29,24]
        mov     x1,0

I submitted Glibc patches some time before
https://sourceware.org/ml/libc-ports/2013-08/msg00044.html.

There are few regressions, the pthread_cancel tests in glibc fails I
am currently debugging :(.

GCC with the patch generates below code for stack test

        ldr     x1, [x29,24]
        ldr     x0, [x0,-8]
        eor     x0, x1, x0
        cbnz    x0, .L4
.................................
......................................
.L4:
        bl      __stack_chk_f


I generate "eor" since it has 2 purpose one for checking equality, and
 two  for clearing the canary loaded register.

Request your feedback to shape this into a better patch.

regards,
Venkat.

On Tue, Nov 19, 2013 at 04:30:21PM +0530, Venkataramanan Kumar wrote:
> This is RFC patch that adds machine descriptions to support stack
> smashing protection in AArch64.

Most of the testsuite changes look wrong.  The fact that aarch64
gets stack protector support doesn't mean all other targets do as well.
So please leave all the changes that remove native or stack_protector
requirements out.

	Jakub

On Tue, 19 Nov 2013, Jakub Jelinek wrote:

> On Tue, Nov 19, 2013 at 04:30:21PM +0530, Venkataramanan Kumar wrote:
> > This is RFC patch that adds machine descriptions to support stack
> > smashing protection in AArch64.
> 
> Most of the testsuite changes look wrong.  The fact that aarch64
> gets stack protector support doesn't mean all other targets do as well.
> So please leave all the changes that remove native or stack_protector
> requirements out.

The tests for "target native" look wrong for me ("native" conditionals 
only make sense for special cases such as guality tests that expect to 
exec another tool on the host) - so I think they should be removed, but 
that removal needs submitting as a separate patch.

I would like to see a clear description of what happens with all eight 
combinations of (glibc version does or doesn't support this, GCC building 
glibc does or doesn't support this, GCC building user program does or 
doesn't support this).  Which of the (GCC building glibc, glibc) 
combinations will successfully build glibc?  Will all such glibcs be 
binary-compatible?  Will both old and new GCC work for building user 
programs with both old and new glibc?

Hi Joseph,

On 19 November 2013 21:53, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Tue, 19 Nov 2013, Jakub Jelinek wrote:
>
>> On Tue, Nov 19, 2013 at 04:30:21PM +0530, Venkataramanan Kumar wrote:
>> > This is RFC patch that adds machine descriptions to support stack
>> > smashing protection in AArch64.
>>
>> Most of the testsuite changes look wrong.  The fact that aarch64
>> gets stack protector support doesn't mean all other targets do as well.
>> So please leave all the changes that remove native or stack_protector
>> requirements out.
>
> The tests for "target native" look wrong for me ("native" conditionals
> only make sense for special cases such as guality tests that expect to
> exec another tool on the host) - so I think they should be removed, but
> that removal needs submitting as a separate patch.
>

Yes apologies for that, I will send another patch.

> I would like to see a clear description of what happens with all eight
> combinations of (glibc version does or doesn't support this, GCC building
> glibc does or doesn't support this, GCC building user program does or
> doesn't support this).  Which of the (GCC building glibc, glibc)
> combinations will successfully build glibc?  Will all such glibcs be
> binary-compatible?  Will both old and new GCC work for building user
> programs with both old and new glibc?

Can you please clarify why we need to consider "the gcc compiler that
builds the glibc" in the combinations you want me to describe. I am
not able to understand that.

I tested the below three GCC compiler versions for building user programs.

1) GCC trunk (without my patch) generates code for "stack protection
set/test" when -fstack-protector-all is enabled.
This is based on global variable  "__stack_chk_guard" access, which
glibc supports from version 2.4.

-----snip-----
         adrp    x0, __stack_chk_guard
         add     x0, x0, :lo12:__stack_chk_guard
         ldr     x0, [x0]
         str     x0, [x29,24]
-----snip-----

GCC has generic code emitted for stack protection, enabled for targets
where frame growth is downwards. The patch for frame growing downwards
in Aarch64 is recently committed in trunk.

2) Now with the patch, GCC compiler will generate TLS based access.

----snip-----
   mrs     x0, tpidr_el0
   ldr     x1, [x0,-8]
   str     x1, [x29,24]
-----snip-----

I need to check if target glibc for Aarch64 supports TLS based ssp
support. Currently some targets check and generate TLS based access
when glibc version is >=2.4.

3) GCC linaro compiler packaged with open embedded image and which I
boot in V8 foundation model as I don't have access to Aarch64 hardware
yet.

It will not emit any stack protection code.
test.c:1:0: warning: -fstack-protector not supported for this target
[enabled by default]

regards,
Venkat.

On Wed, 20 Nov 2013, Venkataramanan Kumar wrote:

> > I would like to see a clear description of what happens with all eight
> > combinations of (glibc version does or doesn't support this, GCC building
> > glibc does or doesn't support this, GCC building user program does or
> > doesn't support this).  Which of the (GCC building glibc, glibc)
> > combinations will successfully build glibc?  Will all such glibcs be
> > binary-compatible?  Will both old and new GCC work for building user
> > programs with both old and new glibc?
> 
> Can you please clarify why we need to consider "the gcc compiler that
> builds the glibc" in the combinations you want me to describe. I am
> not able to understand that.

Let's imagine this support goes in GCC 4.9 and the glibc support goes in 
glibc 2.19, whereas GCC 4.8 and glibc 2.18 are versions without this 
support.

* Building glibc 2.18 with GCC 4.8 already works (I presume).

* Suppose you use GCC 4.9 to build glibc 2.18.  Does this work?  If it 
works, it must produce a glibc binary that's ABI compatible with one built 
with GCC 4.8, meaning same symbols exported at same symbol versions.

* Suppose you build glibc 2.19 with GCC 4.8.  Does this work?  If it does, 
then it must be ABI compatible with 2.18 (meaning the symbols exported at 
GLIBC_2.18 or earlier versions must be exactly the same as exported at 
those versions in 2.18).

* Suppose you build glibc 2.19 with GCC 4.9.  This needs to work and must 
again produce a binary compatible with the previous ones.

Note: there is no current glibc support for architectures that gained 
TLS-based stack guards after 2.4 (meaning they need both a TLS-based 
scheme and backwards compatibility for binaries using __stack_chk_guard), 
and your glibc patch doesn't seem to add any.  It looks to me like your 
glibc patch would have removed the __stack_chk_guard symbol and so failed 
ABI tests (this symbol being defined only if THREAD_SET_STACK_GUARD isn't 
defined) - you didn't make clear if your patch was tested with the glibc 
testsuite including passing the ABI tests.  The normal presumption is that 
it's not acceptable to remove exported symbols from glibc as some 
application might be using them.

You need to ensure that, when new glibc is built, whatever compiler it's 
built with, it continues to export the __stack_chk_guard symbol at version 
GLIBC_2.17.  Furthermore, if any released GCC version would generate 
references to __stack_chk_guard when compiling code for AArch64 with stack 
protection, you need to ensure __stack_chk_guard is a normal symbol not a 
compat symbol so that people can continue to link against new glibc when 
using old GCC.  If it's only a limited range of development versions of 
GCC that could have generated code using __stack_chk_guard because 
released versions didn't support stack protection on AArch64 at all, a 
compat symbol would probably be OK (but you should still ensure that the 
variable gets initialized with the correct value for any applications 
built with those development versions of GCC).

Hi Joseph,

Thank you for the detail explanation.

> You need to ensure that, when new glibc is built, whatever compiler it's
> built with, it continues to export the __stack_chk_guard symbol at version
> GLIBC_2.17.  Furthermore, if any released GCC version would generate
> references to __stack_chk_guard when compiling code for AArch64 with stack
> protection, you need to ensure __stack_chk_guard is a normal symbol not a
> compat symbol so that people can continue to link against new glibc when
> using old GCC.  If it's only a limited range of development versions of
> GCC that could have generated code using __stack_chk_guard because
> released versions didn't support stack protection on AArch64 at all, a
> compat symbol would probably be OK (but you should still ensure that the
> variable gets initialized with the correct value for any applications
> built with those development versions of GCC).

As you said when THREAD_SET_STACK_GUARD is set glibc does not export
__stack_chk_guard. So I am planning to change the export logic by
adding a new macro EXPORT_GLOBAL_STACK_GUARD
and set it for Aarch64 port in glibc.

----snip----
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
-# ifndef THREAD_SET_STACK_GUARD
+
+#if !defined(THREAD_SET_STACK_GUARD) || defined(EXPORT_GLOBAL_STACK_GUARD)
 /* Only exported for architectures that don't store the stack guard canary
    in thread local area.  */
 uintptr_t __stack_chk_guard attribute_relro;
-# endif
+#endif
+
----snip----

I will find a better way to version that symbol as well. I will sent a
RFC patch to glibc mailing list.

On the GCC side, trunk GCC configure script checks and sets
TARGET_LIBC_PROVIDES_SSP support when glibc is >=2.4

-----snip----
# Test for stack protector support in target C library.
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking __stack_chk_fail in
target C library" >&5
$as_echo_n "checking __stack_chk_fail in target C library... " >&6; }
if test "${gcc_cv_libc_provides_ssp+set}" = set; then :
  $as_echo_n "(cached) " >&6
else
  gcc_cv_libc_provides_ssp=no
    case "$target" in
       *-*-linux* | *-*-kfreebsd*-gnu | *-*-knetbsd*-gnu)
      # glibc 2.4 and later provides __stack_chk_fail and
      # either __stack_chk_guard, or TLS access to stack guard canary.

if test $glibc_version_major -gt 2 \
  || ( test $glibc_version_major -eq 2 && test $glibc_version_minor
-ge 4 ); then :
  gcc_cv_libc_provides_ssp=yes


if test x$gcc_cv_libc_provides_ssp = xyes; then

$as_echo "#define TARGET_LIBC_PROVIDES_SSP 1" >>confdefs.h
fi
----snip-----

To make GCC for AArch64 generate TLS based stack access for glibc >=
2.19 I need to introduce a new macro
TARGET_LIBC_PROVIDES_TLS_SSP and check and set it for glibc >= 2.19 in
GCC configure .

Any better approach to this since it is specific to Aarch64?

regards,
Venkat.

On 20 November 2013 22:38, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Wed, 20 Nov 2013, Venkataramanan Kumar wrote:
>
>> > I would like to see a clear description of what happens with all eight
>> > combinations of (glibc version does or doesn't support this, GCC building
>> > glibc does or doesn't support this, GCC building user program does or
>> > doesn't support this).  Which of the (GCC building glibc, glibc)
>> > combinations will successfully build glibc?  Will all such glibcs be
>> > binary-compatible?  Will both old and new GCC work for building user
>> > programs with both old and new glibc?
>>
>> Can you please clarify why we need to consider "the gcc compiler that
>> builds the glibc" in the combinations you want me to describe. I am
>> not able to understand that.
>
> Let's imagine this support goes in GCC 4.9 and the glibc support goes in
> glibc 2.19, whereas GCC 4.8 and glibc 2.18 are versions without this
> support.
>
> * Building glibc 2.18 with GCC 4.8 already works (I presume).
>
> * Suppose you use GCC 4.9 to build glibc 2.18.  Does this work?  If it
> works, it must produce a glibc binary that's ABI compatible with one built
> with GCC 4.8, meaning same symbols exported at same symbol versions.
>
> * Suppose you build glibc 2.19 with GCC 4.8.  Does this work?  If it does,
> then it must be ABI compatible with 2.18 (meaning the symbols exported at
> GLIBC_2.18 or earlier versions must be exactly the same as exported at
> those versions in 2.18).
>
> * Suppose you build glibc 2.19 with GCC 4.9.  This needs to work and must
> again produce a binary compatible with the previous ones.
>
> Note: there is no current glibc support for architectures that gained
> TLS-based stack guards after 2.4 (meaning they need both a TLS-based
> scheme and backwards compatibility for binaries using __stack_chk_guard),
> and your glibc patch doesn't seem to add any.  It looks to me like your
> glibc patch would have removed the __stack_chk_guard symbol and so failed
> ABI tests (this symbol being defined only if THREAD_SET_STACK_GUARD isn't
> defined) - you didn't make clear if your patch was tested with the glibc
> testsuite including passing the ABI tests.  The normal presumption is that
> it's not acceptable to remove exported symbols from glibc as some
> application might be using them.
>
> You need to ensure that, when new glibc is built, whatever compiler it's
> built with, it continues to export the __stack_chk_guard symbol at version
> GLIBC_2.17.  Furthermore, if any released GCC version would generate
> references to __stack_chk_guard when compiling code for AArch64 with stack
> protection, you need to ensure __stack_chk_guard is a normal symbol not a
> compat symbol so that people can continue to link against new glibc when
> using old GCC.  If it's only a limited range of development versions of
> GCC that could have generated code using __stack_chk_guard because
> released versions didn't support stack protection on AArch64 at all, a
> compat symbol would probably be OK (but you should still ensure that the
> variable gets initialized with the correct value for any applications
> built with those development versions of GCC).
>
> --
> Joseph S. Myers
> joseph@codesourcery.com