Skip to content

cfi: do not transmute function pointers in formatting code #139632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Apr 15, 2025
Merged

cfi: do not transmute function pointers in formatting code #139632

merged 7 commits into from
Apr 15, 2025

Conversation

Darksonn
Copy link
Contributor

Follow-up to #115954.
Addresses #115199 point 2.
Related to #128728.
Discussion on the LKML.

cc @maurer @rcvalle @RalfJung

@rustbot
Copy link
Collaborator

rustbot commented Apr 10, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 10, 2025
@RalfJung
Copy link
Member

RalfJung commented Apr 10, 2025 •
edited
Loading

From a @rust-lang/opsem perspective (but speaking only for myself), I continue to have my gripes with CFI complaining about code that Rust considers to be entirely well-defined. I don't think we want to make a promise to follow some arbitrary rules that some third-party tool is enforcing. If @rust-lang/libs wants to carry this as a work-around until the situation is resolved, that's fine for me.

The proper fix is to figure out whether we can adjust CFI and the Rust spec to make "code rejected by CFI" a (strict) subset of "code that has UB or EB". But making all fn ptr transmute / type erasure schemes EB doesn't sound good, I assume there's people out there that rely on this working properly.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Apr 10, 2025

Some changes occurred in coverage tests.

cc @Zalathar

@m-ou-se m-ou-se assigned m-ou-se and unassigned ibraheemdev Apr 10, 2025
@ojeda ojeda mentioned this pull request Apr 10, 2025
Open
16 tasks
@rcvalle rcvalle added PG-exploit-mitigations Project group: Exploit mitigations A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation labels Apr 10, 2025
@rcvalle
Copy link
Member

rcvalle commented Apr 10, 2025

@rustbot claim

@rustbot rustbot assigned rcvalle and unassigned m-ou-se Apr 10, 2025
@rcvalle rcvalle requested a review from m-ou-se April 10, 2025 17:28
@rcvalle
Copy link
Member

rcvalle commented Apr 10, 2025

