cmse: lint when a (partially) uninitialized value crosses the secure boundary

Author: folkertdev

Cargo.lock

@@ -4100,6 +4100,7 @@ dependencies = [
  "rustc_span",
  "rustc_target",
  "rustc_trait_selection",
+ "rustc_transmute",
  "smallvec",
  "tracing",
  "unicode-security",

compiler/rustc_lint/Cargo.toml

@@ -24,6 +24,7 @@ rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 rustc_target = { path = "../rustc_target" }
 rustc_trait_selection = { path = "../rustc_trait_selection" }
+rustc_transmute = { path = "../rustc_transmute" }
 smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
 tracing = "0.1"
 unicode-security = "0.1.0"

compiler/rustc_lint/messages.ftl

@@ -187,9 +187,9 @@ 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_cmse_uninitialized_may_leak_information =
+    passing a (partially) uninitialized value across the security boundary may leak information
+    .note = padding or fields not used by the current variant of a union may contain stale secure data
 
 lint_command_line_source = `forbid` lint level was set on command line
 

compiler/rustc_lint/src/cmse_uninitialized_leak.rs

@@ -1,7 +1,7 @@
 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_middle::ty::layout::TyAndLayout;
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
 use rustc_session::{declare_lint, declare_lint_pass};
 
 use crate::{LateContext, LateLintPass, LintContext, lints};
@@ -86,11 +86,12 @@ fn check_cmse_call_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
             continue;
         };
 
-        if layout_contains_union(cx.tcx, &layout) {
+        if !is_transmutable_to_array_u8(cx.tcx, ty.clone(), layout) {
+            // Some part of the source type may be uninitialized.
             cx.emit_span_lint(
                 CMSE_UNINITIALIZED_LEAK,
                 arg.span,
-                lints::CmseUnionMayLeakInformation,
+                lints::CmseUninitializedMayLeakInformation,
             );
         }
     }
