cmse: lint on unions crossing the secure boundary

compiler/rustc_lint/src/cmse_uninitialized_leak.rs

@@ -0,0 +1,161 @@
+use rustc_abi::ExternAbi;
+use rustc_hir::{self as hir, Expr, ExprKind};
+use rustc_middle::ty::{self, Ty, 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);
+
+    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;
+        }
+
+        if contains_union_or_enum(cx.tcx, *ty) {
+            // Some part of the source type may be uninitialized.
+            cx.emit_span_lint(
+                CMSE_UNINITIALIZED_LEAK,
+                arg.span,
+                lints::CmseUninitializedMayLeakInformation,
+            );
+        }
+    }
+}
+
+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;
+    }
+
+    if contains_union_or_enum(cx.tcx, return_type) {
+        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
+        };
+
+        // Some part of the source type may be uninitialized.
+        cx.emit_span_lint(
+            CMSE_UNINITIALIZED_LEAK,
+            return_expr_span,
+            lints::CmseUninitializedMayLeakInformation,
+        );
+    }
+}
+
+/// Traverse `T` for any `union` or `enum`.
+fn contains_union_or_enum<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
+    match ty.kind() {
+        ty::Adt(adt_def, args) => {
+            if adt_def.is_union() || adt_def.is_enum() {
+                return true;
+            }
+
+            adt_def
+                .all_fields()
+                .any(|field| contains_union_or_enum(tcx, field.ty(tcx, args).skip_norm_wip()))
+        }
+        ty::Tuple(tys) => tys.iter().any(|ty| contains_union_or_enum(tcx, ty)),
+        ty::Array(ty, _) => contains_union_or_enum(tcx, *ty),
+        _ => false,
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -32,6 +32,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::*;
@@ -268,6 +270,7 @@ late_lint_methods!(
             CheckTransmutes: CheckTransmutes,
             LifetimeSyntax: LifetimeSyntax,
             InternalEqTraitMethodImpls: InternalEqTraitMethodImpls,
+            CmseUninitializedLeak: CmseUninitializedLeak,
             ImplicitProvenanceCasts: ImplicitProvenanceCasts,
         ]
     ]

compiler/rustc_lint/src/lints.rs

@@ -1799,6 +1799,14 @@ pub(crate) struct NonLocalDefinitionsCargoUpdateNote {
     pub crate_name: Symbol,
 }
 
+// cmse_uninitialized_leak.rs
+#[derive(Diagnostic)]
+#[diag(
+    "this value crossing a secure boundary may contain (partially) uninitialized data which can leak information"
+)]
+#[note("enum and unions have variant-specific padding that may contain stale secure data")]
+pub(crate) struct CmseUninitializedMayLeakInformation;
+
 // precedence.rs
 #[derive(Diagnostic)]
 #[diag("`-` has lower precedence than method calls, which might be unexpected")]

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

@@ -0,0 +1,109 @@
+//@ add-minicore
+//@ compile-flags: --target thumbv8m.main-none-eabi --crate-type lib
+//@ needs-llvm-components: arm
+//@ check-pass
+//@ ignore-backends: gcc
+#![feature(abi_cmse_nonsecure_call, no_core, lang_items)]
+#![no_core]
+#![allow(improper_ctypes_definitions)]
+
+extern crate minicore;
+use minicore::*;
+
+#[repr(Rust)]
+union ReprRustUnionU64 {
+    _unused: u64,
+}
+
+#[repr(Rust)]
+union ReprRustUnionPartiallyUninit {
+    _unused1: u32,
+    _unused2: u16,
+}
+
+#[repr(C)]
+union ReprCUnionU64 {
+    _unused: u64,
+    _unused1: u32,
+}
+
+#[repr(C)]
+struct ReprCAggregate {
+    a: usize,
+    b: ReprCUnionU64,
+}
+
+// This is an aggregate that cannot be unwrapped, and has 1 (uninitialized) padding byte.
+#[repr(C, align(4))]
+struct PaddedStruct {
+    a: u8,
+    b: u16,
+}
+
+#[repr(C)]
+enum VariantsSameSize {
+    A(u16),
+    B(u16),
+}
+
+#[repr(C)]
+enum VariantsDifferentSize {
+    A(u8),
+    B(u16),
+}
+
+enum Void {}
+
+#[repr(C)]
+enum UninhabitedVariant {
+    A(Void),
+    B(u16),
+}
+
+#[no_mangle]
+fn test_uninitialized(
+    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),
+    f7: extern "cmse-nonsecure-call" fn(ReprRustUnionPartiallyUninit),
+    f8: extern "cmse-nonsecure-call" fn(PaddedStruct),
+    f9: extern "cmse-nonsecure-call" fn(VariantsSameSize),
+    f10: extern "cmse-nonsecure-call" fn(VariantsDifferentSize),
+    f11: extern "cmse-nonsecure-call" fn(UninhabitedVariant),
+) {
+    f1(ReprRustUnionU64 { _unused: 1 });
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f2(ReprCUnionU64 { _unused: 1 });
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f3(MaybeUninit::uninit());
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f4(MaybeUninit::uninit());
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f5((0, MaybeUninit::uninit()));
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f6(ReprCAggregate { a: 0, b: ReprCUnionU64 { _unused: 1 } });
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f7(ReprRustUnionPartiallyUninit { _unused1: 0 });
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    // This struct only has no value-dependent padding, the guaranteed padding is zeroed.
+    f8(PaddedStruct { a: 0, b: 0 });
+
+    f9(VariantsSameSize::A(0));
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f10(VariantsDifferentSize::A(0));
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+
+    f11(UninhabitedVariant::B(0));
+    //~^ WARN this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+}

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

@@ -0,0 +1,83 @@
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:77:8
+   |
+LL |     f1(ReprRustUnionU64 { _unused: 1 });
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+   = note: `#[warn(cmse_uninitialized_leak)]` on by default
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:80:8
+   |
+LL |     f2(ReprCUnionU64 { _unused: 1 });
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:83:8
+   |
+LL |     f3(MaybeUninit::uninit());
+   |        ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:86:8
+   |
+LL |     f4(MaybeUninit::uninit());
+   |        ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:89:8
+   |
+LL |     f5((0, MaybeUninit::uninit()));
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:92:8
+   |
+LL |     f6(ReprCAggregate { a: 0, b: ReprCUnionU64 { _unused: 1 } });
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:95:8
+   |
+LL |     f7(ReprRustUnionPartiallyUninit { _unused1: 0 });
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:101:8
+   |
+LL |     f9(VariantsSameSize::A(0));
+   |        ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:104:9
+   |
+LL |     f10(VariantsDifferentSize::A(0));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: this value crossing a secure boundary may contain (partially) uninitialized data which can leak information
+  --> $DIR/params-uninitialized.rs:107:9
+   |
+LL |     f11(UninhabitedVariant::B(0));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: enum and unions have variant-specific padding that may contain stale secure data
+
+warning: 10 warnings emitted
+