From 7724c2d703c1d1dce91762d00489bd26a3785b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Ondruch?= Date: Tue, 18 Jun 2024 12:28:13 +0200 Subject: [PATCH] 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. --- ...GS-to-a-special-hardenflags-variable.patch | 302 ++++++++++++++++++ ...ranch-protection-compilation-for-arm.patch | 24 -- ruby.spec | 10 +- 3 files changed, 309 insertions(+), 27 deletions(-) create mode 100644 ruby-3.4.0-Extract-hardening-CFLAGS-to-a-special-hardenflags-variable.patch delete mode 100644 ruby-3.4.0-fix-branch-protection-compilation-for-arm.patch diff --git a/ruby-3.4.0-Extract-hardening-CFLAGS-to-a-special-hardenflags-variable.patch b/ruby-3.4.0-Extract-hardening-CFLAGS-to-a-special-hardenflags-variable.patch new file mode 100644 index 0000000..b03b618 --- /dev/null +++ b/ruby-3.4.0-Extract-hardening-CFLAGS-to-a-special-hardenflags-variable.patch @@ -0,0 +1,302 @@ +From 3d405634f43d39079ee93cdc59ed7fc0a5e8917a Mon Sep 17 00:00:00 2001 +From: KJ Tsanaktsidis +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 ]) + ]) + +@@ -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 diff --git a/ruby-3.4.0-fix-branch-protection-compilation-for-arm.patch b/ruby-3.4.0-fix-branch-protection-compilation-for-arm.patch deleted file mode 100644 index 45ed5a9..0000000 --- a/ruby-3.4.0-fix-branch-protection-compilation-for-arm.patch +++ /dev/null @@ -1,24 +0,0 @@ -From db4ba95bf12f9303e38a9a78979cd363cb9a19fb Mon Sep 17 00:00:00 2001 -From: Jarek Prokop -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 diff --git a/ruby.spec b/ruby.spec index a34a5cd..b6f6900 100644 --- a/ruby.spec +++ b/ruby.spec @@ -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 - 3.3.2-10 +- Make sure hardening configuration flags are correctly applied. + * Thu Jun 06 2024 Vít Ondruch - 3.3.2-9 - Upgrade to Ruby 3.3.2. Resolves: rhbz#2284020