diff mbox series

[v6,09/14] aarch64: ensure objects are BTI compatible

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

Commit Message

Szabolcs Nagy July 1, 2020, 2:39 p.m. UTC 
When glibc is built with branch protection (i.e. with a gcc configured
with --enable-standard-branch-protection), all glibc binaries should
be BTI compatible and marked as such.

It is easy to link BTI incompatible objects by accident and this is
silent currently which is usually not the expectation, so this is
changed into a link error. (There is no linker flag for failing on
BTI incompatible inputs so all warnings are turned into fatal errors
outside the test system when building glibc with branch protection.)

Unfortunately, outlined atomic functions are not BTI compatible in
libgcc (PR libgcc/96001), so to build glibc with current gcc use
'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
and then glibc can be built and tested without such workarounds.
---
 sysdeps/aarch64/Makefile     | 8 ++++++++
 sysdeps/aarch64/configure    | 2 ++
 sysdeps/aarch64/configure.ac | 1 +
 3 files changed, 11 insertions(+)

Comments

Adhemerval Zanella July 6, 2020, 5:37 p.m. UTC | #1 
On 01/07/2020 11:39, Szabolcs Nagy wrote:
> When glibc is built with branch protection (i.e. with a gcc configured
> with --enable-standard-branch-protection), all glibc binaries should
> be BTI compatible and marked as such.
> 
> It is easy to link BTI incompatible objects by accident and this is
> silent currently which is usually not the expectation, so this is
> changed into a link error. (There is no linker flag for failing on
> BTI incompatible inputs so all warnings are turned into fatal errors
> outside the test system when building glibc with branch protection.)
> 
> Unfortunately, outlined atomic functions are not BTI compatible in
> libgcc (PR libgcc/96001), so to build glibc with current gcc use
> 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
> and then glibc can be built and tested without such workarounds.

Is the outlined atomic present in any current released gcc? If so
should we add a configure handler to add -mno-outline-atomics for
such cases?

Besides that LGTM, thanks.

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

> ---
>  sysdeps/aarch64/Makefile     | 8 ++++++++
>  sysdeps/aarch64/configure    | 2 ++
>  sysdeps/aarch64/configure.ac | 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index 5ae8b082b0..7f82fc055e 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -1,5 +1,13 @@
>  long-double-fcts = yes
>  
> +ifeq (yes,$(aarch64-bti))
> +# Mark linker output BTI compatible, it warns on non-BTI inputs.
> +sysdep-LDFLAGS += -Wl,-z,force-bti
> +# Make warnings fatal outside the test system.
> +LDFLAGS-lib.so += -Wl,--fatal-warnings
> +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings
> +endif
> +
>  ifeq ($(subdir),elf)
>  sysdep-dl-routines += dl-bti
>  endif

Ok.

> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 70477a7fa5..c637540436 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -210,6 +210,8 @@ EOF
>  fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
>  $as_echo "$libc_cv_aarch64_bti" >&6; }
> +config_vars="$config_vars
> +aarch64-bti = $libc_cv_aarch64_bti"
>  if test $libc_cv_aarch64_bti = yes; then
>    $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
>  
> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 798f494740..2c2817514d 100644
> --- a/sysdeps/aarch64/configure.ac
> +++ b/sysdeps/aarch64/configure.ac
> @@ -36,6 +36,7 @@ EOF
>      libc_cv_aarch64_bti=yes
>    fi
>    rm -rf conftest.*])
> +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
>  if test $libc_cv_aarch64_bti = yes; then
>    AC_DEFINE(HAVE_AARCH64_BTI)
>  fi
> 

Ok.
Szabolcs Nagy July 6, 2020, 6:01 p.m. UTC | #2 
The 07/06/2020 14:37, Adhemerval Zanella via Libc-alpha wrote:
> On 01/07/2020 11:39, Szabolcs Nagy wrote:
> > When glibc is built with branch protection (i.e. with a gcc configured
> > with --enable-standard-branch-protection), all glibc binaries should
> > be BTI compatible and marked as such.
> > 
> > It is easy to link BTI incompatible objects by accident and this is
> > silent currently which is usually not the expectation, so this is
> > changed into a link error. (There is no linker flag for failing on
> > BTI incompatible inputs so all warnings are turned into fatal errors
> > outside the test system when building glibc with branch protection.)
> > 
> > Unfortunately, outlined atomic functions are not BTI compatible in
> > libgcc (PR libgcc/96001), so to build glibc with current gcc use
> > 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
> > and then glibc can be built and tested without such workarounds.
> 
> Is the outlined atomic present in any current released gcc? If so
> should we add a configure handler to add -mno-outline-atomics for
> such cases?

outlined atomics is supported for a while now (since
gcc-9 i think) but it is now the default in gcc-10.

i am testing patches to fix libgcc in gcc trunk and
then backport to gcc-10 branch.

my preference is to only support fixed gcc and use
the -mno-outline-atomics as a temporary workaround
so we can test the patches with gcc-10.1.0

it's not a good workaround for using the resulting
toolchain for building user code with branch protection
(which will have the same problem: silently dropping
bti protection) so i think it's better to fail the build.

