diff mbox series

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

Message ID 171c576f3a8d9de36996721b8bab2884499c648b.1594209990.git.szabolcs.nagy@arm.com
State Committed
Commit de9301c02e898fb20a609b459d81afda42f39c61
Headers
Series aarch64: branch protection support |

Commit Message

Szabolcs Nagy July 8, 2020, 12:13 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.

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(+)

Comments

Andreas Schwab Sept. 20, 2021, 8:58 a.m. UTC | #1 
On Jul 08 2020, 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.

That breaks with binutils 2.37.

gcc   -nostdlib -nostartfiles -r -o /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os '-Wl,-(' /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/dl-allobjs.os /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/rtld-libc.a -lgcc '-Wl,-)' \
          -Wl,-Map,/home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os.map
/usr/lib64/gcc/aarch64-suse-linux/11/../../../../aarch64-suse-linux/bin/ld: /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.109.ga93d9e03a3/cc-base/elf/librtld.os: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.

Andreas.
Andreas Schwab Sept. 20, 2021, 9:43 a.m. UTC | #2 
Actually, it is commit a93d9e03a3 that broke this.

Andreas.
Szabolcs Nagy Sept. 20, 2021, 10:49 a.m. UTC | #3 
The 09/20/2021 11:43, Andreas Schwab wrote:
> Actually, it is commit a93d9e03a3 that broke this.

thanks for catching this.

both arm and aarch64 requires asm files to include
sysdep.h to get the right build attributes / object
file markings.

i think the most portable way would be to write that
code in c (possibly with a toplevel asm block) so
the compiler can emit the necessary directives based
on cflags, but including sysdep.h should work too.
Florian Weimer Sept. 20, 2021, 10:58 a.m. UTC | #4 
* Szabolcs Nagy via Libc-alpha:

> i think the most portable way would be to write that
> code in c (possibly with a toplevel asm block) so
> the compiler can emit the necessary directives based
> on cflags, but including sysdep.h should work too.

We struggled to make this work with a C source.  The similar construct
in sysdeps/unix/sysv/linux/errlist-compat.h is a bit brittle.

Thanks,
Florian
Florian Weimer Sept. 20, 2021, 11:06 a.m. UTC | #5 
* Szabolcs Nagy via Libc-alpha:

> The 09/20/2021 11:43, Andreas Schwab wrote:
>> Actually, it is commit a93d9e03a3 that broke this.
>
> thanks for catching this.
>
> both arm and aarch64 requires asm files to include
> sysdep.h to get the right build attributes / object
> file markings.

One more thing: I looked at this when reviewing the patch, and saw
-Wl,-z,force-bti, so I thought this was going to be okay.  I did not
expect that -Wl,-z,force-bti,--fatal-warnings essentially means “BTI
needs to enable to through objectg markers” (i.e., it's not exactly
*forcing* BTI to be on).

Thanks,
Florian
Andreas Schwab Sept. 20, 2021, 11:34 a.m. UTC | #6 
I think it is wrong for the linker to complain about pure-data objects.

Andreas.
Florian Weimer Sept. 20, 2021, 11:40 a.m. UTC | #7 
* Andreas Schwab:

> I think it is wrong for the linker to complain about pure-data objects.

Is it still a pure-data object if it has a .text section, although it is
empty?  Is there an easy way to stop GAS from creating the section?

Thanks,
Florian
Andreas Schwab Sept. 20, 2021, 11:55 a.m. UTC | #8 
On Sep 20 2021, Florian Weimer wrote:

> Is it still a pure-data object if it has a .text section, although it is
> empty?

Sure, there is no executable code.

Andreas.
Szabolcs Nagy Sept. 20, 2021, 12:41 p.m. UTC | #9 
The 09/20/2021 13:55, Andreas Schwab wrote:
> On Sep 20 2021, Florian Weimer wrote:
> 
> > Is it still a pure-data object if it has a .text section, although it is
> > empty?
> 
> Sure, there is no executable code.

maybe bti could have done it that way, but gnu
properties are handled by linkers in a generic way
without understanding their meaning and a marking
may apply to data sections.

so with the current logic we even have to mark empty
objects. i think requiring sysdep.h in asm files is
not too bad.

it is not ideal that -z force-bti complains about
unmarked input objects, but in glibc that's what i'd
want: ensure all objects are marked.
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