ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: "jprokop (Jarek Prokop) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "jprokop (Jarek Prokop)" <noreply@ruby-lang.org>
Subject: [ruby-core:116039] [Ruby master Bug#20154] aarch64: configure overrides `-mbranch-protection` if it was set in CFLAGS via environment
Date: Fri, 05 Jan 2024 21:25:03 +0000 (UTC)	[thread overview]
Message-ID: <redmine.issue-20154.20240105212503.52316@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-20154.20240105212503.52316@ruby-lang.org

Issue #20154 has been reported by jprokop (Jarek Prokop).

----------------------------------------
Bug #20154: aarch64: configure overrides `-mbranch-protection` if it was set in CFLAGS via environment
https://bugs.ruby-lang.org/issues/20154

* Author: jprokop (Jarek Prokop)
* Status: Open
* Priority: Normal
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Recently a GH PR was merged <https://github.com/ruby/ruby/pull/9306> For PAC/BTI support on ARM CPUs for Coroutine.S.

Without proper compilation support in configure.ac it segfaults Ruby with fibers on CPUs where PAC is supported: https://bugs.ruby-lang.org/issues/20085

At the time of writing, configure.ac appends the first option from a list for flag `-mbranch-protection` that successfully compiles a program <https://github.com/ruby/ruby/blob/master/configure.ac#L829>,
to XCFLAGS and now also ASFLAGS to fix issue 20085 for Ruby master.

This is suboptimal for Fedora as we set -mbranch-protection=standard by default in C{,XX}FLAGS:
```
CFLAGS='-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -mbranch-protection=standard -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer '
export CFLAGS
CXXFLAGS='-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -mbranch-protection=standard -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer'
export CXXFLAGS
```

And the appended flag overrides distribution's compilation configuration, which in this case ends up omitting BTI instructions and only using PAC.

Would it make sense to check if such flags exist and not overwrite them if they do?

Serious proposals:
1. Simplest fix that does not overwrite what is set in the distribution and results in higher security is simply prepending the list of options with `-mbranch-protection=standard`, it should cause no problems on ARMv8 CPUs and forward, BTI similarly to PAC instructions result into NOP, it is only extending the capability.

See attached 0001-aarch64-Check-mbranch-protection-standard-first-to-u.patch


2. Other fix that sounds more sane IMO and dodges this kind of guessing where are all the correct places for the flag is what another Fedora contributor Florian Weimer suggested: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/CVTNF2OQCL3XZHUUFNYMDK6ZEF2SWUEN/

"The reliable way to do this would be to compile a C file and check
whether that enables __ARM_FEATURE_PAC_DEFAULT, and if that's the case,
define a *different* macro for use in the assembler implementation.
This way, you don't need to care about the exact name of the option."

IOW instead of using __ARM_FEATURE_* directly in that code, define a macro in the style of "USE_PAC" with value of the feature if it is defined, I think that way we shouldn't need to append ASFLAGS anymore.

However it's also important to catch the value of those macros as their values have meaning, I have an idea how to do that but I'd get on that monday earliest.

---Files--------------------------------
0001-aarch64-Check-mbranch-protection-standard-first-to-u.patch (1004 Bytes)


-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

       reply	other threads:[~2024-01-05 21:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-05 21:25 jprokop (Jarek Prokop) via ruby-core [this message]
2024-01-05 21:27 ` [ruby-core:116040] [Ruby master Bug#20154] aarch64: configure overrides `-mbranch-protection` if it was set in CFLAGS via environment jprokop (Jarek Prokop) via ruby-core
2024-01-07 18:10 ` [ruby-core:116062] " katei (Yuta Saito) via ruby-core
2024-04-23 15:27 ` [ruby-core:117654] " vo.x (Vit Ondruch) via ruby-core

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.issue-20154.20240105212503.52316@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    --cc=noreply@ruby-lang.org \
    --cc=ruby-core@ml.ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).