> 
> Besides that LGTM, thanks.
> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 
> > ---
> >  sysdeps/aarch64/Makefile     | 8 ++++++++
> >  sysdeps/aarch64/configure    | 2 ++
> >  sysdeps/aarch64/configure.ac | 1 +
> >  3 files changed, 11 insertions(+)
> > 
> > diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> > index 5ae8b082b0..7f82fc055e 100644
> > --- a/sysdeps/aarch64/Makefile
> > +++ b/sysdeps/aarch64/Makefile
> > @@ -1,5 +1,13 @@
> >  long-double-fcts = yes
> >  
> > +ifeq (yes,$(aarch64-bti))
> > +# Mark linker output BTI compatible, it warns on non-BTI inputs.
> > +sysdep-LDFLAGS += -Wl,-z,force-bti
> > +# Make warnings fatal outside the test system.
> > +LDFLAGS-lib.so += -Wl,--fatal-warnings
> > +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings
> > +endif
> > +
> >  ifeq ($(subdir),elf)
> >  sysdep-dl-routines += dl-bti
> >  endif
> 
> Ok.
> 
> > diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> > index 70477a7fa5..c637540436 100644
> > --- a/sysdeps/aarch64/configure
> > +++ b/sysdeps/aarch64/configure
> > @@ -210,6 +210,8 @@ EOF
> >  fi
> >  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
> >  $as_echo "$libc_cv_aarch64_bti" >&6; }
> > +config_vars="$config_vars
> > +aarch64-bti = $libc_cv_aarch64_bti"
> >  if test $libc_cv_aarch64_bti = yes; then
> >    $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
> >  
> > diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> > index 798f494740..2c2817514d 100644
> > --- a/sysdeps/aarch64/configure.ac
> > +++ b/sysdeps/aarch64/configure.ac
> > @@ -36,6 +36,7 @@ EOF
> >      libc_cv_aarch64_bti=yes
> >    fi
> >    rm -rf conftest.*])
> > +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
> >  if test $libc_cv_aarch64_bti = yes; then
> >    AC_DEFINE(HAVE_AARCH64_BTI)
> >  fi
> > 
> 
> Ok.

--
Adhemerval Zanella July 6, 2020, 6:17 p.m. UTC | #3 
On 06/07/2020 15:01, Szabolcs Nagy wrote:
> The 07/06/2020 14:37, Adhemerval Zanella via Libc-alpha wrote:
>> On 01/07/2020 11:39, Szabolcs Nagy wrote:
>>> When glibc is built with branch protection (i.e. with a gcc configured
>>> with --enable-standard-branch-protection), all glibc binaries should
>>> be BTI compatible and marked as such.
>>>
>>> It is easy to link BTI incompatible objects by accident and this is
>>> silent currently which is usually not the expectation, so this is
>>> changed into a link error. (There is no linker flag for failing on
>>> BTI incompatible inputs so all warnings are turned into fatal errors
>>> outside the test system when building glibc with branch protection.)
>>>
>>> Unfortunately, outlined atomic functions are not BTI compatible in
>>> libgcc (PR libgcc/96001), so to build glibc with current gcc use
>>> 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon
>>> and then glibc can be built and tested without such workarounds.
>>
>> Is the outlined atomic present in any current released gcc? If so
>> should we add a configure handler to add -mno-outline-atomics for
>> such cases?
> 
> outlined atomics is supported for a while now (since
> gcc-9 i think) but it is now the default in gcc-10.
> 
> i am testing patches to fix libgcc in gcc trunk and
> then backport to gcc-10 branch.
> 
> my preference is to only support fixed gcc and use
> the -mno-outline-atomics as a temporary workaround
> so we can test the patches with gcc-10.1.0
> 
> it's not a good workaround for using the resulting
> toolchain for building user code with branch protection
> (which will have the same problem: silently dropping
> bti protection) so i think it's better to fail the build.

Right, I think it should be ok to sign it with a build failure.
diff mbox series

Patch

diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 5ae8b082b0..7f82fc055e 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -1,5 +1,13 @@ 
 long-double-fcts = yes
 
+ifeq (yes,$(aarch64-bti))
+# Mark linker output BTI compatible, it warns on non-BTI inputs.
+sysdep-LDFLAGS += -Wl,-z,force-bti
+# Make warnings fatal outside the test system.
+LDFLAGS-lib.so += -Wl,--fatal-warnings
+LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings
+endif
+
 ifeq ($(subdir),elf)
 sysdep-dl-routines += dl-bti
 endif
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 70477a7fa5..c637540436 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -210,6 +210,8 @@  EOF
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5
 $as_echo "$libc_cv_aarch64_bti" >&6; }
+config_vars="$config_vars
+aarch64-bti = $libc_cv_aarch64_bti"
 if test $libc_cv_aarch64_bti = yes; then
   $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h
 
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 798f494740..2c2817514d 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -36,6 +36,7 @@  EOF
     libc_cv_aarch64_bti=yes
   fi
   rm -rf conftest.*])
+LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti])
 if test $libc_cv_aarch64_bti = yes; then
   AC_DEFINE(HAVE_AARCH64_BTI)
 fi