cfi: Simpler launder implementation for common types (#1714)

* `cfi_launder` is meant to prevent the Rust compiler from optimizing a value away.

Our current implementation uses `core::hint::black_box()`, which is the recommended way in Rust. The problem is, this appears to often force the argument to spill into memory and to be reloaded, which can be a lot of extra instructions.

The original inspiration for this function is from, I believe, [OpenTitan's launder* functions](https://github.com/lowRISC/opentitan/blob/master/sw/device/lib/base/hardened.h#L193). There, they use an LLVM-specific trick of a blank inline assembly block to force the compiler to keep the argument in a register.

After reviewing our code and speaking with @vsonims, it sounds like the intention of the launder in our code is to prevent the compiler from optimizing the value away (as the comments suggest), so the simpler inline assembly trick may be sufficient (since we use the official Rust compiler, which uses LLVM).

The biggest problem is that we launder many types of values in our code and not all of them fit into a register.

So, this PR represents an incremental change: for `u32`s and similar small types, we implement `cfi_launder` using the inline assembly trick from OpenTitan. For any other types, we have a trait that can be derived that will call `core::hint::black_box` in the same way as today.

We can do future follow-up PRs to try to try to clean up some of those other uses of `cfi_launder` to hopefully shrink the code more.

I also slipped in avoid a few extra copies in the verifier by using references instead of copies (this saves ~80 bytes of instruction space).

This PR appears to shrink the ROM code size by 1232 bytes and the runtime firmware by 700 bytes.

FROZEN_IMAGES.sha384sum

@@ -1,3 +1,3 @@
 # WARNING: Do not update this file without the approval of the Caliptra TAC
-9537318fd30c3e3d341cffab5721ba3242810be85e79ed9dc644c47c062555ef7519fb48857745e7ccb9917ac1fe120a  caliptra-rom-no-log.bin
-e4e74d2d1c4794b950a548072fc8dc4c9ab64aba7a01ae400e9fe66c64b43f715e72dc430e7318496009ebedd0412bc6  caliptra-rom-with-log.bin
+ad1064cba5b190e4f5258c175b7ec7c697ccb188ba0ed18302c6e924f0ea0b10457dc753d6d2963f415fbaf761eace96  caliptra-rom-no-log.bin
+4d93a6856a0cba636a5d2a9c116393f56dc3b34113f7149ae62325772c1eacb08b1d78424771cc71197252abe603eb92  caliptra-rom-with-log.bin

cfi/derive/src/cfi_asm_test.rs

@@ -0,0 +1,78 @@
+// Licensed under the Apache-2.0 license
+
+// These tests are here so that they are excluded in FPGA tests.
+
+// These tests don't directly import the CFI code. If they fail,
+// this likely indicates that the CFI laundering code may not
+// be doing what we want, and we need to investigate.
+
+#[cfg(test)]
+mod test {
+
+    const START: &str = "
+#![no_std]
+
+pub fn add(mut a: u32, mut b: u32) -> u32 {
+    launder(a) + launder(a) + launder(b) + launder(b)
+}
+";
+
+    const LAUNDER: &str = "
+#[inline(always)]
+fn launder(mut val: u32) -> u32 {
+    // Safety: this is a no-op, since we don't modify the input.
+    unsafe {
+        core::arch::asm!(
+            \"/* {t} */\",
+            t = inout(reg) val,
+        );
+    }
+    val
+}";
+
+    const NO_LAUNDER: &str = "
+#[inline(always)]
+fn launder(mut val: u32) -> u32 {
+    val
+}
+";
+
+    fn compile_to_riscv32_asm(src: String) -> String {
+        let dir = std::env::temp_dir();
+        let src_path = dir.join("asm.rs");
+        let dst_path = dir.join("asm.s");
+
+        std::fs::write(src_path.clone(), src).expect("could not write asm file");
+
+        let p = std::process::Command::new("rustc")
+            .args([
+                "--crate-type=lib",
+                "--target",
+                "riscv32imc-unknown-none-elf",
+                "-C",
+                "opt-level=s",
+                "--emit",
+                "asm",
+                src_path.to_str().expect("could not convert path"),
+                "-o",
+                dst_path.to_str().expect("could not convert path"),
+            ])
+            .output()
+            .expect("failed to compile");
+        assert!(p.status.success());
+        std::fs::read_to_string(dst_path).expect("could not read asm file")
+    }
+
+    #[test]
+    fn test_launder() {
+        // With no laundering, LLVM can simplify the double add to a shift left.
+        let src = format!("{}{}", START, NO_LAUNDER);
+        let asm = compile_to_riscv32_asm(src);
+        assert!(asm.contains("sll"));
+
+        // With laundering, LLVM cannot simplify the double add and has to use the register twice.
+        let src = format!("{}{}", START, LAUNDER);
+        let asm = compile_to_riscv32_asm(src);
+        assert!(!asm.contains("sll"));
+    }
+}

cfi/derive/src/lib.rs

@@ -16,11 +16,14 @@ References:
 
 --*/
 
+mod cfi_asm_test;
+
 use proc_macro::TokenStream;
 use quote::{format_ident, quote, ToTokens};
 use syn::__private::TokenStream2;
 use syn::parse_macro_input;
 use syn::parse_quote;
+use syn::DeriveInput;
 use syn::FnArg;
 use syn::ItemFn;
 
@@ -94,3 +97,15 @@ fn cfi_fn(mod_fn: bool, input: TokenStream) -> TokenStream {
 
     code.into()
 }
+
+#[proc_macro_derive(Launder)]
+pub fn derive_launder_trait(input: TokenStream) -> TokenStream {
+    let input = parse_macro_input!(input as DeriveInput);
+    let name = input.ident;
+    let (impl_generics, ty_generics, _) = input.generics.split_for_impl();
+    let expanded = quote! {
+        impl #impl_generics caliptra_cfi_lib::LaunderTrait<#name #ty_generics> for caliptra_cfi_lib::Launder<#name #ty_generics> {}
+        impl #impl_generics caliptra_cfi_lib::LaunderTrait<&#name #ty_generics> for caliptra_cfi_lib::Launder<&#name #ty_generics> {}
+    };
+    TokenStream::from(expanded)
+}

cfi/lib/src/cfi.rs

@@ -20,7 +20,7 @@ use caliptra_error::CaliptraError;
 use crate::CfiCounter;
 use core::cfg;
 use core::cmp::{Eq, Ord, PartialEq, PartialOrd};
-use core::marker::Copy;
+use core::marker::{Copy, PhantomData};
 
 /// CFI Panic Information
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
@@ -94,16 +94,107 @@ impl From<CfiPanicInfo> for CaliptraError {
 /// # Returns
 ///
 /// `T` - Same value
-pub fn cfi_launder<T>(val: T) -> T {
+pub fn cfi_launder<T>(val: T) -> T
+where
+    Launder<T>: LaunderTrait<T>,
+{
     if cfg!(feature = "cfi") {
-        // Note: The black box seems to be disabling more optimization
-        // than necessary and results in larger binary size
-        core::hint::black_box(val)
+        Launder { _val: PhantomData }.launder(val)
     } else {
         val
     }
 }
 
+pub trait LaunderTrait<T> {
+    fn launder(&self, val: T) -> T {
+        core::hint::black_box(val)
+    }
+}
+
+pub struct Launder<T> {
+    _val: PhantomData<T>,
+}
+
+// Inline-assembly laundering trick is adapted from OpenTitan:
+// https://github.com/lowRISC/opentitan/blob/master/sw/device/lib/base/hardened.h#L193
+//
+// NOTE: This implementation is LLVM-specific, and should be considered to be
+// a no-op in every other compiler. For example, GCC has in the past peered
+// into the insides of assembly blocks.
+//
+// At the time of writing, it seems preferable to have something we know is
+// correct rather than being overly clever; this is recorded here in case
+// the current implementation is unsuitable and we need something more
+// carefully tuned.
+//
+// Unlike in C, we don't have volatile assembly blocks, so this doesn't
+// necessarily prevent reordering by LLVM.
+//
+// When we're building for static analysis, reduce false positives by
+// short-circuiting the inline assembly block.
+impl LaunderTrait<u32> for Launder<u32> {
+    #[allow(asm_sub_register)]
+    fn launder(&self, val: u32) -> u32 {
+        let mut val = val;
+        // Safety: this is a no-op, since we don't modify the input.
+        unsafe {
+            // We use inout so that LLVM thinks the value might
+            // be mutated by the assembly and can't eliminate it.
+            core::arch::asm!(
+                "/* {t} */",
+                t = inout(reg) val,
+            );
+        }
+        val
+    }
+}
+
+impl LaunderTrait<bool> for Launder<bool> {
+    #[allow(asm_sub_register)]
+    fn launder(&self, val: bool) -> bool {
+        let mut val = val as u32;
+        // Safety: this is a no-op, since we don't modify the input.
+        unsafe {
+            core::arch::asm!(
+                "/* {t} */",
+                t = inout(reg) val,
+            );
+        }
+        val != 0
+    }
+}
+
+impl LaunderTrait<usize> for Launder<usize> {
+    #[allow(asm_sub_register)]
+    fn launder(&self, mut val: usize) -> usize {
+        // Safety: this is a no-op, since we don't modify the input.
+        unsafe {
+            core::arch::asm!(
+                "/* {t} */",
+                t = inout(reg) val,
+            );
+        }
+        val
+    }
+}
+
+impl<const N: usize, T> LaunderTrait<[T; N]> for Launder<[T; N]> {}
+impl<'a, const N: usize, T> LaunderTrait<&'a [T; N]> for Launder<&'a [T; N]> {
+    fn launder(&self, val: &'a [T; N]) -> &'a [T; N] {
+        let mut valp = val.as_ptr() as *const [T; N];
+        // Safety: this is a no-op, since we don't modify the input.
+        unsafe {
+            core::arch::asm!(
+                "/* {t} */",
+                t = inout(reg) valp,
+            );
+            &*valp
+        }
+    }
+}
+impl LaunderTrait<Option<u32>> for Launder<Option<u32>> {}
+impl LaunderTrait<CfiPanicInfo> for Launder<CfiPanicInfo> {}
+
 /// Control flow integrity panic
 ///
 /// This panic is raised when the control flow integrity error is detected
@@ -157,6 +248,7 @@ macro_rules! cfi_assert_macro {
         pub fn $name<T>(lhs: T, rhs: T)
         where
             T: $trait1 + $trait2,
+            Launder<T>: LaunderTrait<T>,
         {
             if cfg!(feature = "cfi") {
                 CfiCounter::delay();
@@ -184,10 +276,28 @@ cfi_assert_macro!(cfi_assert_lt, <, Ord, PartialOrd, AssertLtFail);
 cfi_assert_macro!(cfi_assert_ge, >=, Ord, PartialOrd, AssertGeFail);
 cfi_assert_macro!(cfi_assert_le, <=, Ord, PartialOrd, AssertLeFail);
 
+// special case for bool assert
+#[inline(always)]
+#[allow(unused)]
+pub fn cfi_assert_bool(cond: bool) {
+    if cfg!(feature = "cfi") {
+        CfiCounter::delay();
+        if !cond {
+            cfi_panic(CfiPanicInfo::AssertEqFail);
+        }
+
+        // Second check for glitch protection
+        CfiCounter::delay();
+        if !cfi_launder(cond) {
+            cfi_panic(CfiPanicInfo::AssertEqFail);
+        }
+    }
+}
+
 #[macro_export]
 macro_rules! cfi_assert {
     ($cond: expr) => {
-        cfi_assert_eq($cond, true)
+        cfi_assert_bool($cond)
     };
 }
 

drivers/src/array.rs

@@ -13,6 +13,7 @@ Abstract:
 
 --*/
 
+use caliptra_cfi_derive::Launder;
 use core::mem::MaybeUninit;
 use zerocopy::{AsBytes, FromBytes};
 use zeroize::Zeroize;
@@ -26,7 +27,7 @@ macro_rules! static_assert {
 /// The `Array4xN` type represents large arrays in the native format of the Caliptra
 /// cryptographic hardware, and provides From traits for converting to/from byte arrays.
 #[repr(transparent)]
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Zeroize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Launder, Zeroize)]
 pub struct Array4xN<const W: usize, const B: usize>(pub [u32; W]);
 impl<const W: usize, const B: usize> Array4xN<W, B> {
     pub const fn new(val: [u32; W]) -> Self {

drivers/src/fuse_bank.rs

@@ -13,6 +13,7 @@ Abstract:
 --*/
 
 use crate::Array4x12;
+use caliptra_cfi_derive::Launder;
 use caliptra_registers::soc_ifc::SocIfcReg;
 use zerocopy::AsBytes;
 
@@ -33,7 +34,7 @@ pub enum X509KeyIdAlgo {
 }
 
 bitflags::bitflags! {
-    #[derive(Default, Copy, Clone, Debug)]
+    #[derive(Default, Copy, Clone, Debug, Launder)]
     pub struct VendorPubKeyRevocation : u32 {
         const KEY0 = 0b0001;
         const KEY1 = 0b0010;
@@ -76,7 +77,7 @@ impl FuseBank<'_> {
     /// * None
     ///
     /// # Returns
-    ///     key id crypto algorithm  
+    ///     key id crypto algorithm
     ///
     pub fn idev_id_x509_key_id_algo(&self) -> X509KeyIdAlgo {
         let soc_ifc_regs = self.soc_ifc.regs();
@@ -101,7 +102,7 @@ impl FuseBank<'_> {
     /// * None
     ///
     /// # Returns
-    ///     manufacturer serial number  
+    ///     manufacturer serial number
     ///
     pub fn ueid(&self) -> [u8; 17] {
         let soc_ifc_regs = self.soc_ifc.regs();

drivers/src/soc_ifc.rs

@@ -12,6 +12,7 @@ Abstract:
 
 --*/
 
+use caliptra_cfi_derive::Launder;
 use caliptra_error::{CaliptraError, CaliptraResult};
 use caliptra_registers::soc_ifc::enums::DeviceLifecycleE;
 use caliptra_registers::soc_ifc::{self, SocIfcReg};
@@ -382,7 +383,7 @@ impl From<u32> for MfgFlags {
 }
 
 /// Reset Reason
-#[derive(Debug, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Launder)]
 pub enum ResetReason {
     /// Cold Reset
     ColdReset,

fmc/src/flow/rt_alias.rs

@@ -12,8 +12,7 @@ Abstract:
 
 --*/
 use caliptra_cfi_derive::cfi_impl_fn;
-use caliptra_cfi_lib::cfi_assert_eq;
-use caliptra_cfi_lib::{cfi_assert, cfi_launder};
+use caliptra_cfi_lib::{cfi_assert, cfi_assert_bool, cfi_assert_eq, cfi_launder};
 
 use crate::flow::crypto::Crypto;
 use crate::flow::dice::{DiceInput, DiceOutput};

image/types/Cargo.toml

@@ -10,6 +10,8 @@ doctest = false
 
 [dependencies]
 arbitrary = { workspace = true, optional = true }
+caliptra-cfi-derive.workspace = true
+caliptra-cfi-lib.workspace = true
 caliptra-lms-types.workspace = true
 memoffset.workspace = true
 zerocopy.workspace = true