cmse: lint on unions crossing the secure boundary

compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs

@@ -1,4 +1,4 @@
-use rustc_abi::{BackendRepr, ExternAbi, Float, Integer, Primitive, Scalar};
+use rustc_abi::{BackendRepr, ExternAbi, Float, Integer, Primitive};
 use rustc_errors::{DiagCtxtHandle, E0781, struct_span_code_err};
 use rustc_hir::{self as hir, HirId};
 use rustc_middle::bug;
@@ -163,11 +163,7 @@ fn is_valid_cmse_output_layout<'tcx>(layout: TyAndLayout<'tcx>) -> bool {
  return false;
  };
 
- let Scalar::Initialized { value, .. } = scalar else {
- return false;
- };
-
- matches!(value, Primitive::Int(Integer::I64, _) | Primitive::Float(Float::F64))
+ matches!(scalar.primitive(), Primitive::Int(Integer::I64, _) | Primitive::Float(Float::F64))
 }
 
 fn should_emit_layout_error<'tcx>(abi: ExternAbi, layout_err: &'tcx LayoutError<'tcx>) -> bool {

compiler/rustc_lint/messages.ftl

@@ -187,6 +187,10 @@ lint_closure_returning_async_block = closure returning async block can be made i
  .label = this async block can be removed, and the closure can be turned into an async closure
  .suggestion = turn this into an async closure
 
+lint_cmse_union_may_leak_information =
+ passing a union across the security boundary may leak information
+ .note = the bits not used by the current variant may contain stale secure data
+
 lint_command_line_source = `forbid` lint level was set on command line
 
 lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as identifiers, which look alike

compiler/rustc_lint/src/cmse_uninitialized_leak.rs