@@ -126,11 +127,12 @@ fn check_cmse_entry_return<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
     }
 
     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) {
+    if !is_transmutable_to_array_u8(cx.tcx, return_type.clone(), ret_layout) {
         let return_expr_span = if is_implicit_return {
             match expr.kind {
                 ExprKind::Block(block, _) => match block.expr {
@@ -143,46 +145,39 @@ fn check_cmse_entry_return<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
             expr.span
         };
 
+        // Some part of the source type may be uninitialized.
         cx.emit_span_lint(
             CMSE_UNINITIALIZED_LEAK,
             return_expr_span,
-            lints::CmseUnionMayLeakInformation,
+            lints::CmseUninitializedMayLeakInformation,
         );
     }
 }
 
-/// 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 => {}
+/// Check whether the source type `T` can be safely transmuted to `[u8; size_of::<T>()]`.
+///
+/// If the transmute is valid, `T` must be fully initialized. Otherwise, parts of it may be
+/// uninitialized, which should trigger the lint defined by this module.
+fn is_transmutable_to_array_u8<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    ty: Ty<'tcx>,
+    layout: TyAndLayout<'tcx>,
+) -> bool {
+    use rustc_transmute::{Answer, Assume, TransmuteTypeEnv, Types};
+
+    let mut transmute_env = TransmuteTypeEnv::new(tcx);
+    let assume = Assume { alignment: true, lifetimes: true, safety: false, validity: false };
+
+    let array_u8_ty = Ty::new_array_with_const_len(
+        tcx,
+        tcx.types.u8,
+        ty::Const::from_target_usize(tcx, layout.size.bytes()),
+    );
+
+    let types = Types { src: ty, dst: array_u8_ty };
+
+    match transmute_env.is_transmutable(types, assume) {
+        Answer::Yes => true,
+        Answer::No(_) | Answer::If(_) => false,
     }
-
-    false
 }

compiler/rustc_lint/src/lints.rs

@@ -1469,9 +1469,9 @@ pub(crate) struct NonLocalDefinitionsCargoUpdateNote {
 
 // cmse_uninitialized_leak.rs
 #[derive(LintDiagnostic)]
-#[diag(lint_cmse_union_may_leak_information)]
+#[diag(lint_cmse_uninitialized_may_leak_information)]
 #[note]
-pub(crate) struct CmseUnionMayLeakInformation;
+pub(crate) struct CmseUninitializedMayLeakInformation;
 
 // precedence.rs
 #[derive(LintDiagnostic)]

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

@@ -10,46 +10,67 @@ extern crate minicore;
 use minicore::*;
 
 #[repr(Rust)]
-pub union ReprRustUnionU64 {
+union ReprRustUnionU64 {
     _unused: u64,
 }
 
+#[repr(Rust)]
+union ReprRustUnionPartiallyUninit {
+    _unused1: u32,
+    _unused2: u16,
+}
+
 #[repr(C)]
-pub union ReprCUnionU64 {
+union ReprCUnionU64 {
     _unused: u64,
     _unused1: u32,
 }
 
 #[repr(C)]
-pub struct ReprCAggregate {
+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,
+}
+
 #[no_mangle]
-pub fn test_union(
+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),
 ) {
+    // With `repr(Rust)` this union is always initialized.
     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
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 
     f3(MaybeUninit::uninit());
-    //~^ WARN passing a union across the security boundary may leak information
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 
     f4(MaybeUninit::uninit());
-    //~^ WARN passing a union across the security boundary may leak information
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 
     f5((0, MaybeUninit::uninit()));
-    //~^ WARN passing a union across the security boundary may leak information
+    //~^ WARN passing a (partially) uninitialized value 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
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
+
+    f7(ReprRustUnionPartiallyUninit { _unused1: 0 });
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
+
+    f8(PaddedStruct { a: 0, b: 0 });
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 }

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

@@ -1,51 +1,59 @@
-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
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:56:8
    |
 LL |     f2(ReprCUnionU64 { _unused: 1 });
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: the bits not used by the current variant may contain stale secure data
+   = note: padding or fields not used by the current variant of a union 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:44:8
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:59:8
    |
 LL |     f3(MaybeUninit::uninit());
    |        ^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: the bits not used by the current variant may contain stale secure data
+   = note: padding or fields not used by the current variant of a union may contain stale secure data
 
-warning: passing a union across the security boundary may leak information
-  --> $DIR/params-uninitialized.rs:47:8
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:62:8
    |
 LL |     f4(MaybeUninit::uninit());
    |        ^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: the bits not used by the current variant may contain stale secure data
+   = note: padding or fields not used by the current variant of a union may contain stale secure data
 
-warning: passing a union across the security boundary may leak information
-  --> $DIR/params-uninitialized.rs:50:8
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:65:8
    |
 LL |     f5((0, MaybeUninit::uninit()));
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: the bits not used by the current variant may contain stale secure data
+   = note: padding or fields not used by the current variant of a union may contain stale secure data
 
-warning: passing a union across the security boundary may leak information
-  --> $DIR/params-uninitialized.rs:53:8
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:68:8
    |
 LL |     f6(ReprCAggregate { a: 0, b: ReprCUnionU64 { _unused: 1 } });
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: the bits not used by the current variant may contain stale secure data
+   = note: padding or fields not used by the current variant of a union may contain stale secure data
+
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:71:8
+   |
+LL |     f7(ReprRustUnionPartiallyUninit { _unused1: 0 });
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: padding or fields not used by the current variant of a union may contain stale secure data
+
+warning: passing a (partially) uninitialized value across the security boundary may leak information
+  --> $DIR/params-uninitialized.rs:74:8
+   |
+LL |     f8(PaddedStruct { a: 0, b: 0 });
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: padding or fields not used by the current variant of a union may contain stale secure data
 
-warning: 6 warnings emitted
+warning: 7 warnings emitted
 

tests/ui/cmse-nonsecure/cmse-nonsecure-entry/return-uninitialized.rs

@@ -10,42 +10,67 @@ extern crate minicore;
 use minicore::*;
 
 #[repr(Rust)]
-pub union ReprRustUnionU64 {
+union ReprRustUnionU64 {
     _unused: u64,
 }
 
 #[no_mangle]
 #[allow(improper_ctypes_definitions)]
-pub extern "cmse-nonsecure-entry" fn union_rust() -> ReprRustUnionU64 {
+extern "cmse-nonsecure-entry" fn union_rust() -> ReprRustUnionU64 {
+    // With `repr(Rust)` value is always fully initialized.
     ReprRustUnionU64 { _unused: 1 }
-    //~^ WARN passing a union across the security boundary may leak information
+}
+
+#[repr(Rust)]
+union ReprRustUnionPartiallyUninit {
+    _unused1: u32,
+    _unused2: u16,
+}
+
+#[no_mangle]
+#[allow(improper_ctypes_definitions)]
+extern "cmse-nonsecure-entry" fn union_rust_partially_uninit() -> ReprRustUnionPartiallyUninit {
+    ReprRustUnionPartiallyUninit { _unused1: 1 }
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 }
 
 #[no_mangle]
-pub extern "cmse-nonsecure-entry" fn maybe_uninit_32bit() -> MaybeUninit<u32> {
+extern "cmse-nonsecure-entry" fn maybe_uninit_32bit() -> MaybeUninit<u32> {
     MaybeUninit::uninit()
-    //~^ WARN passing a union across the security boundary may leak information
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 }
 
 #[no_mangle]
-pub extern "cmse-nonsecure-entry" fn maybe_uninit_64bit() -> MaybeUninit<f64> {
+extern "cmse-nonsecure-entry" fn maybe_uninit_64bit() -> MaybeUninit<f64> {
     if true {
         return MaybeUninit::new(6.28);
-        //~^ WARN passing a union across the security boundary may leak information
+        //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
     }
     MaybeUninit::new(3.14)
-    //~^ WARN passing a union across the security boundary may leak information
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
 }
 
 #[repr(transparent)]
-pub struct Wrapper(ReprRustUnionU64);
+struct Wrapper(MaybeUninit<u64>);
 
 #[no_mangle]
-pub extern "cmse-nonsecure-entry" fn repr_transparent_union() -> Wrapper {
-    //~^ WARN improper_ctypes_definitions
+extern "cmse-nonsecure-entry" fn repr_transparent_union() -> Wrapper {
     match 0 {
-        //~^ WARN passing a union across the security boundary may leak information
-        0 => Wrapper(ReprRustUnionU64 { _unused: 1 }),
-        _ => Wrapper(ReprRustUnionU64 { _unused: 2 }),
+        //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
+        0 => Wrapper(MaybeUninit::new(0)),
+        _ => Wrapper(MaybeUninit::new(1)),
     }
 }
+
+// This is an aggregate that cannot be unwrapped, and has 1 (uninitialized) padding byte.
+#[repr(C, align(4))]
+struct PaddedStruct {
+    a: u8,
+    b: u16,
+}
+
+#[no_mangle]
+extern "cmse-nonsecure-entry" fn padded_struct() -> PaddedStruct {
+    PaddedStruct { a: 0, b: 1 }
+    //~^ WARN passing a (partially) uninitialized value across the security boundary may leak information
+}