Make sure hardening configuration flags are correctly applied.

Previously, upstream flags were overriding our configuration flags,
therefore we had two workarounds in place. This commit replaces these
workarounds by upstream solution. While it should not result in any
functional changes, it is be better to have this patch in place to make
easier to spot when it is not needed anymore.
This commit is contained in:
Vít Ondruch 2024-06-18 12:28:13 +02:00
parent 42b0e43e5a
commit 7724c2d703
3 changed files with 309 additions and 27 deletions

View File

@ -0,0 +1,302 @@
From 3d405634f43d39079ee93cdc59ed7fc0a5e8917a Mon Sep 17 00:00:00 2001
From: KJ Tsanaktsidis <kj@kjtsanaktsidis.id.au>
Date: Sun, 9 Jun 2024 21:15:39 +1000
Subject: [PATCH] Extract hardening CFLAGS to a special $hardenflags variable
This changes the automatic detection of -fstack-protector,
-D_FORTIFY_SOURCE, and -mbranch-protection to write to $hardenflags
instead of $XCFLAGS. The definition of $cflags is changed to
"$hardenflags $orig_cflags $optflags $debugflags $warnflags" to match.
Furthermore, these flags are _prepended_ to $hardenflags, rather than
appended.
The implications of doing this are as follows:
* If a CRuby builder specifies cflags="-mbranch-protection=foobar" at
the ./configure script, and the configure script detects that
-mbranch-protection=pac-ret is accepted, then GCC will be invoked as
"gcc -mbranch-protection=pac-ret -mbranch-protection=foobar". Since
the last flags take precedence, that means that user-supplied values
of these flags in $cflags will take priority.
* Likewise, if a CRuby builder explicitly specifies
"hardenflags=-mbranch-protection=foobar", because we _prepend_ to
$hardenflags in our autoconf script, we will still invoke GCC as
"gcc -mbranch-protection=pac-ret -mbranch-protection=foobar".
* If a CRuby builder specifies CFLAGS="..." at the configure line,
automatic detection of hardening flags is ignored as before.
* C extensions will _also_ be built with hardening flags now as well
(this was not the case by default before because the detected flags
went into $XCFLAGS).
Additionally, as part of this work, I changed how the detection of
PAC/BTI in Context.S works. Rather than appending the autodetected
option to ASFLAGS, we simply compile a set of test programs with the
actual CFLAGS in use to determine what PAC/BTI settings were actually
chosen by the builder. Context.S is made aware of these choices through
some custom macros.
The result of this work is that:
* Ruby will continue to choose some sensible defaults for hardening
options for the C compiler
* Distributors are able to specify CFLAGS that are consistent with their
distribution and override these defaults
* Context.S will react to whatever -mbranch-protection is actually in
use, not what was autodetected
* Extensions get built with hardening flags too.
[Bug #20154]
[Bug #20520]
---
configure.ac | 81 ++++++++++++++++++++++++++++++-----
coroutine/arm64/Context.S | 14 +++---
template/Makefile.in | 1 +
tool/m4/ruby_append_option.m4 | 4 ++
tool/m4/ruby_try_cflags.m4 | 17 ++++++++
5 files changed, 100 insertions(+), 17 deletions(-)
diff --git a/configure.ac b/configure.ac
index f35fad6a362611..0da15772d36671 100644
--- a/configure.ac
+++ b/configure.ac
@@ -352,7 +352,7 @@ test -z "$warnflags" ||
AS_IF([test -z "${CFLAGS+set}"], [
cflags=`echo " $cflags " | sed "$cflagspat;s/^ *//;s/ *$//"`
orig_cflags="$cflags"
- cflags="$cflags "'${optflags} ${debugflags} ${warnflags}'
+ cflags='${hardenflags} '"$cflags "'${optflags} ${debugflags} ${warnflags}'
])
dnl AS_IF([test -z "${CXXFLAGS+set}"], [
dnl cxxflags=`echo " $cxxflags " | sed "$cflagspat;s/^ *//;s/ *$//"`
@@ -800,7 +800,7 @@ AS_IF([test "$GCC" = yes], [
[fortify_source=$enableval])
AS_IF([test "x$fortify_source" != xno], [
RUBY_TRY_CFLAGS([$optflags -D_FORTIFY_SOURCE=2],
- [RUBY_APPEND_OPTION(XCFLAGS, -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2)], [],
+ [RUBY_PREPEND_OPTION(hardenflags, -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2)], [],
[@%:@include <stdio.h>])
])
@@ -821,20 +821,24 @@ AS_IF([test "$GCC" = yes], [
AC_MSG_CHECKING([for -fstack-protector])
AC_MSG_RESULT(["$stack_protector"])
AS_CASE(["$stack_protector"], [-*], [
- RUBY_APPEND_OPTION(XCFLAGS, $stack_protector)
- RUBY_APPEND_OPTION(XLDFLAGS, $stack_protector)
- RUBY_APPEND_OPTION(LDFLAGS, $stack_protector)
+ RUBY_PREPEND_OPTION(hardenflags, $stack_protector)
+ RUBY_APPEND_OPTION(XLDFLAGS, $stack_protector)
+ RUBY_APPEND_OPTION(LDFLAGS, $stack_protector)
])
# aarch64 branch protection
AS_CASE(["$target_cpu"], [aarch64], [
AS_FOR(option, opt, [-mbranch-protection=pac-ret -msign-return-address=all], [
- RUBY_TRY_CFLAGS(option, [branch_protection=yes], [branch_protection=no])
+ # Try these flags in the _prepended_ position - i.e. we want to try building a program
+ # with CFLAGS="-mbranch-protection=pac-ret $CFLAGS". If the builder has provided different
+ # branch protection flags in CFLAGS, we don't want to overwrite those. We just want to
+ # find some branch protection flags which work if none were provided.
+ RUBY_TRY_CFLAGS_PREPEND(option, [branch_protection=yes], [branch_protection=no])
AS_IF([test "x$branch_protection" = xyes], [
- # C compiler and assembler must be consistent for -mbranch-protection
- # since they both check `__ARM_FEATURE_PAC_DEFAULT` definition.
- RUBY_APPEND_OPTION(XCFLAGS, option)
- RUBY_APPEND_OPTION(ASFLAGS, option)
+ # _prepend_ the options to CFLAGS, so that user-provided flags will overwrite them.
+ # These CFLAGS are used during the configure script to compile further test programs;
+ # however, $harden_flags is prepended separately to CFLAGS at the end of the script.
+ RUBY_PREPEND_OPTION(hardenflags, $opt)
break
])
])
@@ -983,6 +987,59 @@ AC_SUBST(incflags, "$INCFLAGS")
test -z "${ac_env_CXXFLAGS_set}" -a -n "${cxxflags+set}" && eval CXXFLAGS="\"$cxxflags $ARCH_FLAG\""
}
+# The lines above expand out the $cflags/$optflags/$debugflags/$hardenflags variables into the
+# CFLAGS variable. So, at this point, we have a $CFLAGS var with the actual compiler flags we're
+# going to use.
+# That means this is the right time to check what branch protection flags are going to be in use
+# and define appropriate macros for use in Context.S based on this
+AS_CASE(["$target_cpu"], [aarch64], [
+ AC_CACHE_CHECK([whether __ARM_FEATURE_BTI_DEFAULT is defined],
+ rb_cv_aarch64_bti_enabled,
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([[
+ @%:@ifndef __ARM_FEATURE_BTI_DEFAULT
+ @%:@error "__ARM_FEATURE_BTI_DEFAULT not defined"
+ @%:@endif
+ ]])],
+ [rb_cv_aarch64_bti_enabled=yes],
+ [rb_cv_aarch64_bti_enabled=no])
+ )
+ AS_IF([test "$rb_cv_aarch64_bti_enabled" = yes],
+ AC_DEFINE(RUBY_AARCH64_BTI_ENABLED, 1))
+ AC_CACHE_CHECK([whether __ARM_FEATURE_PAC_DEFAULT is defined],
+ rb_cv_aarch64_pac_enabled,
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([[
+ @%:@ifndef __ARM_FEATURE_PAC_DEFAULT
+ @%:@error "__ARM_FEATURE_PAC_DEFAULT not defined"
+ @%:@endif
+ ]])],
+ [rb_cv_aarch64_pac_enabled=yes],
+ [rb_cv_aarch64_pac_enabled=no])
+ )
+ AS_IF([test "$rb_cv_aarch64_pac_enabled" = yes],
+ AC_DEFINE(RUBY_AARCH64_PAC_ENABLED, 1))
+ # Context.S will only ever sign its return address with the A-key; it doesn't support
+ # the B-key at the moment.
+ AS_IF([test "$rb_cv_aarch64_pac_enabled" = yes], [
+ AC_CACHE_CHECK([whether __ARM_FEATURE_PAC_DEFAULT specifies the b-key bit 0x02],
+ rb_cv_aarch64_pac_b_key,
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([[
+ @%:@ifdef __ARM_FEATURE_PAC_DEFAULT
+ @%:@if __ARM_FEATURE_PAC_DEFAULT & 0x02
+ @%:@error "__ARM_FEATURE_PAC_DEFAULT specifies B key"
+ @%:@endif
+ @%:@endif
+ ]])],
+ [rb_cv_aarch64_pac_b_key=no],
+ [rb_cv_aarch64_pac_b_key=yes])
+ )
+ AS_IF([test "$rb_cv_aarch64_pac_b_key" = yes],
+ AC_MSG_ERROR(-mbranch-protection flag specified b-key but Ruby's Context.S does not support this yet.))
+ ])
+])
+
AC_CACHE_CHECK([whether compiler has statement and declarations in expressions],
rb_cv_have_stmt_and_decl_in_expr,
[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[ __extension__ ({ int a = 0; a; }); ]])],
@@ -4211,12 +4268,13 @@ AS_IF([test "${ARCH_FLAG}"], [
rb_cv_warnflags=`echo "$rb_cv_warnflags" | sed 's/^ *//;s/ *$//'`
warnflags="$rb_cv_warnflags"
AC_SUBST(cppflags)dnl
-AC_SUBST(cflags, ["${orig_cflags:+$orig_cflags }"'${optflags} ${debugflags} ${warnflags}'])dnl
+AC_SUBST(cflags, ['${hardenflags} '"${orig_cflags:+$orig_cflags }"' ${optflags} ${debugflags} ${warnflags}'])dnl
AC_SUBST(cxxflags)dnl
AC_SUBST(optflags)dnl
AC_SUBST(debugflags)dnl
AC_SUBST(warnflags)dnl
AC_SUBST(strict_warnflags)dnl
+AC_SUBST(hardenflags)dnl
AC_SUBST(XCFLAGS)dnl
AC_SUBST(XLDFLAGS)dnl
AC_SUBST(EXTLDFLAGS)dnl
@@ -4684,6 +4742,7 @@ config_summary "DLDFLAGS" "$DLDFLAGS"
config_summary "optflags" "$optflags"
config_summary "debugflags" "$debugflags"
config_summary "warnflags" "$warnflags"
+config_summary "hardenflags" "$hardenflags"
config_summary "strip command" "$STRIP"
config_summary "install doc" "$DOCTARGETS"
config_summary "YJIT support" "$YJIT_SUPPORT"
diff --git a/coroutine/arm64/Context.S b/coroutine/arm64/Context.S
index 5251ab214df1f0..54611a247e2f66 100644
--- a/coroutine/arm64/Context.S
+++ b/coroutine/arm64/Context.S
@@ -5,6 +5,8 @@
## Copyright, 2018, by Samuel Williams.
##
+#include "ruby/config.h"
+
#define TOKEN_PASTE(x,y) x##y
#define PREFIXED_SYMBOL(prefix,name) TOKEN_PASTE(prefix,name)
@@ -27,10 +29,10 @@
.global PREFIXED_SYMBOL(SYMBOL_PREFIX,coroutine_transfer)
PREFIXED_SYMBOL(SYMBOL_PREFIX,coroutine_transfer):
-#if defined(__ARM_FEATURE_PAC_DEFAULT) && (__ARM_FEATURE_PAC_DEFAULT != 0)
+#if defined(RUBY_AARCH64_PAC_ENABLED)
# paciasp (it also acts as BTI landing pad, so no need to insert BTI also)
hint #25
-#elif defined(__ARM_FEATURE_BTI_DEFAULT) && (__ARM_FEATURE_BTI_DEFAULT != 0)
+#elif defined(RUBY_AARCH64_BTI_ENABLED)
# For the the case PAC is not enabled but BTI is.
# bti c
hint #34
@@ -73,7 +75,7 @@ PREFIXED_SYMBOL(SYMBOL_PREFIX,coroutine_transfer):
# Pop stack frame
add sp, sp, 0xa0
-#if defined(__ARM_FEATURE_PAC_DEFAULT) && (__ARM_FEATURE_PAC_DEFAULT != 0)
+#if defined(RUBY_AARCH64_PAC_ENABLED)
# autiasp: Authenticate x30 (LR) with SP and key A
hint #29
#endif
@@ -85,18 +87,18 @@ PREFIXED_SYMBOL(SYMBOL_PREFIX,coroutine_transfer):
.section .note.GNU-stack,"",%progbits
#endif
-#if __ARM_FEATURE_BTI_DEFAULT != 0 || __ARM_FEATURE_PAC_DEFAULT != 0
+#if defined(RUBY_AARCH64_BTI_ENABLED) || defined(RUBY_AARCH64_PAC_ENABLED)
/* See "ELF for the Arm 64-bit Architecture (AArch64)"
https://github.com/ARM-software/abi-aa/blob/2023Q3/aaelf64/aaelf64.rst#program-property */
# define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1<<0)
# define GNU_PROPERTY_AARCH64_FEATURE_1_PAC (1<<1)
-# if __ARM_FEATURE_BTI_DEFAULT != 0
+# if defined(RUBY_AARCH64_BTI_ENABLED)
# define BTI_FLAG GNU_PROPERTY_AARCH64_FEATURE_1_BTI
# else
# define BTI_FLAG 0
# endif
-# if __ARM_FEATURE_PAC_DEFAULT != 0
+# if defined(RUBY_AARCH64_PAC_ENABLED)
# define PAC_FLAG GNU_PROPERTY_AARCH64_FEATURE_1_PAC
# else
# define PAC_FLAG 0
diff --git a/template/Makefile.in b/template/Makefile.in
index 033ac56cb38886..abb4469777ce8a 100644
--- a/template/Makefile.in
+++ b/template/Makefile.in
@@ -89,6 +89,7 @@ cflags = @cflags@
optflags = @optflags@
debugflags = @debugflags@
warnflags = @warnflags@ @strict_warnflags@
+hardenflags = @hardenflags@
cppflags = @cppflags@
incflags = @incflags@
RUBY_DEVEL = @RUBY_DEVEL@ # "yes" or empty
diff --git a/tool/m4/ruby_append_option.m4 b/tool/m4/ruby_append_option.m4
index ff828d2162c22f..98359fa1f95f52 100644
--- a/tool/m4/ruby_append_option.m4
+++ b/tool/m4/ruby_append_option.m4
@@ -3,3 +3,7 @@ AC_DEFUN([RUBY_APPEND_OPTION],
[# RUBY_APPEND_OPTION($1)
AS_CASE([" [$]{$1-} "],
[*" $2 "*], [], [' '], [ $1="$2"], [ $1="[$]$1 $2"])])dnl
+AC_DEFUN([RUBY_PREPEND_OPTION],
+ [# RUBY_APPEND_OPTION($1)
+ AS_CASE([" [$]{$1-} "],
+ [*" $2 "*], [], [' '], [ $1="$2"], [ $1="$2 [$]$1"])])dnl
diff --git a/tool/m4/ruby_try_cflags.m4 b/tool/m4/ruby_try_cflags.m4
index b74718fe5e1cef..b397642aad9ca2 100644
--- a/tool/m4/ruby_try_cflags.m4
+++ b/tool/m4/ruby_try_cflags.m4
@@ -17,3 +17,20 @@ AC_DEFUN([RUBY_TRY_CFLAGS], [
AC_MSG_RESULT(no)])
])
])dnl
+
+AC_DEFUN([_RUBY_TRY_CFLAGS_PREPEND], [
+ RUBY_WERROR_FLAG([
+ CFLAGS="$1 [$]CFLAGS"
+ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[$4]], [[$5]])],
+ [$2], [$3])
+ ])dnl
+])dnl
+AC_DEFUN([RUBY_TRY_CFLAGS_PREPEND], [
+ AC_MSG_CHECKING([whether ]$1[ is accepted as CFLAGS])dnl
+ _RUBY_TRY_CFLAGS_PREPEND([$1],
+ [$2
+ AC_MSG_RESULT(yes)],
+ [$3
+ AC_MSG_RESULT(no)],
+ [$4], [$5])
+])dnl