@Darksonn since this issue happens with FineIBT only, would it be possible to add a regression test or test for FineIBT? (I don't know whether we can do that with our current test and CI infrastructure tbh.)

@maurer
Copy link
Contributor

maurer commented Apr 10, 2025

Re: FineIBT, that would be difficult to test because it would require us to pull objtool from the Linux kernel into the test environment - it's implemented by editing the compiled code after compilation.

The main way we could add a regression test would be to forbid #[no_sanitize(kcfi)] in core, as the problem with FineIBT is that it does not support not sanitizing a call-site, because the typecheck part of the sanitization is implemented at the call target (it relies on endbr to be able to assume the target will have sanitization).

@rcvalle
Copy link
Member

rcvalle commented Apr 10, 2025

Got it. Thank you, @maurer! SGTM. FYI, @1c3t3a and @jakos-sec are working on fixing all issues listed in #115199 and already fixed the weakly-linked functions issue in #138349 (which unblocked fixing some of the issues listed there), and will soon remove all no_sanitize in core and stdlib.

For this, I guess now for this it's whether the @rust-lang/libs is okay with the small refactoring as @RalfJung mentioned.

@rcvalle

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 10, 2025
@rcvalle

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 10, 2025
@rcvalle

This comment has been minimized.

Sign in to view
@rcvalle

This comment has been minimized.

Sign in to view
tamird and others added 3 commits April 15, 2025 07:24
@Darksonn
Copy link
Contributor Author

Squashed as per your request.

@Darksonn
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 15, 2025
@m-ou-se
Copy link
Member

m-ou-se commented Apr 15, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 15, 2025

📌 Commit 6513df9 has been approved by m-ou-se

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2025
@bors
Copy link
Collaborator

bors commented Apr 15, 2025

⌛ Testing commit 6513df9 with merge 40dacd5...

@bors
Copy link
Collaborator

bors commented Apr 15, 2025

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 40dacd5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 15, 2025
@bors bors merged commit 40dacd5 into rust-lang:master Apr 15, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 15, 2025
@Darksonn Darksonn deleted the cfi-fmt branch April 15, 2025 14:17
@github-actions GitHub Actions
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing f433fa4 (parent) -> 40dacd5 (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Job duration changes

  1. dist-aarch64-linux: 8318.3s -> 5158.2s (-38.0%)
  2. x86_64-apple-2: 5541.6s -> 6595.0s (19.0%)
  3. dist-i686-msvc: 8273.7s -> 7045.0s (-14.9%)
  4. dist-android: 2476.6s -> 2702.6s (9.1%)
  5. dist-apple-various: 8429.3s -> 7712.8s (-8.5%)
  6. dist-powerpc64le-linux: 9336.1s -> 10089.3s (8.1%)
  7. x86_64-gnu-distcheck: 4846.0s -> 4460.8s (-8.0%)
  8. dist-x86_64-apple: 9388.7s -> 8724.0s (-7.1%)
  9. x86_64-apple-1: 8917.4s -> 8366.4s (-6.2%)
  10. dist-powerpc64-linux: 5502.1s -> 5831.2s (6.0%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

This was referenced Apr 15, 2025
Closed
Closed
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (40dacd5): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -0.6%, secondary -2.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 1.2% [0.4%, 3.6%] 8
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -5.5% [-11.9%, -0.7%] 3
Improvements ✅
(secondary)		 -2.6% [-2.6%, -2.6%] 1
All ❌✅ (primary) -0.6% [-11.9%, 3.6%] 11

Cycles

Results (primary 0.5%, secondary 9.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.5% [0.5%, 0.5%] 3
Regressions ❌
(secondary)		 9.6% [9.6%, 9.6%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.5% [0.5%, 0.5%] 3

Binary size

Results (primary -0.1%, secondary -0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.1% [0.0%, 0.3%] 4
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.1% [-0.3%, -0.0%] 46
Improvements ✅
(secondary)		 -0.0% [-0.0%, -0.0%] 4
All ❌✅ (primary) -0.1% [-0.3%, 0.3%] 50

Bootstrap: 781.719s -> 781.212s (-0.06%)
Artifact size: 365.16 MiB -> 365.11 MiB (-0.01%)

@aDotInTheVoid aDotInTheVoid added the A-rust-for-linux Relevant for the Rust-for-Linux project label Apr 15, 2025
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 19, 2025
@bors
Auto merge of rust-lang#139632 - Darksonn:cfi-fmt, r=m-ou-se
8c88f85 
cfi: do not transmute function pointers in formatting code

Follow-up to rust-lang#115954.
Addresses rust-lang#115199 point 2.
Related to rust-lang#128728.
Discussion [on the LKML](https://lore.kernel.org/all/[email protected]/).

cc `@maurer` `@rcvalle` `@RalfJung`
@Darksonn Darksonn mentioned this pull request Apr 23, 2025
Open
hubot pushed a commit to aosp-mirror/kernel_common that referenced this pull request Apr 30, 2025
@panikiel
ANDROID: gki_defconfig: do not use FineIBT on x86
e5ea70a 
The kernel is currently configured to patch itself to use FineIBT
instead of the traditional kCFI implementation when the hardware
supports IBT. However, due to recent security concerns with the design
of FineIBT [1], enabling FineIBT is currently considered a security
downgrade. Thus, disable this option and always use traditional kCFI.

This issue was discovered in relation to an issue with Rust drivers (see
the referenced bug). That issue has already been fixed [2][3][4].

The enablement of FineIBT happens at runtime, and it's controlled by
the cfi= kernel parameter. cfi=auto always enables FineIBT if the CPU
supports IBT, whereas cfi=kcfi tells the kernel not to use FineIBT.
The default value for this parameter depends on CONFIG_CFI_AUTO_DEFAULT.
If set, it's cfi=auto, if unset, it's cfi=kcfi.

This patch does not modify arch/arm64/configs/gki_defconfig because
FineIBT does not exist on ARM, as it relies on the hardware feature IBT
that only exists on x86.

Link: https://lore.kernel.org/linux-hardening/Z60NwR4w%2F28Z7XUa@ubun/ [1]
Link: rust-lang/rust#139632 [2]
Link: https://r.android.com/3602350 [3]
Link: https://lore.kernel.org/all/[email protected] [4]
Bug: 404795641
Change-Id: Idfb23c122e53d2d46612cceff0d1dbf5c1c0c813
Signed-off-by: Paweł Anikiel <[email protected]>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
@panikiel @ojeda
x86/Kconfig: make CFI_AUTO_DEFAULT depend on !RUST or Rust >= 1.88
0461335 
Calling core::fmt::write() from rust code while FineIBT is enabled
results in a kernel panic:

[ 4614.199779] kernel BUG at arch/x86/kernel/cet.c:132!
[ 4614.205343] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 4614.211781] CPU: 2 UID: 0 PID: 6057 Comm: dmabuf_dump Tainted: G     U     O       6.12.17-android16-0-g6ab38c534a43 #1 9da040f27673ec3945e23b998a0f8bd64c846599
[ 4614.227832] Tainted: [U]=USER, [O]=OOT_MODULE
[ 4614.241247] RIP: 0010:do_kernel_cp_fault+0xea/0xf0
...
[ 4614.398144] RIP: 0010:_RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x0/0x20
[ 4614.407792] Code: 48 f7 df 48 0f 48 f9 48 89 f2 89 c6 5d e9 18 fd ff ff 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 41 81 ea 14 61 af 2c 74 03 0f 0b 90 <66> 0f 1f 00 55 48 89 e5 48 89 f2 48 8b 3f be 01 00 00 00 5d e9 e7
[ 4614.428775] RSP: 0018:ffffb95acfa4ba68 EFLAGS: 00010246
[ 4614.434609] RAX: 0000000000000000 RBX: 0000000000000010 RCX: 0000000000000000
[ 4614.442587] RDX: 0000000000000007 RSI: ffffb95acfa4ba70 RDI: ffffb95acfa4bc88
[ 4614.450557] RBP: ffffb95acfa4bae0 R08: ffff0a00ffffff05 R09: 0000000000000070
[ 4614.458527] R10: 0000000000000000 R11: ffffffffab67eaf0 R12: ffffb95acfa4bcc8
[ 4614.466493] R13: ffffffffac5d50f0 R14: 0000000000000000 R15: 0000000000000000
[ 4614.474473]  ? __cfi__RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x10/0x10
[ 4614.484118]  ? _RNvNtCs3o2tGsuHyou_4core3fmt5write+0x1d2/0x250

This happens because core::fmt::write() calls
core::fmt::rt::Argument::fmt(), which currently has CFI disabled:

library/core/src/fmt/rt.rs:
171     // FIXME: Transmuting formatter in new and indirectly branching to/calling
172     // it here is an explicit CFI violation.
173     #[allow(inline_no_sanitize)]
174     #[no_sanitize(cfi, kcfi)]
175     #[inline]
176     pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {

This causes a Control Protection exception, because FineIBT has sealed
off the original function's endbr64.

This makes rust currently incompatible with FineIBT. Add a Kconfig
dependency that prevents FineIBT from getting turned on by default
if rust is enabled.

[ Rust 1.88.0 (scheduled for 2025-06-26) should have this fixed [1],
  and thus we relaxed the condition with Rust >= 1.88.

  When `objtool` lands checking for this with e.g. [2], the plan is
  to ideally run that in upstream Rust's CI to prevent regressions
  early [3], since we do not control `core`'s source code.

  Alice tested the Rust PR backported to an older compiler.

  Peter would like that Rust provides a stable `core` which can be
  pulled into the kernel: "Relying on that much out of tree code is
  'unfortunate'".

    - Miguel ]

Signed-off-by: Paweł Anikiel <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Link: rust-lang/rust#139632 [1]
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2]
Link: rust-lang/rust#139632 (comment) [3]
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/att0-CANiq72kjDM0cKALVy4POEzhfdT4nO7tqz0Pm7xM+3=_0+L1t=A@mail.gmail.com
[ Reduced splat. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
@panikiel @ojeda
x86/Kconfig: make CFI_AUTO_DEFAULT depend on !RUST or Rust >= 1.88
54d51d6 
Calling core::fmt::write() from rust code while FineIBT is enabled
results in a kernel panic:

[ 4614.199779] kernel BUG at arch/x86/kernel/cet.c:132!
[ 4614.205343] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 4614.211781] CPU: 2 UID: 0 PID: 6057 Comm: dmabuf_dump Tainted: G     U     O       6.12.17-android16-0-g6ab38c534a43 #1 9da040f27673ec3945e23b998a0f8bd64c846599
[ 4614.227832] Tainted: [U]=USER, [O]=OOT_MODULE
[ 4614.241247] RIP: 0010:do_kernel_cp_fault+0xea/0xf0
...
[ 4614.398144] RIP: 0010:_RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x0/0x20
[ 4614.407792] Code: 48 f7 df 48 0f 48 f9 48 89 f2 89 c6 5d e9 18 fd ff ff 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 41 81 ea 14 61 af 2c 74 03 0f 0b 90 <66> 0f 1f 00 55 48 89 e5 48 89 f2 48 8b 3f be 01 00 00 00 5d e9 e7
[ 4614.428775] RSP: 0018:ffffb95acfa4ba68 EFLAGS: 00010246
[ 4614.434609] RAX: 0000000000000000 RBX: 0000000000000010 RCX: 0000000000000000
[ 4614.442587] RDX: 0000000000000007 RSI: ffffb95acfa4ba70 RDI: ffffb95acfa4bc88
[ 4614.450557] RBP: ffffb95acfa4bae0 R08: ffff0a00ffffff05 R09: 0000000000000070
[ 4614.458527] R10: 0000000000000000 R11: ffffffffab67eaf0 R12: ffffb95acfa4bcc8
[ 4614.466493] R13: ffffffffac5d50f0 R14: 0000000000000000 R15: 0000000000000000
[ 4614.474473]  ? __cfi__RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x10/0x10
[ 4614.484118]  ? _RNvNtCs3o2tGsuHyou_4core3fmt5write+0x1d2/0x250

This happens because core::fmt::write() calls
core::fmt::rt::Argument::fmt(), which currently has CFI disabled:

library/core/src/fmt/rt.rs:
171     // FIXME: Transmuting formatter in new and indirectly branching to/calling
172     // it here is an explicit CFI violation.
173     #[allow(inline_no_sanitize)]
174     #[no_sanitize(cfi, kcfi)]
175     #[inline]
176     pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {

This causes a Control Protection exception, because FineIBT has sealed
off the original function's endbr64.

This makes rust currently incompatible with FineIBT. Add a Kconfig
dependency that prevents FineIBT from getting turned on by default
if rust is enabled.

[ Rust 1.88.0 (scheduled for 2025-06-26) should have this fixed [1],
  and thus we relaxed the condition with Rust >= 1.88.

  When `objtool` lands checking for this with e.g. [2], the plan is
  to ideally run that in upstream Rust's CI to prevent regressions
  early [3], since we do not control `core`'s source code.

  Alice tested the Rust PR backported to an older compiler.

  Peter would like that Rust provides a stable `core` which can be
  pulled into the kernel: "Relying on that much out of tree code is
  'unfortunate'".

    - Miguel ]

Signed-off-by: Paweł Anikiel <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Link: rust-lang/rust#139632 [1]
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2]
Link: rust-lang/rust#139632 (comment) [3]
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/att0-CANiq72kjDM0cKALVy4POEzhfdT4nO7tqz0Pm7xM+3=_0+L1t=A@mail.gmail.com
[ Reduced splat. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
@panikiel @ojeda
x86/Kconfig: make CFI_AUTO_DEFAULT depend on !RUST or Rust >= 1.88
66fdd69 
Calling core::fmt::write() from rust code while FineIBT is enabled
results in a kernel panic:

[ 4614.199779] kernel BUG at arch/x86/kernel/cet.c:132!
[ 4614.205343] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 4614.211781] CPU: 2 UID: 0 PID: 6057 Comm: dmabuf_dump Tainted: G     U     O       6.12.17-android16-0-g6ab38c534a43 #1 9da040f27673ec3945e23b998a0f8bd64c846599
[ 4614.227832] Tainted: [U]=USER, [O]=OOT_MODULE
[ 4614.241247] RIP: 0010:do_kernel_cp_fault+0xea/0xf0
...
[ 4614.398144] RIP: 0010:_RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x0/0x20
[ 4614.407792] Code: 48 f7 df 48 0f 48 f9 48 89 f2 89 c6 5d e9 18 fd ff ff 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 41 81 ea 14 61 af 2c 74 03 0f 0b 90 <66> 0f 1f 00 55 48 89 e5 48 89 f2 48 8b 3f be 01 00 00 00 5d e9 e7
[ 4614.428775] RSP: 0018:ffffb95acfa4ba68 EFLAGS: 00010246
[ 4614.434609] RAX: 0000000000000000 RBX: 0000000000000010 RCX: 0000000000000000
[ 4614.442587] RDX: 0000000000000007 RSI: ffffb95acfa4ba70 RDI: ffffb95acfa4bc88
[ 4614.450557] RBP: ffffb95acfa4bae0 R08: ffff0a00ffffff05 R09: 0000000000000070
[ 4614.458527] R10: 0000000000000000 R11: ffffffffab67eaf0 R12: ffffb95acfa4bcc8
[ 4614.466493] R13: ffffffffac5d50f0 R14: 0000000000000000 R15: 0000000000000000
[ 4614.474473]  ? __cfi__RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x10/0x10
[ 4614.484118]  ? _RNvNtCs3o2tGsuHyou_4core3fmt5write+0x1d2/0x250

This happens because core::fmt::write() calls
core::fmt::rt::Argument::fmt(), which currently has CFI disabled:

library/core/src/fmt/rt.rs:
171     // FIXME: Transmuting formatter in new and indirectly branching to/calling
172     // it here is an explicit CFI violation.
173     #[allow(inline_no_sanitize)]
174     #[no_sanitize(cfi, kcfi)]
175     #[inline]
176     pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {

This causes a Control Protection exception, because FineIBT has sealed
off the original function's endbr64.

This makes rust currently incompatible with FineIBT. Add a Kconfig
dependency that prevents FineIBT from getting turned on by default
if rust is enabled.

[ Rust 1.88.0 (scheduled for 2025-06-26) should have this fixed [1],
  and thus we relaxed the condition with Rust >= 1.88.

  When `objtool` lands checking for this with e.g. [2], the plan is
  to ideally run that in upstream Rust's CI to prevent regressions
  early [3], since we do not control `core`'s source code.

  Alice tested the Rust PR backported to an older compiler.

  Peter would like that Rust provides a stable `core` which can be
  pulled into the kernel: "Relying on that much out of tree code is
  'unfortunate'".

    - Miguel ]

Signed-off-by: Paweł Anikiel <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Link: rust-lang/rust#139632 [1]
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2]
Link: rust-lang/rust#139632 (comment) [3]
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/att0-CANiq72kjDM0cKALVy4POEzhfdT4nO7tqz0Pm7xM+3=_0+L1t=A@mail.gmail.com
[ Reduced splat. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
@panikiel @ojeda
x86/Kconfig: make CFI_AUTO_DEFAULT depend on !RUST or Rust >= 1.88
5595c31 
Calling core::fmt::write() from rust code while FineIBT is enabled
results in a kernel panic:

[ 4614.199779] kernel BUG at arch/x86/kernel/cet.c:132!
[ 4614.205343] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 4614.211781] CPU: 2 UID: 0 PID: 6057 Comm: dmabuf_dump Tainted: G     U     O       6.12.17-android16-0-g6ab38c534a43 #1 9da040f27673ec3945e23b998a0f8bd64c846599
[ 4614.227832] Tainted: [U]=USER, [O]=OOT_MODULE
[ 4614.241247] RIP: 0010:do_kernel_cp_fault+0xea/0xf0
...
[ 4614.398144] RIP: 0010:_RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x0/0x20
[ 4614.407792] Code: 48 f7 df 48 0f 48 f9 48 89 f2 89 c6 5d e9 18 fd ff ff 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 41 81 ea 14 61 af 2c 74 03 0f 0b 90 <66> 0f 1f 00 55 48 89 e5 48 89 f2 48 8b 3f be 01 00 00 00 5d e9 e7
[ 4614.428775] RSP: 0018:ffffb95acfa4ba68 EFLAGS: 00010246
[ 4614.434609] RAX: 0000000000000000 RBX: 0000000000000010 RCX: 0000000000000000
[ 4614.442587] RDX: 0000000000000007 RSI: ffffb95acfa4ba70 RDI: ffffb95acfa4bc88
[ 4614.450557] RBP: ffffb95acfa4bae0 R08: ffff0a00ffffff05 R09: 0000000000000070
[ 4614.458527] R10: 0000000000000000 R11: ffffffffab67eaf0 R12: ffffb95acfa4bcc8
[ 4614.466493] R13: ffffffffac5d50f0 R14: 0000000000000000 R15: 0000000000000000
[ 4614.474473]  ? __cfi__RNvXs5_NtNtNtCs3o2tGsuHyou_4core3fmt3num3impyNtB9_7Display3fmt+0x10/0x10
[ 4614.484118]  ? _RNvNtCs3o2tGsuHyou_4core3fmt5write+0x1d2/0x250

This happens because core::fmt::write() calls
core::fmt::rt::Argument::fmt(), which currently has CFI disabled:

library/core/src/fmt/rt.rs:
171     // FIXME: Transmuting formatter in new and indirectly branching to/calling
172     // it here is an explicit CFI violation.
173     #[allow(inline_no_sanitize)]
174     #[no_sanitize(cfi, kcfi)]
175     #[inline]
176     pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {

This causes a Control Protection exception, because FineIBT has sealed
off the original function's endbr64.

This makes rust currently incompatible with FineIBT. Add a Kconfig
dependency that prevents FineIBT from getting turned on by default
if rust is enabled.

[ Rust 1.88.0 (scheduled for 2025-06-26) should have this fixed [1],
  and thus we relaxed the condition with Rust >= 1.88.

  When `objtool` lands checking for this with e.g. [2], the plan is
  to ideally run that in upstream Rust's CI to prevent regressions
  early [3], since we do not control `core`'s source code.

  Alice tested the Rust PR backported to an older compiler.

  Peter would like that Rust provides a stable `core` which can be
  pulled into the kernel: "Relying on that much out of tree code is
  'unfortunate'".

    - Miguel ]

Signed-off-by: Paweł Anikiel <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Link: rust-lang/rust#139632 [1]
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2]
Link: rust-lang/rust#139632 (comment) [3]
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/att0-CANiq72kjDM0cKALVy4POEzhfdT4nO7tqz0Pm7xM+3=_0+L1t=A@mail.gmail.com
[ Reduced splat. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@tamird tamird tamird approved these changes

@rcvalle rcvalle rcvalle left review comments

@workingjubilee workingjubilee workingjubilee left review comments

@m-ou-se m-ou-se Awaiting requested review from m-ou-se

Assignees

@m-ou-se m-ou-se

Labels
A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation A-rust-for-linux Relevant for the Rust-for-Linux project CI-spurious-fail-msvc CI spurious failure: target env msvc merged-by-bors This PR was explicitly merged by bors. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.