Consistently include BTI markers in every assembly file

Trying to migrate Chromium to the "link all the asm files together" strategy broke the aarch64 Android build because some of the ifdef'd out assembly files were missing the .note.gnu.property section for BTI. If we add support for IBT, that'll be another one. To fix this, introduce <openssl/asm_base.h>, which must be included at the start of every assembly file (before the target ifdefs). This does a couple things: - It emits BTI and noexecstack markers into every assembly file, even those that ifdef themselves out. - It resolves the MSan -> OPENSSL_NO_ASM logic, so we only need to do it once. - It defines the same OPENSSL_X86_64, etc., defines we set elsewhere, so we can ensure they're consistent. This required carving files up a bit. <openssl/base.h> has a lot of things, such that trying to guard everything in it on __ASSEMBLER__ would be tedious. Instead, I moved the target defines to a new <openssl/target.h>. Then <openssl/asm_base.h> is the new header that pulls in all those things. Bug: 542 Change-Id: I1682b4d929adea72908655fa1bb15765a6b3473b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60765 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
briansmith · Jun 22, 2023 · a905bbb · a905bbb
1 parent e79649b
commit a905bbb
Show file tree

Hide file tree

Showing 13 changed files with 373 additions and 346 deletions.
diff --git a/crypto/curve25519/asm/x25519-asm-arm.S b/crypto/curve25519/asm/x25519-asm-arm.S
@@ -17,17 +17,9 @@
  * domain licensed but the standard ISC license is included above to keep
  * licensing simple. */
 
-#if defined(__has_feature)
-#if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM)
-#define OPENSSL_NO_ASM
-#endif
-#endif
+#include <openssl/asm_base.h>
 
-#if !defined(OPENSSL_NO_ASM) && defined(__ARMEL__) && defined(__ELF__)
-
-#if defined(BORINGSSL_PREFIX)
-#include <boringssl_prefix_symbols_asm.h>
-#endif
+#if !defined(OPENSSL_NO_ASM) && defined(OPENSSL_ARM) && defined(__ELF__)
 
 .fpu neon
 .text
@@ -2129,8 +2121,4 @@ mov sp,r12
 vpop {q4,q5,q6,q7}
 bx lr
 
-#endif  /* !OPENSSL_NO_ASM && __ARMEL__ && __ELF__ */
-
-#if defined(__ELF__)
-.section	.note.GNU-stack,"",%progbits
-#endif
+#endif  /* !OPENSSL_NO_ASM && OPENSSL_ARM && __ELF__ */
diff --git a/crypto/fipsmodule/CMakeLists.txt b/crypto/fipsmodule/CMakeLists.txt
@@ -85,12 +85,16 @@ if(FIPS_DELOCATE)
     -cc ${CMAKE_ASM_COMPILER}
     -cc-flags "${TARGET} $CMAKE_ASM_FLAGS"
     ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h
+    ${PROJECT_SOURCE_DIR}/include/openssl/asm_base.h
+    ${PROJECT_SOURCE_DIR}/include/openssl/target.h
     ${BCM_SOURCES_ASM_USED}
     DEPENDS
     bcm_c_generated_asm
     delocate
     ${BCM_SOURCES_ASM_USED}
     ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h
+    ${PROJECT_SOURCE_DIR}/include/openssl/asm_base.h
+    ${PROJECT_SOURCE_DIR}/include/openssl/target.h
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   )
 

diff --git a/crypto/hrss/asm/poly_rq_mul.S b/crypto/hrss/asm/poly_rq_mul.S
@@ -12,11 +12,9 @@
 // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
 // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-#if !defined(OPENSSL_NO_ASM) && !defined(OPENSSL_SMALL) && defined(__linux__) && defined(__x86_64__)
+#include <openssl/asm_base.h>
 
-#if defined(BORINGSSL_PREFIX)
-#include <boringssl_prefix_symbols_asm.h>
-#endif
+#if !defined(OPENSSL_NO_ASM) && !defined(OPENSSL_SMALL) && defined(OPENSSL_LINUX) && defined(OPENSSL_X86_64)
 
 // This is the polynomial multiplication function from [HRSS], provided by kind
 // permission of the authors.