View File

@ -1,24 +0,0 @@
From db4ba95bf12f9303e38a9a78979cd363cb9a19fb Mon Sep 17 00:00:00 2001
From: Jarek Prokop <jprokop@redhat.com>
Date: Fri, 12 Jan 2024 18:33:34 +0100
Subject: [PATCH] aarch64: Prepend -mbranch-protection=standard option when
checking branch protection.
Related Upstream issue: https://bugs.ruby-lang.org/issues/20154
---
configure.ac | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 18b4247991..5ea8ada8f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -828,7 +828,7 @@ AS_IF([test "$GCC" = yes], [
# aarch64 branch protection
AS_CASE(["$target_cpu"], [aarch64], [
- AS_FOR(option, opt, [-mbranch-protection=pac-ret -msign-return-address=all], [
+ AS_FOR(option, opt, [-mbranch-protection=standard -mbranch-protection=pac-ret -msign-return-address=all], [
RUBY_TRY_CFLAGS(option, [branch_protection=yes], [branch_protection=no])
AS_IF([test "x$branch_protection" = xyes], [
# C compiler and assembler must be consistent for -mbranch-protection

View File

@ -171,7 +171,7 @@
Summary: An interpreter of object-oriented scripting language
Name: ruby
Version: %{ruby_version}%{?development_release}
Release: 9%{?dist}
Release: 10%{?dist}
# Licenses, which are likely not included in binary RPMs:
# Apache-2.0:
# benchmark/gc/redblack.rb
@ -274,7 +274,9 @@ Patch9: ruby-3.3.0-Disable-syntax-suggest-test-case.patch
# Armv8.3+ capable CPUs might segfault with incorrect compilation options.
# See related upstream report: https://bugs.ruby-lang.org/issues/20085
# https://bugs.ruby-lang.org/issues/20154
Patch12: ruby-3.4.0-fix-branch-protection-compilation-for-arm.patch
# Make sure hardeding flags are correctly applied.
# https://bugs.ruby-lang.org/issues/20520
Patch12: ruby-3.4.0-Extract-hardening-CFLAGS-to-a-special-hardenflags-variable.patch
# Fix build issue on i686 due to "incompatible pointer type" error.
# https://bugs.ruby-lang.org/issues/20447
Patch13: ruby-3.4.0-Fix-pointer-incompatiblity.patch
@ -792,7 +794,6 @@ pushd %{_vpath_builddir}
--enable-shared \
--with-ruby-version='' \
--enable-multiarch \
--disable-fortify-source `# Should not really be needed: https://bugs.ruby-lang.org/issues/20520` \
%{?with_yjit: --enable-yjit} \
popd
@ -1722,6 +1723,9 @@ make -C %{_vpath_builddir} runruby TESTRUN_SCRIPT=" \
%changelog
* Tue Jun 18 2024 Vít Ondruch <vondruch@redhat.com> - 3.3.2-10
- Make sure hardening configuration flags are correctly applied.
* Thu Jun 06 2024 Vít Ondruch <vondruch@redhat.com> - 3.3.2-9
- Upgrade to Ruby 3.3.2.
Resolves: rhbz#2284020