Skip to content

Commit 4a269f5

Browse files
committed
cmse: lint when unions cross the security boundary
When a union passes from secure to non-secure (so, passed as an argument to an nonsecure call, or returned by a nonsecure entry), warn that there may be secure information lingering in the unused or uninitialized parts of a union value. https://godbolt.org/z/vq9xnrnEs
1 parent 2ce515a commit 4a269f5

File tree

14 files changed

+388
-60
lines changed

14 files changed

+388
-60
lines changed

compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use rustc_abi::{BackendRepr, ExternAbi, Float, Integer, Primitive, Scalar};
1+
use rustc_abi::{BackendRepr, ExternAbi, Float, Integer, Primitive};
22
use rustc_errors::{DiagCtxtHandle, E0781, struct_span_code_err};
33
use rustc_hir::{self as hir, HirId};
44
use rustc_middle::bug;
@@ -163,11 +163,7 @@ fn is_valid_cmse_output_layout<'tcx>(layout: TyAndLayout<'tcx>) -> bool {
163163
return false;
164164
};
165165

166-
let Scalar::Initialized { value, .. } = scalar else {
167-
return false;
168-
};
169-
170-
matches!(value, Primitive::Int(Integer::I64, _) | Primitive::Float(Float::F64))
166+
matches!(scalar.primitive(), Primitive::Int(Integer::I64, _) | Primitive::Float(Float::F64))
171167
}
172168

173169
fn should_emit_layout_error<'tcx>(abi: ExternAbi, layout_err: &'tcx LayoutError<'tcx>) -> bool {

compiler/rustc_lint/messages.ftl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,10 @@ lint_closure_returning_async_block = closure returning async block can be made i
187187
.label = this async block can be removed, and the closure can be turned into an async closure
188188
.suggestion = turn this into an async closure
189189
190+
lint_cmse_union_may_leak_information =
191+
passing a union across the security boundary may leak information
192+
.note = the bits not used by the current variant may contain stale secure data
193+
190194
lint_command_line_source = `forbid` lint level was set on command line
191195
192196
lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as identifiers, which look alike
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
use rustc_abi::ExternAbi;
2+
use rustc_hir::{self as hir, Expr, ExprKind};
3+
use rustc_middle::ty::layout::{LayoutCx, TyAndLayout};
4+
use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
5+
use rustc_session::{declare_lint, declare_lint_pass};
6+
7+
use crate::{LateContext, LateLintPass, LintContext, lints};
8+
9+
declare_lint! {
10+
/// The `cmse_uninitialized_leak` lint detects values that may be (partially) uninitialized that
11+
/// cross the secure boundary.
12+
///
13+
/// ### Example
14+
///
15+
/// ```rust,ignore (ABI is only supported on thumbv8)
16+
/// extern "cmse-nonsecure-entry" fn foo() -> MaybeUninit<u64> {
17+
/// MaybeUninit::uninit()
18+
/// }
19+
/// ```
20+
///
21+
/// This will produce:
22+
///
23+
/// ```
24+
/// warning: passing a union across the security boundary may leak information
25+
/// --> lint_example.rs:2:5
26+
/// |
27+
/// 2 | MaybeUninit::uninit()
28+
/// | ^^^^^^^^^^^^^^^^^^^^^
29+
/// |
30+
/// = note: the bits not used by the current variant may contain stale secure data
31+
/// = note: `#[warn(cmse_uninitialized_leak)]` on by default
32+
/// ```
33+
///
34+
/// ### Explanation
35+
///
36+
/// The cmse calling conventions normally take care of clearing registers to make sure that
37+
/// stale secure information is not observable from non-secure code. Uninitialized memory may
38+
/// still contain secret information, so the programmer must be careful when (partially)
39+
/// uninitialized values cross the secure boundary. This lint fires when a partially
40+
/// uninitialized value (e.g. a `union` value or a type with a niche) crosses the secure
41+
/// boundary, i.e.:
42+
///
43+
/// - when returned from a `cmse-nonsecure-entry` function
44+
/// - when passed as an argument to a `cmse-nonsecure-call` function
45+
///
46+
/// This lint is a best effort: not all cases of (partially) uninitialized data crossing the
47+
/// secure boundary are caught.
48+
pub CMSE_UNINITIALIZED_LEAK,
49+
Warn,
50+
"(partially) uninitialized value may leak secure information"
51+
}
52+
53+
declare_lint_pass!(CmseUninitializedLeak => [CMSE_UNINITIALIZED_LEAK]);
54+
55+
impl<'tcx> LateLintPass<'tcx> for CmseUninitializedLeak {
56+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
57+
check_cmse_entry_return(cx, expr);
58+
check_cmse_call_call(cx, expr);
59+
}
60+
}
61+
62+
fn check_cmse_call_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
63+
let ExprKind::Call(callee, arguments) = expr.kind else {
64+
return;
65+
};
66+
67+
// Determine the callee ABI.
68+
let callee_ty = cx.typeck_results().expr_ty(callee);
69+
let sig = match callee_ty.kind() {
70+
ty::FnPtr(poly_sig, header) if header.abi == ExternAbi::CmseNonSecureCall => {
71+
poly_sig.skip_binder()
72+
}
73+
_ => return,
74+
};
75+
76+
let fn_sig = cx.tcx.erase_and_anonymize_regions(sig);
77+
let typing_env = ty::TypingEnv::fully_monomorphized();
78+
79+
for (arg, ty) in arguments.iter().zip(fn_sig.inputs()) {
80+
// `impl Trait` is not allowed in the argument types.
81+
if ty.has_opaque_types() {
82+
continue;
83+
}
84+
85+
let Ok(layout) = cx.tcx.layout_of(typing_env.as_query_input(*ty)) else {
86+
continue;
87+
};
88+
89+
if layout_contains_union(cx.tcx, &layout) {
90+
cx.emit_span_lint(
91+
CMSE_UNINITIALIZED_LEAK,
92+
arg.span,
93+
lints::CmseUnionMayLeakInformation,
94+
);
95+
}
96+
}
97+
}
98+
99+
fn check_cmse_entry_return<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
100+
let owner = cx.tcx.hir_enclosing_body_owner(expr.hir_id);
101+
102+
match cx.tcx.def_kind(owner) {
103+
hir::def::DefKind::Fn | hir::def::DefKind::AssocFn => {}
104+
_ => return,
105+
}
106+
107+
// Only continue if the current expr is an (implicit) return.
108+
let body = cx.tcx.hir_body_owned_by(owner);
109+
let is_implicit_return = expr.hir_id == body.value.hir_id;
110+
if !(matches!(expr.kind, ExprKind::Ret(_)) || is_implicit_return) {
111+
return;
112+
}
113+
114+
let sig = cx.tcx.fn_sig(owner).skip_binder();
115+
if sig.abi() != ExternAbi::CmseNonSecureEntry {
116+
return;
117+
}
118+
119+
let fn_sig = cx.tcx.instantiate_bound_regions_with_erased(sig);
120+
let fn_sig = cx.tcx.erase_and_anonymize_regions(fn_sig);
121+
let return_type = fn_sig.output();
122+
123+
// `impl Trait` is not allowed in the return type.
124+
if return_type.has_opaque_types() {
125+
return;
126+
}
127+
128+
let typing_env = ty::TypingEnv::fully_monomorphized();
129+
let Ok(ret_layout) = cx.tcx.layout_of(typing_env.as_query_input(return_type)) else {
130+
return;
131+
};
132+
133+
if layout_contains_union(cx.tcx, &ret_layout) {
134+
let return_expr_span = if is_implicit_return {
135+
match expr.kind {
136+
ExprKind::Block(block, _) => match block.expr {
137+
Some(tail) => tail.span,
138+
None => expr.span,
139+
},
140+
_ => expr.span,
141+
}
142+
} else {
143+
expr.span
144+
};
145+
146+
cx.emit_span_lint(
147+
CMSE_UNINITIALIZED_LEAK,
148+
return_expr_span,
149+
lints::CmseUnionMayLeakInformation,
150+
);
151+
}
152+
}
153+
154+
/// Check whether any part of the layout is a union, which may contain secure data still.
155+
fn layout_contains_union<'tcx>(tcx: TyCtxt<'tcx>, layout: &TyAndLayout<'tcx>) -> bool {
156+
if layout.ty.is_union() {
157+
return true;
158+
}
159+
160+
let typing_env = ty::TypingEnv::fully_monomorphized();
161+
let cx = LayoutCx::new(tcx, typing_env);
162+
163+
match &layout.variants {
164+
rustc_abi::Variants::Single { .. } => {
165+
for i in 0..layout.fields.count() {
166+
if layout_contains_union(tcx, &layout.field(&cx, i)) {
167+
return true;
168+
}
169+
}
170+
}
171+
172+
rustc_abi::Variants::Multiple { variants, .. } => {
173+
for (variant_idx, _vdata) in variants.iter_enumerated() {
174+
let variant_layout = layout.for_variant(&cx, variant_idx);
175+
176+
for i in 0..variant_layout.fields.count() {
177+
if layout_contains_union(tcx, &variant_layout.field(&cx, i)) {
178+
return true;
179+
}
180+
}
181+
}
182+
}
183+
184+
rustc_abi::Variants::Empty => {}
185+
}
186+
187+
false
188+
}