@@ -0,0 +1,188 @@
+use rustc_abi::ExternAbi;
+use rustc_hir::{self as hir, Expr, ExprKind};
+use rustc_middle::ty::layout::{LayoutCx, TyAndLayout};
+use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
+use rustc_session::{declare_lint, declare_lint_pass};
+
+use crate::{LateContext, LateLintPass, LintContext, lints};
+
+declare_lint! {
+ /// The `cmse_uninitialized_leak` lint detects values that may be (partially) uninitialized that
+ /// cross the secure boundary.
+ ///
+ /// ### Example
+ ///
+ /// ```rust,ignore (ABI is only supported on thumbv8)
+ /// extern "cmse-nonsecure-entry" fn foo() -> MaybeUninit<u64> {
+ /// MaybeUninit::uninit()
+ /// }
+ /// ```
+ ///
+ /// This will produce:
+ ///
+ /// ```text
+ /// warning: passing a union across the security boundary may leak information
+ /// --> lint_example.rs:2:5
+ /// |
+ /// 2 | MaybeUninit::uninit()
+ /// | ^^^^^^^^^^^^^^^^^^^^^
+ /// |
+ /// = note: the bits not used by the current variant may contain stale secure data
+ /// = note: `#[warn(cmse_uninitialized_leak)]` on by default
+ /// ```
+ ///
+ /// ### Explanation
+ ///
+ /// The cmse calling conventions normally take care of clearing registers to make sure that
+ /// stale secure information is not observable from non-secure code. Uninitialized memory may
+ /// still contain secret information, so the programmer must be careful when (partially)
+ /// uninitialized values cross the secure boundary. This lint fires when a partially
+ /// uninitialized value (e.g. a `union` value or a type with a niche) crosses the secure
+ /// boundary, i.e.:
+ ///
+ /// - when returned from a `cmse-nonsecure-entry` function
+ /// - when passed as an argument to a `cmse-nonsecure-call` function
+ ///
+ /// This lint is a best effort: not all cases of (partially) uninitialized data crossing the
+ /// secure boundary are caught.
+ pub CMSE_UNINITIALIZED_LEAK,
+ Warn,
+ "(partially) uninitialized value may leak secure information"
+}
+
+declare_lint_pass!(CmseUninitializedLeak => [CMSE_UNINITIALIZED_LEAK]);
+
+impl<'tcx> LateLintPass<'tcx> for CmseUninitializedLeak {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+ check_cmse_entry_return(cx, expr);
+ check_cmse_call_call(cx, expr);
+ }
+}
+
+fn check_cmse_call_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+ let ExprKind::Call(callee, arguments) = expr.kind else {
+ return;
+ };
+
+ // Determine the callee ABI.
+ let callee_ty = cx.typeck_results().expr_ty(callee);
+ let sig = match callee_ty.kind() {
+ ty::FnPtr(poly_sig, header) if header.abi == ExternAbi::CmseNonSecureCall => {
+ poly_sig.skip_binder()
+ }
+ _ => return,
+ };
+
+ let fn_sig = cx.tcx.erase_and_anonymize_regions(sig);
+ let typing_env = ty::TypingEnv::fully_monomorphized();
+
+ for (arg, ty) in arguments.iter().zip(fn_sig.inputs()) {
+ // `impl Trait` is not allowed in the argument types.
+ if ty.has_opaque_types() {
+ continue;
+ }
+
+ let Ok(layout) = cx.tcx.layout_of(typing_env.as_query_input(*ty)) else {
+ continue;
+ };
+
+ if layout_contains_union(cx.tcx, &layout) {
+ cx.emit_span_lint(
+ CMSE_UNINITIALIZED_LEAK,
+ arg.span,
+ lints::CmseUnionMayLeakInformation,
+ );
+ }
+ }
+}
+
+fn check_cmse_entry_return<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+ let owner = cx.tcx.hir_enclosing_body_owner(expr.hir_id);
+
+ match cx.tcx.def_kind(owner) {
+ hir::def::DefKind::Fn | hir::def::DefKind::AssocFn => {}
+ _ => return,
+ }
+
+ // Only continue if the current expr is an (implicit) return.
+ let body = cx.tcx.hir_body_owned_by(owner);
+ let is_implicit_return = expr.hir_id == body.value.hir_id;
+ if !(matches!(expr.kind, ExprKind::Ret(_)) || is_implicit_return) {
+ return;
+ }
+
+ let sig = cx.tcx.fn_sig(owner).skip_binder();
+ if sig.abi() != ExternAbi::CmseNonSecureEntry {
+ return;
+ }
+
+ let fn_sig = cx.tcx.instantiate_bound_regions_with_erased(sig);
+ let fn_sig = cx.tcx.erase_and_anonymize_regions(fn_sig);
+ let return_type = fn_sig.output();
+
+ // `impl Trait` is not allowed in the return type.
+ if return_type.has_opaque_types() {
+ return;
+ }
+
+ let typing_env = ty::TypingEnv::fully_monomorphized();
+ let Ok(ret_layout) = cx.tcx.layout_of(typing_env.as_query_input(return_type)) else {
+ return;
+ };
+
+ if layout_contains_union(cx.tcx, &ret_layout) {
+ let return_expr_span = if is_implicit_return {
+ match expr.kind {
+ ExprKind::Block(block, _) => match block.expr {
+ Some(tail) => tail.span,
+ None => expr.span,
+ },
+ _ => expr.span,
+ }
+ } else {
+ expr.span
+ };
+
+ cx.emit_span_lint(
+ CMSE_UNINITIALIZED_LEAK,
+ return_expr_span,
+ lints::CmseUnionMayLeakInformation,
+ );
+ }
+}
+
+/// Check whether any part of the layout is a union, which may contain secure data still.
+fn layout_contains_union<'tcx>(tcx: TyCtxt<'tcx>, layout: &TyAndLayout<'tcx>) -> bool {
+ if layout.ty.is_union() {
+ return true;
+ }
+
+ let typing_env = ty::TypingEnv::fully_monomorphized();
+ let cx = LayoutCx::new(tcx, typing_env);
+
+ match &layout.variants {
+ rustc_abi::Variants::Single { .. } => {
+ for i in 0..layout.fields.count() {
+ if layout_contains_union(tcx, &layout.field(&cx, i)) {
+ return true;
+ }
+ }
+ }
+
+ rustc_abi::Variants::Multiple { variants, .. } => {
+ for (variant_idx, _vdata) in variants.iter_enumerated() {
+ let variant_layout = layout.for_variant(&cx, variant_idx);
+
+ for i in 0..variant_layout.fields.count() {
+ if layout_contains_union(tcx, &variant_layout.field(&cx, i)) {
+ return true;
+ }
+ }
+ }
+ }
+
+ rustc_abi::Variants::Empty => {}
+ }
+
+ false
+}

compiler/rustc_lint/src/lib.rs

@@ -37,6 +37,7 @@ mod async_closures;
 mod async_fn_in_trait;
 mod autorefs;
 pub mod builtin;
+mod cmse_uninitialized_leak;
 mod context;
 mod dangling;
 mod default_could_be_derived;
@@ -86,6 +87,7 @@ use async_closures::AsyncClosureUsage;
 use async_fn_in_trait::AsyncFnInTrait;
 use autorefs::*;
 use builtin::*;
+use cmse_uninitialized_leak::*;
 use dangling::*;
 use default_could_be_derived::DefaultCouldBeDerived;
 use deref_into_dyn_supertrait::*;
@@ -246,6 +248,7 @@ late_lint_methods!(
  UnqualifiedLocalImports: UnqualifiedLocalImports,
  CheckTransmutes: CheckTransmutes,
  LifetimeSyntax: LifetimeSyntax,
+ CmseUninitializedLeak: CmseUninitializedLeak,
  ]
  ]
 );

compiler/rustc_lint/src/lints.rs

@@ -1460,6 +1460,12 @@ pub(crate) struct NonLocalDefinitionsCargoUpdateNote {
  pub crate_name: Symbol,
 }
 
+// cmse_uninitialized_leak.rs
+#[derive(LintDiagnostic)]
+#[diag(lint_cmse_union_may_leak_information)]
+#[note]
+pub(crate) struct CmseUnionMayLeakInformation;
+
 // precedence.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_ambiguous_negative_literals)]

