Skip to content

Commit 9d276b5

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 9d276b5

File tree

14 files changed

+377
-60
lines changed

14 files changed

+377
-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: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
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+
/// {{produces}}
22+
///
23+
/// ### Explanation
24+
///
25+
/// The cmse calling conventions normally take care of clearing registers to make sure that
26+
/// stale secure information is not observable from non-secure code. Uninitialized memory may
27+
/// still contain secret information, so the programmer must be careful when (partially)
28+
/// uninitialized values cross the secure boundary. This lint fires when a partially
29+
/// uninitialized value (e.g. a `union` value or a type with a niche) crosses the secure
30+
/// boundary, i.e.:
31+
///
32+
/// - when returned from a `cmse-nonsecure-entry` function
33+
/// - when passed as an argument to a `cmse-nonsecure-call` function
34+
///
35+
/// This lint is a best effort: not all cases of (partially) uninitialized data crossing the
36+
/// secure boundary are caught.
37+
pub CMSE_UNINITIALIZED_LEAK,
38+
Warn,
39+
"(partially) uninitialized value may leak secure information"
40+
}
41+
42+
declare_lint_pass!(CmseUninitializedLeak => [CMSE_UNINITIALIZED_LEAK]);
43+
44+
impl<'tcx> LateLintPass<'tcx> for CmseUninitializedLeak {
45+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
46+
check_cmse_entry_return(cx, expr);
47+
check_cmse_call_call(cx, expr);
48+
}
49+
}
50+
51+
fn check_cmse_call_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
52+
let ExprKind::Call(callee, arguments) = expr.kind else {
53+
return;
54+
};
55+
56+
// Determine the callee ABI.
57+
let callee_ty = cx.typeck_results().expr_ty(callee);
58+
let sig = match callee_ty.kind() {
59+
ty::FnPtr(poly_sig, header) if header.abi == ExternAbi::CmseNonSecureCall => {
60+
poly_sig.skip_binder()
61+
}
62+
_ => return,
63+
};
64+
65+
let fn_sig = cx.tcx.erase_and_anonymize_regions(sig);
66+
let typing_env = ty::TypingEnv::fully_monomorphized();
67+
68+
for (arg, ty) in arguments.iter().zip(fn_sig.inputs()) {
69+
// `impl Trait` is not allowed in the argument types.
70+
if ty.has_opaque_types() {
71+
continue;
72+
}
73+
74+
let Ok(layout) = cx.tcx.layout_of(typing_env.as_query_input(*ty)) else {
75+
continue;
76+
};
77+
78+
if layout_contains_union(cx.tcx, &layout) {
79+
cx.emit_span_lint(
80+
CMSE_UNINITIALIZED_LEAK,
81+
arg.span,
82+
lints::CmseUnionMayLeakInformation,
83+
);
84+
}
85+
}
86+
}
87+
88+
fn check_cmse_entry_return<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
89+
let owner = cx.tcx.hir_enclosing_body_owner(expr.hir_id);
90+
91+
match cx.tcx.def_kind(owner) {
92+
hir::def::DefKind::Fn | hir::def::DefKind::AssocFn => {}
93+
_ => return,
94+
}
95+
96+
// Only continue if the current expr is an (implicit) return.
97+
let body = cx.tcx.hir_body_owned_by(owner);
98+
let is_implicit_return = expr.hir_id == body.value.hir_id;
99+
if !(matches!(expr.kind, ExprKind::Ret(_)) || is_implicit_return) {
100+
return;
101+
}
102+
103+
let sig = cx.tcx.fn_sig(owner).skip_binder();
104+
if sig.abi() != ExternAbi::CmseNonSecureEntry {
105+
return;
106+
}
107+
108+
let fn_sig = cx.tcx.instantiate_bound_regions_with_erased(sig);
109+
let fn_sig = cx.tcx.erase_and_anonymize_regions(fn_sig);
110+
let return_type = fn_sig.output();
111+
112+
// `impl Trait` is not allowed in the return type.
113+
if return_type.has_opaque_types() {
114+
return;
115+
}
116+
117+
let typing_env = ty::TypingEnv::fully_monomorphized();
118+
let Ok(ret_layout) = cx.tcx.layout_of(typing_env.as_query_input(return_type)) else {
119+
return;
120+
};
121+
122+
if layout_contains_union(cx.tcx, &ret_layout) {
123+
let return_expr_span = if is_implicit_return {
124+
match expr.kind {
125+
ExprKind::Block(block, _) => match block.expr {
126+
Some(tail) => tail.span,
127+
None => expr.span,
128+
},
129+
_ => expr.span,
130+
}
131+
} else {
132+
expr.span
133+
};
134+
135+
cx.emit_span_lint(
136+
CMSE_UNINITIALIZED_LEAK,
137+
return_expr_span,
138+
lints::CmseUnionMayLeakInformation,
139+
);
140+
}
141+
}
142+
143+
/// Check whether any part of the layout is a union, which may contain secure data still.
144+
fn layout_contains_union<'tcx>(tcx: TyCtxt<'tcx>, layout: &TyAndLayout<'tcx>) -> bool {
145+
if layout.ty.is_union() {
146+
return true;
147+
}
148+
149+
let typing_env = ty::TypingEnv::fully_monomorphized();
150+
let cx = LayoutCx::new(tcx, typing_env);
151+
152+
match &layout.variants {
153+
rustc_abi::Variants::Single { .. } => {
154+
for i in 0..layout.fields.count() {
155+
if layout_contains_union(tcx, &layout.field(&cx, i)) {
156+
return true;
157+
}
158+
}
159+
}
160+
161+
rustc_abi::Variants::Multiple { variants, .. } => {
162+
for (variant_idx, _vdata) in variants.iter_enumerated() {
163+
let variant_layout = layout.for_variant(&cx, variant_idx);
164+
165+
for i in 0..variant_layout.fields.count() {
166+
if layout_contains_union(tcx, &variant_layout.field(&cx, i)) {
167+
return true;
168+
}
169+
}
170+
}
171+
}
172+
173+
rustc_abi::Variants::Empty => {}
174+
}
175+
176+
false
177+
}

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)