Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit 40dacd5

Browse files
committed
Auto merge of #139632 - Darksonn:cfi-fmt, r=m-ou-se
cfi: do not transmute function pointers in formatting code Follow-up to #115954. Addresses #115199 point 2. Related to #128728. Discussion [on the LKML](https://lore.kernel.org/all/20250410115420.366349-1-panikiel@google.com/). cc `@maurer` `@rcvalle` `@RalfJung`
2 parents f433fa4 + 6513df9 commit 40dacd5

File tree

3 files changed

+50
-25
lines changed

3 files changed

+50
-25
lines changed

‎library/core/src/fmt/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::char::{EscapeDebugExtArgs, MAX_LEN_UTF8};
77
use crate::marker::PhantomData;
88
use crate::num::fmt as numfmt;
99
use crate::ops::Deref;
10-
use crate::{iter, mem,result, str};
10+
use crate::{iter, result, str};
1111

1212
mod builders;
1313
#[cfg(not(no_fp_fmt_parse))]

‎library/core/src/fmt/rt.rs

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -65,61 +65,92 @@ pub struct Argument<'a> {
6565
ty: ArgumentType<'a>,
6666
}
6767

68-
#[rustc_diagnostic_item = "ArgumentMethods"]
69-
impl Argument<'_> {
70-
#[inline]
71-
const fn new<'a, T>(x: &'a T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument<'a> {
68+
macro_rules! argument_new {
69+
($t:ty, $x:expr, $f:expr) => {
7270
Argument {
7371
// INVARIANT: this creates an `ArgumentType<'a>` from a `&'a T` and
7472
// a `fn(&T, ...)`, so the invariant is maintained.
7573
ty: ArgumentType::Placeholder {
76-
value: NonNull::from_ref(x).cast(),
77-
// SAFETY: function pointers always have the same layout.
78-
formatter: unsafe { mem::transmute(f) },
74+
value: NonNull::<$t>::from_ref($x).cast(),
75+
// The Rust ABI considers all pointers to be equivalent, so transmuting a fn(&T) to
76+
// fn(NonNull<()>) and calling it with a NonNull<()> that points at a T is allowed.
77+
// However, the CFI sanitizer does not allow this, and triggers a crash when it
78+
// happens.
79+
//
80+
// To avoid this crash, we use a helper function when CFI is enabled. To avoid the
81+
// cost of this helper function (mainly code-size) when it is not needed, we
82+
// transmute the function pointer otherwise.
83+
//
84+
// This is similar to what the Rust compiler does internally with vtables when KCFI
85+
// is enabled, where it generates trampoline functions that only serve to adjust the
86+
// expected type of the argument. `ArgumentType::Placeholder` is a bit like a
87+
// manually constructed trait object, so it is not surprising that the same approach
88+
// has to be applied here as well.
89+
//
90+
// It is still considered problematic (from the Rust side) that CFI rejects entirely
91+
// legal Rust programs, so we do not consider anything done here a stable guarantee,
92+
// but meanwhile we carry this work-around to keep Rust compatible with CFI and
93+
// KCFI.
94+
#[cfg(not(any(sanitize = "cfi", sanitize = "kcfi")))]
95+
formatter: {
96+
let f: fn(&$t, &mut Formatter<'_>) -> Result = $f;
97+
// SAFETY: This is only called with `value`, which has the right type.
98+
unsafe { core::mem::transmute(f) }
99+
},
100+
#[cfg(any(sanitize = "cfi", sanitize = "kcfi"))]
101+
formatter: |ptr: NonNull<()>, fmt: &mut Formatter<'_>| {
102+
let func = $f;
103+
// SAFETY: This is the same type as the `value` field.
104+
let r = unsafe { ptr.cast::<$t>().as_ref() };
105+
(func)(r, fmt)
106+
},
79107
_lifetime: PhantomData,
80108
},
81109
}
82-
}
110+
};
111+
}
83112

113+
#[rustc_diagnostic_item = "ArgumentMethods"]
114+
impl Argument<'_> {
84115
#[inline]
85116
pub fn new_display<T: Display>(x: &T) -> Argument<'_> {
86-
Self::new(x, Display::fmt)
117+
argument_new!(T,x, <TasDisplay>::fmt)
87118
}
88119
#[inline]
89120
pub fn new_debug<T: Debug>(x: &T) -> Argument<'_> {
90-
Self::new(x, Debug::fmt)
121+
argument_new!(T,x, <TasDebug>::fmt)
91122
}
92123
#[inline]
93124
pub fn new_debug_noop<T: Debug>(x: &T) -> Argument<'_> {
94-
Self::new(x, |_, _| Ok(()))
125+
argument_new!(T,x, |_:&T, _| Ok(()))
95126
}
96127
#[inline]
97128
pub fn new_octal<T: Octal>(x: &T) -> Argument<'_> {
98-
Self::new(x, Octal::fmt)
129+
argument_new!(T,x, <TasOctal>::fmt)
99130
}
100131
#[inline]
101132
pub fn new_lower_hex<T: LowerHex>(x: &T) -> Argument<'_> {
102-
Self::new(x, LowerHex::fmt)
133+
argument_new!(T,x, <TasLowerHex>::fmt)
103134
}
104135
#[inline]
105136
pub fn new_upper_hex<T: UpperHex>(x: &T) -> Argument<'_> {
106-
Self::new(x, UpperHex::fmt)
137+
argument_new!(T,x, <TasUpperHex>::fmt)
107138
}
108139
#[inline]
109140
pub fn new_pointer<T: Pointer>(x: &T) -> Argument<'_> {
110-
Self::new(x, Pointer::fmt)
141+
argument_new!(T,x, <TasPointer>::fmt)
111142
}
112143
#[inline]
113144
pub fn new_binary<T: Binary>(x: &T) -> Argument<'_> {
114-
Self::new(x, Binary::fmt)
145+
argument_new!(T,x, <TasBinary>::fmt)
115146
}
116147
#[inline]
117148
pub fn new_lower_exp<T: LowerExp>(x: &T) -> Argument<'_> {
118-
Self::new(x, LowerExp::fmt)
149+
argument_new!(T,x, <TasLowerExp>::fmt)
119150
}
120151
#[inline]
121152
pub fn new_upper_exp<T: UpperExp>(x: &T) -> Argument<'_> {
122-
Self::new(x, UpperExp::fmt)
153+
argument_new!(T,x, <TasUpperExp>::fmt)
123154
}
124155
#[inline]
125156
#[track_caller]
@@ -135,11 +166,6 @@ impl Argument<'_> {
135166
/// # Safety
136167
///
137168
/// This argument must actually be a placeholder argument.
138-
///
139-
// FIXME: Transmuting formatter in new and indirectly branching to/calling
140-
// it here is an explicit CFI violation.
141-
#[allow(inline_no_sanitize)]
142-
#[no_sanitize(cfi, kcfi)]
143169
#[inline]
144170
pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {
145171
match self.ty {

‎library/core/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@
169169
#![feature(negative_impls)]
170170
#![feature(never_type)]
171171
#![feature(no_core)]
172-
#![feature(no_sanitize)]
173172
#![feature(optimize_attribute)]
174173
#![feature(prelude_import)]
175174
#![feature(repr_simd)]

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /