https://fedoraproject.org/wiki/Changes/LIBFFI34_static_trampolines
This document represents a proposed Change. As part of the Changes process, proposals are publicly announced in order to receive community feedback. This proposal will only be implemented if approved by the Fedora Engineering Steering Committee.
== Summary == Libffi is currently configured to use dynamic trampolines, which require some source of memory which is both writable and executable. This is an obvious security issue, and selinux and system defaults have made it more and more difficult to safely provide this memory to libffi clients. With this change, libffi will be configured to use static trampolines, which do not require such memory, and will not pose those security and administrative risks.
== Owner ==
* Name: [[User:djdelorie| DJ Delorie]] * Email: dj@redhat.com
== Detailed Description == The change itself is simple - libffi will no longer be configured with --disable-exec-static-tramp, which changes how closures are generated. Closures, and libffi, are used in many interpreted languages. There are about 145 packages that depend, directly or indirectly, on libffi. I ran the Mass Prebuilder. Of those 145, 10 already FTBFS, and 1 saw a new failure.
The Mass PreBuilder has indicated that cjs (Javascript Bindings for Cinnamon) will fail to build with static trampolines. We debugged this a bit and noted that cjs's upstream seems to be behind the gjs upstream (the gjs package builds OK) it tracks, and is missing at least two closure-related changes (although simply adding those two changes proved nontrivial).
== Feedback ==
== Benefit to Fedora ==
This change is needed to fully secure Fedora systems against attacks that use self-modifying code.
The cjs build failure affects the Cinnamon spin.
== Scope == * Proposal owners: The change is a single line in the spec file.
* Other developers:
The cjs package will need to be fixed to build with the new libffi.
Other libffi users will need to be aware of this change if they make closure-related changes to their packages, but should not need to take any actions at this time.
* Release engineering: [https://pagure.io/releng/issues #Releng issue number]
This change does not affect release engineering or any other packaging issues. No mass rebuild is required.
* Policies and guidelines: N/A (not needed for this Change)
* Trademark approval: N/A (not needed for this Change)
* Alignment with Community Initiatives: N/A
== Upgrade/compatibility impact ==
This change does not change ABI but that assumes clients are not abusing the ABI; the static vs dynamic trampoline change is an internal implementation.
== How To Test ==
Testing static trampolines is the same as testing libffi, with the exception that the tests should pass despite having all writable filesystems locked with selinux such that executable maps cannot be requested. A security context that does not allow the memfd_create() syscall would further isolate this change.
== User Experience ==
Users will be able to follow latest security recommendations without concern for whether interpreters will stop working.
== Dependencies ==
As noted above, the cjs package requires work before this change can go through. They have noted that they'll be rebasing to 5.8.x: https://bugzilla.redhat.com/show_bug.cgi?id=2193287
Note that cjs has rebased to 5.6.1 in the interim, just to fix this bug. I've tested it and it works with static trampolines.
== Contingency Plan ==
Reverting the one line spec file change will revert this change, with no changes needed elsewhere. Not completing this change in time does not negatively impact other package, or Fedora's ability to ship on time.
* Contingency mechanism: We can revert the change at any time, without needing to rebuild any other packages.
* Contingency deadline: Any time before shipping.
* Blocks release? No.
== Documentation ==
This is an internal change that should not need nor affect any documentation.
== Release Notes ==
This change should not require any release notes.
-- Aoife Moloney
Product Owner
Community Platform Engineering Team
Red Hat EMEA
Communications House
Cork Road
Waterford
On Mon, May 08, 2023 at 02:50:21PM +0100, Aoife Moloney wrote:
https://fedoraproject.org/wiki/Changes/LIBFFI34_static_trampolines == Summary == Libffi is currently configured to use dynamic trampolines, which require some source of memory which is both writable and executable. This is an obvious security issue, and selinux and system defaults have made it more and more difficult to safely provide this memory to libffi clients. With this change, libffi will be configured to use static trampolines, which do not require such memory, and will not pose those security and administrative risks.
Sounds good. Can you post the list of affected packages here?
Zbyszek
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl writes:
Sounds good. Can you post the list of affected packages here?
According to mpb at least:
0ad Agda Io-language Macaulay2 ShellCheck alex bench brainfuck bustle cab cabal-install cabal-rpm chromium cjs cpphs darcs dhall dhall-json dl-fedora ecl fbrnch firefox gambas3 gforth ghc ghc-DAV ghc-HaXml ghc-aeson-pretty ghc-cheapskate ghc-clientsession ghc-criterion ghc-doctest ghc-hakyll ghc-hgettext ghc-highlighting-kate ghc-hjsmin ghc-hspec-discover ghc-pretty-show ghc-pretty-simple ghc-servant-server ghc-tidal ghc-vty ghc-wai-app-static ghc-wai-websockets ghc9.0 ghc9.2 ghc9.4 ghc9.6 ghcid git-annex git-repair gitit gjs glib2 gnustep-base gobject-introspection gtk2hs-buildtools guile guile22 guile30 hadolint happy haskell-platform hedgewars hledger hledger-ui hledger-web hlint hscolour hwk icecat idris ispc jna koji-tool libffi libomp librep lldb llvm llvm11 llvm12 llvm13 llvm14 llvm15 llvm7.0 llvm8.0 lsfrom lua-lgi micropython moarvm mozjs102 mozjs78 mozjs91 newlisp ocaml-ctypes ormolu p11-kit pagure-cli pandoc patat perl-Alien-FFI perl-FFI-Platypus perl-Glib-Object-Introspection php pkgtreediff polyml pygobject2 pygobject3 pypy pypy3.9 python-cffi python-pyopencl python-tox python2.7 python3.10 python3.11 python3.12 python3.6 python3.7 python3.8 python3.9 qt-creator racket rakudo rhbzquery rocm-runtime rpmbuild-order ruby rubygem-ffi seamonkey shake squeak-vm thunderbird unlambda wayland xmobar xmonad yosys
On Tue, May 9, 2023 at 4:35 AM DJ Delorie dj@redhat.com wrote:
According to mpb at least:
mpb?
The majority of those packages are maintained by me... so I can't say I thrilled. I thought ghc 9 was supposed to be okay with static trampolines? Jens
On Tue, May 9, 2023 at 12:18 PM Jens-Ulrik Petersen petersen@redhat.com wrote:
On Tue, May 9, 2023 at 4:35 AM DJ Delorie dj@redhat.com wrote:
According to mpb at least:
(Okay I found mpb https://gitlab.com/fedora/packager-tools/mass-prebuild) Are the results available?
The majority of those packages are maintained by me... so I can't say I thrilled. I thought ghc 9 was supposed to be okay with static trampolines?
Jens-Ulrik Petersen petersen@redhat.com writes:
According to mpb at least:
mpb?
Mass PreBuilder https://gitlab.com/fedora/packager-tools/mass-prebuild
The majority of those packages are maintained by me... so I can't say I thrilled. I thought ghc 9 was supposed to be okay with static trampolines?
MPB uses BuildRequires to collect all packages that *might* be affected by your change. It builds all those packages with and without your change, and lets you know what you broke. In this case, one broke (cjs, since fixed), ten didn't build before my change anyway[*], and the rest were OK. So if your package doesn't already FTBFS, you're good.
Are the results available?
MPB uses COPR, so..
"before" builds: https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4.checker/ "after" builds: https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4/
Note: I've done this before, so only look at the builds that are less than a month old. There should be an "after" build for all packages that BuildRequires libffi, and a "before" package for each package that failed to build "after".
[*] I'm still investigating these, but usually, it's not related to the change you're making.
On Wed, May 10, 2023 at 3:13 AM DJ Delorie dj@redhat.com wrote:
Jens-Ulrik Petersen petersen@redhat.com writes: MPB uses BuildRequires to collect all packages that *might* be affected by your change. It builds all those packages with and without your change, and lets you know what you broke. In this case, one broke (cjs, since fixed), ten didn't build before my change anyway[*], and the rest were OK. So if your package doesn't already FTBFS, you're good.
Okay thanks for the clarification and explanations. (I got now that by "affected packages" you didn't mean "failed":)
"after" builds:
https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4/
Okay I was already aware of all the ghc failures: mostly due to the new sphinx version in Rawhide (which I had already fixed for ghc9.4). So it looks fine from my pov so far, thanks. I have just now pushed fixes for all ghc*, so can you try to rebuild them again in your repo? Jens
Jens-Ulrik Petersen petersen@redhat.com writes:
I have just now pushed fixes for all ghc*, so can you try to rebuild them again in your repo?
That's a good question, to which I know not the answer. Fred? Can MPB be told to retest a specific set of packages? Or do I have to start from scratch and/or do them manually?
On Thu, May 11, 2023 at 7:27 AM DJ Delorie dj@redhat.com wrote:
Jens-Ulrik Petersen petersen@redhat.com writes:
I have just now pushed fixes for all ghc*, so can you try to rebuild them again in your repo?
That's a good question, to which I know not the answer. Fred? Can MPB be told to retest a specific set of packages? Or do I have to start from scratch and/or do them manually?i18n Engineering - Display Systems
Well, it is "only" ghc, ghc9.0, ghc9.2, and ghc9.6 (though ideally the ghc-* and other Haskell packages should also be built against the rebuilt ghc package). You could also just build them manually in your djdelorie/libffi-3.4.4 copr or in a separate copr if that is cleaner.
Jens
Jens-Ulrik Petersen petersen@redhat.com writes:
I have just now pushed fixes for all ghc*, so can you try to rebuild them again in your repo?
All succeeded :-)
https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4/builds/
On 5/8/23 15:50, Aoife Moloney wrote:
https://fedoraproject.org/wiki/Changes/LIBFFI34_static_trampolines
This document represents a proposed Change. As part of the Changes process, proposals are publicly announced in order to receive community feedback. This proposal will only be implemented if approved by the Fedora Engineering Steering Committee.
== Summary == Libffi is currently configured to use dynamic trampolines, which require some source of memory which is both writable and executable. This is an obvious security issue, and selinux and system defaults have made it more and more difficult to safely provide this memory to libffi clients. With this change, libffi will be configured to use static trampolines, which do not require such memory, and will not pose those security and administrative risks.
== Owner ==
- Name: [[User:djdelorie| DJ Delorie]]
- Email:dj@redhat.com
== Detailed Description == The change itself is simple - libffi will no longer be configured with --disable-exec-static-tramp, which changes how closures are generated. Closures, and libffi, are used in many interpreted languages. There are about 145 packages that depend, directly or indirectly, on libffi. I ran the Mass Prebuilder. Of those 145, 10 already FTBFS, and 1 saw a new failure.
The Mass PreBuilder has indicated that cjs (Javascript Bindings for Cinnamon) will fail to build with static trampolines. We debugged this a bit and noted that cjs's upstream seems to be behind the gjs upstream (the gjs package builds OK) it tracks, and is missing at least two closure-related changes (although simply adding those two changes proved nontrivial).
Are the libffi/rebuilt packages available anywhere for us to experiment with?
We have a reasonably reliable reproducer in Ruby [0] (also included in commit message [1]), but it is not executed as part of test suite,
Moreover, rebuild with current Ruby specfiles won't tell you much as we commented out the tests [2] to have less flaky builds. I'd recommend uncommenting the lines and run 5 to 10 builds (or just run any of the 2 reproducers).
Regards, Jarek Prokop
[0] https://bugzilla.redhat.com/show_bug.cgi?id=2040380#c5 [1] https://src.fedoraproject.org/rpms/ruby/c/c2026da1750e6d0cc70c7370a0840628bb... [2] https://src.fedoraproject.org/rpms/ruby/blob/9e39fd242a58a5ab286d5da0d54130a...
Jarek Prokop jprokop@redhat.com writes:
Are the libffi/rebuilt packages available anywhere for us to experiment with?
MPB uses COPR, so..
"before" builds: https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4.checker/ "after" builds: https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4/
We have a reasonably reliable reproducer in Ruby [0] (also included in commit message [1]), but it is not executed as part of test suite,
Yes, fork-without-exec case is a known "that should never have worked" case that only happens to work when your closure's backing store is also forked, which file-based mappings are *not*. You need either really old (rwx mmap, which security disables) or really new (static trampolines, which are r-x/rw- mmap'ed) libffi to support that. Hopefully that means your reproducers should not reproduce with the new libffi.
Moreover, rebuild with current Ruby specfiles won't tell you much as we commented out the tests [2] to have less flaky builds. I'd recommend uncommenting the lines and run 5 to 10 builds (or just run any of the 2 reproducers).
Well, if you comment out the tests, I have no way of knowing I broke anything, so have to rely on posting Change Requests and letting you let me know ;-)
Not saying you did anything wrong; if you have tests that pass or fail depending on system configurations outside your control, it's difficult to reliably test what you want to test. I'm just saying that when you disable tests, automated processes have no insight into those failures.
On 5/9/23 21:27, DJ Delorie wrote:
Jarek Prokopjprokop@redhat.com writes:
Are the libffi/rebuilt packages available anywhere for us to experiment with?
MPB uses COPR, so..
"before" builds:https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4.checker/ "after" builds:https://copr.fedorainfracloud.org/coprs/djdelorie/libffi-3.4.4/
Great! Thanks for the links, I've done manual testing with the reproducer: ~~~ require 'fiddle/closure' require 'fiddle/struct'
Fiddle::Closure.new(Fiddle::TYPE_VOID, [])
fork { }
GC.start ~~~
And with the copr build of libffi 3.4.4-3 , Ruby indeed no longer crashes on this code.
We have a reasonably reliable reproducer in Ruby [0] (also included in commit message [1]), but it is not executed as part of test suite,
Yes, fork-without-exec case is a known "that should never have worked" case that only happens to work when your closure's backing store is also forked, which file-based mappings are *not*. You need either really old (rwx mmap, which security disables) or really new (static trampolines, which are r-x/rw- mmap'ed) libffi to support that. Hopefully that means your reproducers should not reproduce with the new libffi.
Moreover, rebuild with current Ruby specfiles won't tell you much as we commented out the tests [2] to have less flaky builds. I'd recommend uncommenting the lines and run 5 to 10 builds (or just run any of the 2 reproducers).
Well, if you comment out the tests, I have no way of knowing I broke anything, so have to rely on posting Change Requests and letting you let me know ;-)
And it is great to see :)
Not saying you did anything wrong; if you have tests that pass or fail depending on system configurations outside your control, it's difficult to reliably test what you want to test. I'm just saying that when you disable tests, automated processes have no insight into those failures.
We are aware of this downside :/, we worked with relevant upstream, but a proper fix on the side of rubygem-fiddle would require nontrivial rewrite. (rubygem-ffi conversely has a closure pool that, AFAICT, prevents the issue altogether.)
This was a long-running issue (read: spanning a few Fedora releases) and doing a rebuild 10 times to have 1 not segfault and go through the rest of the pipeline just for a teeny version rebase got really tiring.
Thanks, Jarek Prokop
-
Aoife Moloney -
DJ Delorie -
Jarek Prokop -
Jens-Ulrik Petersen -
Zbigniew Jędrzejewski-Szmek