@@ -8487,7 +8485,3 @@ ret
 .size poly_Rq_mul,.-poly_Rq_mul
 
 #endif
-
-#if defined(__ELF__)
-.section	.note.GNU-stack,"",%progbits
-#endif
diff --git a/crypto/perlasm/arm-xlate.pl b/crypto/perlasm/arm-xlate.pl
@@ -153,9 +153,9 @@ sub expand_line {
 
 my ($arch_defines, $target_defines);
 if ($flavour =~ /32/) {
-    $arch_defines = "defined(__ARMEL__)";
+    $arch_defines = "defined(OPENSSL_ARM)";
 } elsif ($flavour =~ /64/) {
-    $arch_defines = "defined(__AARCH64EL__)";
+    $arch_defines = "defined(OPENSSL_AARCH64)";
 } else {
     die "unknown architecture: $flavour";
 }
@@ -177,20 +177,11 @@ sub expand_line {
 // This file is generated from a similarly-named Perl script in the BoringSSL
 // source tree. Do not edit by hand.
 
-#if !defined(__has_feature)
-#define __has_feature(x) 0
-#endif
-#if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM)
-#define OPENSSL_NO_ASM
-#endif
+#include <openssl/asm_base.h>
 
 #if !defined(OPENSSL_NO_ASM) && $arch_defines && $target_defines
 ___
 
-print "#if defined(BORINGSSL_PREFIX)\n";
-print "#include <boringssl_prefix_symbols_asm.h>\n";
-print "#endif\n";
-
 while(my $line=<>) {
 
     if ($line =~ m/^\s*(#|@|\/\/)/)	{ print $line; next; }
@@ -260,10 +251,6 @@ sub expand_line {
 
 print <<___;
 #endif  // !OPENSSL_NO_ASM && $arch_defines && $target_defines
-#if defined(__ELF__)
-// See https://www.airs.com/blog/archives/518.
-.section .note.GNU-stack,"",\%progbits
-#endif
 ___
 
 close STDOUT or die "error closing STDOUT: $!";
diff --git a/crypto/perlasm/x86_64-xlate.pl b/crypto/perlasm/x86_64-xlate.pl
@@ -1522,16 +1522,9 @@ sub rxb {
         die "unknown target: $flavour";
     }
     print <<___;
-#if defined(__has_feature)
-#if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM)
-#define OPENSSL_NO_ASM
-#endif
-#endif
-
-#if defined(__x86_64__) && !defined(OPENSSL_NO_ASM) && $target
-#if defined(BORINGSSL_PREFIX)
-#include <boringssl_prefix_symbols_asm.h>
-#endif
+#include <openssl/asm_base.h>
+
+#if !defined(OPENSSL_NO_ASM) && defined(OPENSSL_X86_64) && $target
 ___
 }
 
@@ -1627,13 +1620,7 @@ sub process_line {
 if ($masm) {
     print "END\n";
 } elsif ($gas) {
-    print <<___;
-#endif
-#if defined(__ELF__)
-// See https://www.airs.com/blog/archives/518.
-.section .note.GNU-stack,"",\%progbits
-#endif
-___
+    print "#endif\n";
 } elsif ($nasm) {
     print <<___;
 \%else

diff --git a/crypto/perlasm/x86asm.pl b/crypto/perlasm/x86asm.pl
@@ -307,24 +307,13 @@ sub ::asm_finish
         }
 
         print <<___;
-#if defined(__has_feature)
-#if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM)
-#define OPENSSL_NO_ASM
-#endif
-#endif
-
-#if !defined(OPENSSL_NO_ASM) && defined(__i386__) && $target
-#if defined(BORINGSSL_PREFIX)
-#include <boringssl_prefix_symbols_asm.h>
-#endif
+#include <openssl/asm_base.h>
+
+#if !defined(OPENSSL_NO_ASM) && defined(OPENSSL_X86) && $target
 ___
         print @out;
         print <<___;
-#endif  // !defined(OPENSSL_NO_ASM) && defined(__i386__) && $target
-#if defined(__ELF__)
-// See https://www.airs.com/blog/archives/518.
-.section .note.GNU-stack,"",\%progbits
-#endif
+#endif  // !defined(OPENSSL_NO_ASM) && defined(OPENSSL_X86) && $target
 ___
     }
 }

diff --git a/crypto/poly1305/poly1305_arm_asm.S b/crypto/poly1305/poly1305_arm_asm.S
@@ -1,14 +1,6 @@
-#if defined(__has_feature)
-#if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM)
-#define OPENSSL_NO_ASM
-#endif
-#endif
+#include <openssl/asm_base.h>
 
-#if defined(__ARMEL__) && !defined(OPENSSL_NO_ASM) && defined(__ELF__)
-
-#if defined(BORINGSSL_PREFIX)
-#include <boringssl_prefix_symbols_asm.h>
-#endif
+#if !defined(OPENSSL_NO_ASM) && defined(OPENSSL_ARM) && defined(__ELF__)
 
 # This implementation was taken from the public domain, neon2 version in
 # SUPERCOP by D. J. Bernstein and Peter Schwabe.
@@ -2022,8 +2014,4 @@ vst1.8 d4,[r0,: 64]
 add sp,sp,#0
 bx lr
 
-#endif  /* __ARMEL__ && !OPENSSL_NO_ASM && __ELF__ */
-
-#if defined(__ELF__)
-.section	.note.GNU-stack,"",%progbits
-#endif
+#endif  /* !OPENSSL_NO_ASM && OPENSSL_ARM && __ELF__ */
diff --git a/include/openssl/arm_arch.h b/include/openssl/arm_arch.h
@@ -53,12 +53,13 @@
 #ifndef OPENSSL_HEADER_ARM_ARCH_H
 #define OPENSSL_HEADER_ARM_ARCH_H
 
+#include <openssl/target.h>
+
 // arm_arch.h contains symbols used by ARM assembly, and the C code that calls
 // it. It is included as a public header to simplify the build, but is not
 // intended for external use.
 
-#if defined(__ARMEL__) || defined(_M_ARM) || defined(__AARCH64EL__) || \
-    defined(_M_ARM64)
+#if defined(OPENSSL_ARM) || defined(OPENSSL_AARCH64)
 
 // ARMV7_NEON is true when a NEON unit is present in the current CPU.
 #define ARMV7_NEON (1 << 0)
@@ -97,124 +98,8 @@
 // will be included.
 #define __ARM_MAX_ARCH__ 8
 
-// Support macros for
-//   - Armv8.3-A Pointer Authentication and
-//   - Armv8.5-A Branch Target Identification
-// features which require emitting a .note.gnu.property section with the
-// appropriate architecture-dependent feature bits set.
-//
-// |AARCH64_SIGN_LINK_REGISTER| and |AARCH64_VALIDATE_LINK_REGISTER| expand to
-// PACIxSP and AUTIxSP, respectively. |AARCH64_SIGN_LINK_REGISTER| should be
-// used immediately before saving the LR register (x30) to the stack.
-// |AARCH64_VALIDATE_LINK_REGISTER| should be used immediately after restoring
-// it. Note |AARCH64_SIGN_LINK_REGISTER|'s modifications to LR must be undone
-// with |AARCH64_VALIDATE_LINK_REGISTER| before RET. The SP register must also
-// have the same value at the two points. For example:
-//
-//   .global f
-//   f:
-//     AARCH64_SIGN_LINK_REGISTER
-//     stp x29, x30, [sp, #-96]!
-//     mov x29, sp
-//     ...
-//     ldp x29, x30, [sp], #96
-//     AARCH64_VALIDATE_LINK_REGISTER
-//     ret
-//
-// |AARCH64_VALID_CALL_TARGET| expands to BTI 'c'. Either it, or
-// |AARCH64_SIGN_LINK_REGISTER|, must be used at every point that may be an
-// indirect call target. In particular, all symbols exported from a file must
-// begin with one of these macros. For example, a leaf function that does not
-// save LR can instead use |AARCH64_VALID_CALL_TARGET|:
-//
-//   .globl return_zero
-//   return_zero:
-//     AARCH64_VALID_CALL_TARGET
-//     mov x0, #0
-//     ret
-//
-// A non-leaf function which does not immediately save LR may need both macros
-// because |AARCH64_SIGN_LINK_REGISTER| appears late. For example, the function
-// may jump to an alternate implementation before setting up the stack:
-//
-//   .globl with_early_jump
-//   with_early_jump:
-//     AARCH64_VALID_CALL_TARGET
-//     cmp x0, #128
-//     b.lt .Lwith_early_jump_128
-//     AARCH64_SIGN_LINK_REGISTER
-//     stp x29, x30, [sp, #-96]!
-//     mov x29, sp
-//     ...
-//     ldp x29, x30, [sp], #96
-//     AARCH64_VALIDATE_LINK_REGISTER
-//     ret
-//
-//  .Lwith_early_jump_128:
-//     ...
-//     ret
-//
-// These annotations are only required with indirect calls. Private symbols that
-// are only the target of direct calls do not require annotations. Also note
-// that |AARCH64_VALID_CALL_TARGET| is only valid for indirect calls (BLR), not
-// indirect jumps (BR). Indirect jumps in assembly are currently not supported
-// and would require a macro for BTI 'j'.
-//
-// Although not necessary, it is safe to use these macros in 32-bit ARM
-// assembly. This may be used to simplify dual 32-bit and 64-bit files.
-//
-// References:
-// - "ELF for the Arm® 64-bit Architecture"
-//   https://github.com/ARM-software/abi-aa/blob/master/aaelf64/aaelf64.rst
-// - "Providing protection for complex software"
-//   https://developer.arm.com/architectures/learn-the-architecture/providing-protection-for-complex-software
-
-#if defined(__ARM_FEATURE_BTI_DEFAULT) && __ARM_FEATURE_BTI_DEFAULT == 1
-#define GNU_PROPERTY_AARCH64_BTI (1 << 0)   // Has Branch Target Identification
-#define AARCH64_VALID_CALL_TARGET hint #34  // BTI 'c'
-#else
-#define GNU_PROPERTY_AARCH64_BTI 0  // No Branch Target Identification
-#define AARCH64_VALID_CALL_TARGET
-#endif
-
-#if defined(__ARM_FEATURE_PAC_DEFAULT) && \
-    (__ARM_FEATURE_PAC_DEFAULT & 1) == 1  // Signed with A-key
-#define GNU_PROPERTY_AARCH64_POINTER_AUTH \
-  (1 << 1)                                       // Has Pointer Authentication
-#define AARCH64_SIGN_LINK_REGISTER hint #25      // PACIASP
-#define AARCH64_VALIDATE_LINK_REGISTER hint #29  // AUTIASP
-#elif defined(__ARM_FEATURE_PAC_DEFAULT) && \
-    (__ARM_FEATURE_PAC_DEFAULT & 2) == 2  // Signed with B-key
-#define GNU_PROPERTY_AARCH64_POINTER_AUTH \
-  (1 << 1)                                       // Has Pointer Authentication
-#define AARCH64_SIGN_LINK_REGISTER hint #27      // PACIBSP
-#define AARCH64_VALIDATE_LINK_REGISTER hint #31  // AUTIBSP
-#else
-#define GNU_PROPERTY_AARCH64_POINTER_AUTH 0  // No Pointer Authentication
-#if GNU_PROPERTY_AARCH64_BTI != 0
-#define AARCH64_SIGN_LINK_REGISTER AARCH64_VALID_CALL_TARGET
-#else
-#define AARCH64_SIGN_LINK_REGISTER
-#endif
-#define AARCH64_VALIDATE_LINK_REGISTER
-#endif
-
-#if GNU_PROPERTY_AARCH64_POINTER_AUTH != 0 || GNU_PROPERTY_AARCH64_BTI != 0
-.pushsection .note.gnu.property, "a";
-.balign 8;
-.long 4;
-.long 0x10;
-.long 0x5;
-.asciz "GNU";
-.long 0xc0000000; /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */
-.long 4;
-.long (GNU_PROPERTY_AARCH64_POINTER_AUTH | GNU_PROPERTY_AARCH64_BTI);
-.long 0;
-.popsection;
-#endif
-
 #endif  // __ASSEMBLER__
 
-#endif  // __ARMEL__ || _M_ARM || __AARCH64EL__ || _M_ARM64
+#endif  // ARM || AARCH64
 
 #endif  // OPENSSL_HEADER_ARM_ARCH_H