tests/auxiliary/minicore.rs

@@ -26,6 +26,7 @@
  decl_macro,
  f16,
  f128,
+ transparent_unions,
  asm_experimental_arch,
  unboxed_closures
 )]
@@ -119,6 +120,25 @@ pub struct ManuallyDrop<T: PointeeSized> {
 }
 impl<T: Copy + PointeeSized> Copy for ManuallyDrop<T> {}
 
+#[lang = "maybe_uninit"]
+#[repr(transparent)]
+pub union MaybeUninit<T> {
+ uninit: (),
+ value: ManuallyDrop<T>,
+}
+
+impl<T: Copy + PointeeSized> Copy for MaybeUninit<T> {}
+
+impl<T> MaybeUninit<T> {
+ pub const fn uninit() -> Self {
+ Self { uninit: () }
+ }
+
+ pub const fn new(value: T) -> Self {
+ Self { value: ManuallyDrop { value } }
+ }
+}
+
 #[lang = "unsafe_cell"]
 #[repr(transparent)]
 pub struct UnsafeCell<T: PointeeSized> {

tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-uninitialized.rs

@@ -0,0 +1,55 @@
+//@ add-core-stubs
+//@ compile-flags: --target thumbv8m.main-none-eabi --crate-type lib
+//@ needs-llvm-components: arm
+//@ check-pass
+#![feature(abi_cmse_nonsecure_call, no_core, lang_items)]
+#![no_core]
+#![allow(improper_ctypes_definitions)]
+
+extern crate minicore;
+use minicore::*;
+
+#[repr(Rust)]
+pub union ReprRustUnionU64 {
+ _unused: u64,
+}
+
+#[repr(C)]
+pub union ReprCUnionU64 {
+ _unused: u64,
+ _unused1: u32,
+}
+
+#[repr(C)]
+pub struct ReprCAggregate {
+ a: usize,
+ b: ReprCUnionU64,
+}
+
+#[no_mangle]
+pub fn test_union(
+ f1: extern "cmse-nonsecure-call" fn(ReprRustUnionU64),
+ f2: extern "cmse-nonsecure-call" fn(ReprCUnionU64),
+ f3: extern "cmse-nonsecure-call" fn(MaybeUninit<u32>),
+ f4: extern "cmse-nonsecure-call" fn(MaybeUninit<u64>),
+ f5: extern "cmse-nonsecure-call" fn((usize, MaybeUninit<u64>)),
+ f6: extern "cmse-nonsecure-call" fn(ReprCAggregate),
+) {
+ f1(ReprRustUnionU64 { _unused: 1 });
+ //~^ WARN passing a union across the security boundary may leak information
+
+ f2(ReprCUnionU64 { _unused: 1 });
+ //~^ WARN passing a union across the security boundary may leak information
+
+ f3(MaybeUninit::uninit());
+ //~^ WARN passing a union across the security boundary may leak information
+
+ f4(MaybeUninit::uninit());
+ //~^ WARN passing a union across the security boundary may leak information
+
+ f5((0, MaybeUninit::uninit()));
+ //~^ WARN passing a union across the security boundary may leak information
+
+ f6(ReprCAggregate { a: 0, b: ReprCUnionU64 { _unused: 1 } });
+ //~^ WARN passing a union across the security boundary may leak information
+}

tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-uninitialized.stderr

@@ -0,0 +1,51 @@
+warning: passing a union across the security boundary may leak information
+ --> $DIR/params-uninitialized.rs:38:8
+ |
+LL | f1(ReprRustUnionU64 { _unused: 1 });
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: the bits not used by the current variant may contain stale secure data
+ = note: `#[warn(cmse_uninitialized_leak)]` on by default
+
+warning: passing a union across the security boundary may leak information
+ --> $DIR/params-uninitialized.rs:41:8
+ |
+LL | f2(ReprCUnionU64 { _unused: 1 });
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: the bits not used by the current variant may contain stale secure data
+
+warning: passing a union across the security boundary may leak information
+ --> $DIR/params-uninitialized.rs:44:8
+ |
+LL | f3(MaybeUninit::uninit());
+ | ^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: the bits not used by the current variant may contain stale secure data
+
+warning: passing a union across the security boundary may leak information
+ --> $DIR/params-uninitialized.rs:47:8
+ |
+LL | f4(MaybeUninit::uninit());
+ | ^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: the bits not used by the current variant may contain stale secure data
+
+warning: passing a union across the security boundary may leak information
+ --> $DIR/params-uninitialized.rs:50:8
+ |
+LL | f5((0, MaybeUninit::uninit()));
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: the bits not used by the current variant may contain stale secure data
+
+warning: passing a union across the security boundary may leak information
+ --> $DIR/params-uninitialized.rs:53:8
+ |
+LL | f6(ReprCAggregate { a: 0, b: ReprCUnionU64 { _unused: 1 } });
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: the bits not used by the current variant may contain stale secure data
+
+warning: 6 warnings emitted
+