compiler/rustc_lint/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ mod async_closures;
3737
mod async_fn_in_trait;
3838
mod autorefs;
3939
pub mod builtin;
40+
mod cmse_uninitialized_leak;
4041
mod context;
4142
mod dangling;
4243
mod default_could_be_derived;
@@ -86,6 +87,7 @@ use async_closures::AsyncClosureUsage;
8687
use async_fn_in_trait::AsyncFnInTrait;
8788
use autorefs::*;
8889
use builtin::*;
90+
use cmse_uninitialized_leak::*;
8991
use dangling::*;
9092
use default_could_be_derived::DefaultCouldBeDerived;
9193
use deref_into_dyn_supertrait::*;
@@ -246,6 +248,7 @@ late_lint_methods!(
246248
UnqualifiedLocalImports: UnqualifiedLocalImports,
247249
CheckTransmutes: CheckTransmutes,
248250
LifetimeSyntax: LifetimeSyntax,
251+
CmseUninitializedLeak: CmseUninitializedLeak,
249252
]
250253
]
251254
);

compiler/rustc_lint/src/lints.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,6 +1460,12 @@ pub(crate) struct NonLocalDefinitionsCargoUpdateNote {
14601460
pub crate_name: Symbol,
14611461
}
14621462

1463+
// cmse_uninitialized_leak.rs
1464+
#[derive(LintDiagnostic)]
1465+
#[diag(lint_cmse_union_may_leak_information)]
1466+
#[note]
1467+
pub(crate) struct CmseUnionMayLeakInformation;
1468+
14631469
// precedence.rs
14641470
#[derive(LintDiagnostic)]
14651471
#[diag(lint_ambiguous_negative_literals)]
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
//@ add-core-stubs
2+
//@ compile-flags: --target thumbv8m.main-none-eabi --crate-type lib
3+
//@ needs-llvm-components: arm
4+
//@ check-pass
5+
#![feature(abi_cmse_nonsecure_call, no_core, lang_items)]
6+
#![no_core]
7+
#![allow(improper_ctypes_definitions)]
8+
9+
extern crate minicore;
10+
use minicore::*;
11+
12+
#[repr(Rust)]
13+
pub union ReprRustUnionU64 {
14+
_unused: u64,
15+
}
16+
17+
#[repr(C)]
18+
pub union ReprCUnionU64 {
19+
_unused: u64,
20+
_unused1: u32,
21+
}
22+
23+
#[no_mangle]
24+
pub fn test_union(
25+
f1: extern "cmse-nonsecure-call" fn(ReprRustUnionU64),
26+
f2: extern "cmse-nonsecure-call" fn(ReprCUnionU64),
27+
f3: extern "cmse-nonsecure-call" fn(MaybeUninit<u32>),
28+
f4: extern "cmse-nonsecure-call" fn(MaybeUninit<u64>),
29+
) {
30+
f1(ReprRustUnionU64 { _unused: 1 });
31+
//~^ WARN passing a union across the security boundary may leak information
32+
33+
f2(ReprCUnionU64 { _unused: 1 });
34+
//~^ WARN passing a union across the security boundary may leak information
35+
36+
f3(MaybeUninit::uninit());
37+
//~^ WARN passing a union across the security boundary may leak information
38+
39+
f4(MaybeUninit::uninit());
40+
//~^ WARN passing a union across the security boundary may leak information
41+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
warning: passing a union across the security boundary may leak information
2+
--> $DIR/params-uninitialized.rs:30:8
3+
|
4+
LL | f1(ReprRustUnionU64 { _unused: 1 });
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: the bits not used by the current variant may contain stale secure data
8+
= note: `#[warn(cmse_uninitialized_leak)]` on by default
9+
10+
warning: passing a union across the security boundary may leak information
11+
--> $DIR/params-uninitialized.rs:33:8
12+
|
13+
LL | f2(ReprCUnionU64 { _unused: 1 });
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= note: the bits not used by the current variant may contain stale secure data
17+
18+
warning: passing a union across the security boundary may leak information
19+
--> $DIR/params-uninitialized.rs:36:8
20+
|
21+
LL | f3(MaybeUninit::uninit());
22+
| ^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= note: the bits not used by the current variant may contain stale secure data
25+
26+
warning: passing a union across the security boundary may leak information
27+
--> $DIR/params-uninitialized.rs:39:8
28+
|
29+
LL | f4(MaybeUninit::uninit());
30+
| ^^^^^^^^^^^^^^^^^^^^^
31+
|
32+
= note: the bits not used by the current variant may contain stale secure data
33+
34+
warning: 4 warnings emitted
35+

tests/ui/cmse-nonsecure/cmse-nonsecure-call/return-via-stack.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ pub union ReprRustUnionU64 {
4949

5050
#[no_mangle]
5151
pub fn test_union(
52-
f1: extern "cmse-nonsecure-call" fn() -> ReprRustUnionU64, //~ ERROR [E0798]
53-
f2: extern "cmse-nonsecure-call" fn() -> ReprCUnionU64, //~ ERROR [E0798]
52+
f1: extern "cmse-nonsecure-call" fn() -> ReprRustUnionU64,
53+
f2: extern "cmse-nonsecure-call" fn() -> ReprCUnionU64,
54+
//~^ ERROR return value of `"cmse-nonsecure-call"` function too large to pass via registers [E0798]
55+
f3: extern "cmse-nonsecure-call" fn() -> MaybeUninit<u32>,
56+
f4: extern "cmse-nonsecure-call" fn() -> MaybeUninit<u64>,
5457
) {
5558
}

tests/ui/cmse-nonsecure/cmse-nonsecure-call/return-via-stack.stderr

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,6 @@ LL | f5: extern "cmse-nonsecure-call" fn() -> [u8; 5],
6161
= note: functions with the `"cmse-nonsecure-call"` ABI must pass their result via the available return registers
6262
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size
6363

64-
error[E0798]: return value of `"cmse-nonsecure-call"` function too large to pass via registers
65-
--> $DIR/return-via-stack.rs:52:46
66-
|
67-
LL | f1: extern "cmse-nonsecure-call" fn() -> ReprRustUnionU64,
68-
| ^^^^^^^^^^^^^^^^ this type doesn't fit in the available registers
69-
|
70-
= note: functions with the `"cmse-nonsecure-call"` ABI must pass their result via the available return registers
71-
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size
72-
7364
error[E0798]: return value of `"cmse-nonsecure-call"` function too large to pass via registers
7465
--> $DIR/return-via-stack.rs:53:46
7566
|
@@ -79,6 +70,6 @@ LL | f2: extern "cmse-nonsecure-call" fn() -> ReprCUnionU64,
7970
= note: functions with the `"cmse-nonsecure-call"` ABI must pass their result via the available return registers
8071
= note: the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size
8172

82-
error: aborting due to 9 previous errors
73+
error: aborting due to 8 previous errors
8374

8475
For more information about this error, try `rustc --explain E0798`.

tests/ui/cmse-nonsecure/cmse-nonsecure-entry/params-via-stack.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,26 @@ pub extern "cmse-nonsecure-entry" fn f4(_: AlignRelevant, _: u32) {} //~ ERROR [
2323
#[no_mangle]
2424
#[allow(improper_ctypes_definitions)]
2525
pub extern "cmse-nonsecure-entry" fn f5(_: [u32; 5]) {} //~ ERROR [E0798]
26+
//
27+
#[repr(Rust)]
28+
pub union ReprRustUnionU64 {
29+
_unused: u64,
30+
}
31+
32+
#[repr(C)]
33+
pub union ReprCUnionU64 {
34+
_unused: u64,
35+
}
36+
37+
#[no_mangle]
38+
#[allow(improper_ctypes_definitions)]
39+
pub extern "cmse-nonsecure-entry" fn union_rust(_: ReprRustUnionU64) {}
40+
41+
#[no_mangle]
42+
pub extern "cmse-nonsecure-entry" fn union_c(_: ReprCUnionU64) {}
43+
44+
#[no_mangle]
45+
pub extern "cmse-nonsecure-entry" fn maybe_uninit_32bit(_: MaybeUninit<u32>) {}
46+
47+
#[no_mangle]
48+
pub extern "cmse-nonsecure-entry" fn maybe_uninit_64bit(_: MaybeUninit<f64>) {}

0 commit comments

Comments
 (0)