hashcrypt: minimize panic code paths (#528)

jerrysxie · web-flow · commit 92c81e7cf9d8 · 2025-12-18T18:40:35.000-06:00
This pull request enhances error handling in the `hashcrypt` hasher
implementation by introducing a unified `Error` type and updating
relevant methods to return `Result` instead of panicking. This makes the
code more robust, especially for invalid input scenarios, and improves
its safety and reliability in both blocking and asynchronous hashing
modes.

**Error handling improvements:**

* Introduced a new `Error` enum in `src/hashcrypt/mod.rs` to represent
unsupported configurations and updated code to use this error type
instead of panicking.
* Updated internal helper methods in `Hasher` (such as
`init_final_data`, `init_final_block`, and `init_final_len`) to return
`Result&lt;(), Error&gt;` and propagate errors instead of panicking on invalid
buffer access.

**Blocking hasher API changes:**

* Changed public and internal methods in the blocking `Hasher`
implementation (`submit_blocks`, `finalize`, `hash`) to return `Result`
and handle errors gracefully, replacing panics with error returns for
invalid input lengths.

**Async hasher API changes:**

* Updated async `Hasher` methods (`transfer`, `submit_blocks`,
`finalize`, `hash`) to return `Result` and propagate errors, ensuring
safe handling of invalid conditions such as missing DMA channels or
incorrect data lengths.
[[1]](diffhunk://#diff-2b7655c2223a12761dadfbbd782cba8cf016f31e8b298aa46e9519bd81ff2158L130-R150)
[[2]](diffhunk://#diff-2b7655c2223a12761dadfbbd782cba8cf016f31e8b298aa46e9519bd81ff2158L141-R159)
[[3]](diffhunk://#diff-2b7655c2223a12761dadfbbd782cba8cf016f31e8b298aa46e9519bd81ff2158R191-R232)

**General code safety improvements:**

* Added assertions and explicit error returns to ensure buffer sizes and
data lengths are always valid, replacing unchecked unwraps and panics
with error handling.
[[1]](diffhunk://#diff-2b7655c2223a12761dadfbbd782cba8cf016f31e8b298aa46e9519bd81ff2158R78-R138)
[[2]](diffhunk://#diff-2b7655c2223a12761dadfbbd782cba8cf016f31e8b298aa46e9519bd81ff2158L37-R56)
diff --git a/examples/rt685s-evk/src/bin/sha256-async.rs b/examples/rt685s-evk/src/bin/sha256-async.rs
@@ -22,7 +22,7 @@ async fn main(_spawner: Spawner) {
     info!("Starting hashes");
     // Data that fits into a single block
     info!("Single hash block");
-    hashcrypt.new_sha256().hash(b"abc", &mut hash).await;
+    hashcrypt.new_sha256().hash(b"abc", &mut hash).await.unwrap();
     defmt::assert_eq!(
         &hash,
         &[
@@ -39,7 +39,8 @@ async fn main(_spawner: Spawner) {
             b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*()",
             &mut hash,
         )
-        .await;
+        .await
+        .unwrap();
     defmt::assert_eq!(
         &hash,
         &[
@@ -56,7 +57,8 @@ async fn main(_spawner: Spawner) {
             b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@",
             &mut hash,
         )
-        .await;
+        .await
+        .unwrap();
     defmt::assert_eq!(
         &hash,
         &[
@@ -70,7 +72,7 @@ async fn main(_spawner: Spawner) {
     hashcrypt.new_sha256().hash(
         b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ12345678",
         &mut hash,
-    ).await;
+    ).await.unwrap();
     defmt::assert_eq!(
         &hash,
         &[
diff --git a/examples/rt685s-evk/src/bin/sha256.rs b/examples/rt685s-evk/src/bin/sha256.rs
@@ -17,7 +17,7 @@ async fn main(_spawner: Spawner) {
     info!("Starting hashes");
     // Data that fits into a single block
     info!("Single hash block");
-    hashcrypt.new_sha256().hash(b"abc", &mut hash);
+    hashcrypt.new_sha256().hash(b"abc", &mut hash).unwrap();
     defmt::assert_eq!(
         &hash,
         &[
@@ -28,10 +28,13 @@ async fn main(_spawner: Spawner) {
 
     // Data that fits into two blocks
     info!("Two hash blocks");
-    hashcrypt.new_sha256().hash(
-        b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*()",
-        &mut hash,
-    );
+    hashcrypt
+        .new_sha256()
+        .hash(
+            b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*()",
+            &mut hash,
+        )
+        .unwrap();
     defmt::assert_eq!(
         &hash,
         &[
@@ -42,10 +45,13 @@ async fn main(_spawner: Spawner) {
 
     // Data that is exactly one block
     info!("One block exactly");
-    hashcrypt.new_sha256().hash(
-        b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@",
-        &mut hash,
-    );
+    hashcrypt
+        .new_sha256()
+        .hash(
+            b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@",
+            &mut hash,
+        )
+        .unwrap();
     defmt::assert_eq!(
         &hash,
         &[
@@ -59,7 +65,7 @@ async fn main(_spawner: Spawner) {
     hashcrypt.new_sha256().hash(
         b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ12345678",
         &mut hash,
-    );
+    ).unwrap();
     defmt::assert_eq!(
         &hash,
         &[
diff --git a/src/hashcrypt/hasher.rs b/src/hashcrypt/hasher.rs
@@ -5,7 +5,7 @@ use core::task::Poll;
 
 use embassy_futures::select::select;
 
-use super::{Async, Blocking, Hashcrypt, Mode};
+use super::{Async, Blocking, Error, Hashcrypt, Mode};
 use crate::dma;
 use crate::dma::transfer::{Transfer, Width};
 
@@ -34,18 +34,26 @@ impl<'d, 'a, M: Mode> Hasher<'d, 'a, M> {
         }
     }
 
-    fn init_final_data(&self, data: &[u8], buffer: &mut [u8; BLOCK_LEN]) {
-        buffer[..data.len()].copy_from_slice(data);
-        buffer[data.len()] = END_BYTE;
+    fn init_final_data(&self, data: &[u8], buffer: &mut [u8; BLOCK_LEN]) -> Result<(), Error> {
+        buffer
+            .get_mut(..data.len())
+            .ok_or(Error::UnsupportedConfiguration)?
+            .copy_from_slice(data);
+        *buffer.get_mut(data.len()).ok_or(Error::UnsupportedConfiguration)? = END_BYTE;
+        Ok(())
     }
 
-    fn init_final_block(&self, data: &[u8], buffer: &mut [u8; BLOCK_LEN]) {
-        self.init_final_data(data, buffer);
-        self.init_final_len(buffer);
+    fn init_final_block(&self, data: &[u8], buffer: &mut [u8; BLOCK_LEN]) -> Result<(), Error> {
+        self.init_final_data(data, buffer)?;
+        self.init_final_len(buffer)
     }
 
-    fn init_final_len(&self, buffer: &mut [u8; BLOCK_LEN]) {
-        buffer[BLOCK_LEN - 8..BLOCK_LEN].copy_from_slice(&(8 * self.written as u64).to_be_bytes());
+    fn init_final_len(&self, buffer: &mut [u8; BLOCK_LEN]) -> Result<(), Error> {
+        buffer
+            .get_mut(BLOCK_LEN - 8..BLOCK_LEN)
+            .ok_or(Error::UnsupportedConfiguration)?
+            .copy_from_slice(&(8 * self.written as u64).to_be_bytes());
+        Ok(())
     }
 
     fn wait_for_digest(&self) {
@@ -67,57 +75,67 @@ impl<'d, 'a> Hasher<'d, 'a, Blocking> {
     }
 
     fn transfer_block(&mut self, data: &[u8; BLOCK_LEN]) {
+        const _: () = core::assert!(BLOCK_LEN.is_multiple_of(4), "BLOCK_LEN must be divisible by 4");
+
         for word in data.chunks(4) {
-            self.hashcrypt
-                .hashcrypt
-                .indata()
-                .write(|w| unsafe { w.data().bits(u32::from_le_bytes([word[0], word[1], word[2], word[3]])) });
+            self.hashcrypt.hashcrypt.indata().write(|w| unsafe {
+                #[allow(clippy::unwrap_used)]
+                // panic safety: word is always 4 bytes and BLOCK_LEN is multiple of 4
+                w.data().bits(u32::from_le_bytes(word.try_into().unwrap()))
+            });
         }
         self.wait_for_digest();
     }
 
     /// Submit one or more blocks of data to the hasher, data must be a multiple of the block length
-    pub fn submit_blocks(&mut self, data: &[u8]) {
+    pub fn submit_blocks(&mut self, data: &[u8]) -> Result<(), Error> {
         if data.is_empty() || !data.len().is_multiple_of(BLOCK_LEN) {
-            panic!("Invalid data length");
+            return Err(Error::UnsupportedConfiguration);
         }
 
         for block in data.chunks(BLOCK_LEN) {
+            #[allow(clippy::unwrap_used)] // panic safety: block is always BLOCK_LEN bytes with check above
             self.transfer_block(block.try_into().unwrap());
         }
         self.written += data.len();
+        Ok(())
     }
 
     /// Submits the final data for hashing
-    pub fn finalize(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) {
+    pub fn finalize(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) -> Result<(), Error> {
         let mut buffer = [0u8; BLOCK_LEN];
 
         self.written += data.len();
         if data.len() <= LAST_BLOCK_MAX_DATA {
             // Only have one final block
-            self.init_final_block(data, &mut buffer);
+            self.init_final_block(data, &mut buffer)?;
             self.transfer_block(&buffer);
         } else {
             //End byte and padding won't fit in this block, submit this block and an extra one
-            self.init_final_data(data, &mut buffer);
+            self.init_final_data(data, &mut buffer)?;
             self.transfer_block(&buffer);
 
             buffer.fill(0);
-            self.init_final_len(&mut buffer);
+            self.init_final_len(&mut buffer)?;
             self.transfer_block(&buffer);
         }
 
         self.read_hash(hash);
+
+        Ok(())
     }
 
     /// Computes the hash of the given data
-    pub fn hash(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) {
-        let full_blocks = data.len() / BLOCK_LEN;
+    pub fn hash(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) -> Result<(), Error> {
+        let mut iter = data.chunks_exact(BLOCK_LEN);
 
-        if full_blocks > 0 {
-            self.submit_blocks(&data[0..full_blocks * BLOCK_LEN]);
+        for block in &mut iter {
+            self.submit_blocks(block)?;
         }
-        self.finalize(&data[full_blocks * BLOCK_LEN..], hash);
+
+        self.finalize(iter.remainder(), hash)?;
+
+        Ok(())
     }
 }
 
@@ -127,9 +145,9 @@ impl<'d, 'a> Hasher<'d, 'a, Async> {
         Self::new_inner(hashcrypt)
     }
 
-    async fn transfer(&mut self, data: &[u8]) {
+    async fn transfer(&mut self, data: &[u8]) -> Result<(), Error> {
         if data.is_empty() || !data.len().is_multiple_of(BLOCK_LEN) {
-            panic!("Invalid data length");
+            return Err(Error::UnsupportedConfiguration);
         }
 
         let options = dma::transfer::TransferOptions {
@@ -138,7 +156,7 @@ impl<'d, 'a> Hasher<'d, 'a, Async> {
         };
 
         let transfer = Transfer::new_write(
-            self.hashcrypt.dma_ch.as_ref().unwrap(),
+            self.hashcrypt.dma_ch.as_ref().ok_or(Error::UnsupportedConfiguration)?,
             data,
             self.hashcrypt.hashcrypt.indata().as_ptr() as *mut u8,
             options,
@@ -170,43 +188,47 @@ impl<'d, 'a> Hasher<'d, 'a, Async> {
             Poll::Pending
         })
         .await;
+
+        Ok(())
     }
 
     /// Submit one or more blocks of data to the hasher, data must be a multiple of the block length
-    pub async fn submit_blocks(&mut self, data: &[u8]) {
-        self.transfer(data).await;
+    pub async fn submit_blocks(&mut self, data: &[u8]) -> Result<(), Error> {
+        self.transfer(data).await?;
         self.written += data.len();
+        Ok(())
     }
 
     /// Submits the final data for hashing
-    pub async fn finalize(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) {
+    pub async fn finalize(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) -> Result<(), Error> {
         let mut buffer = [0u8; BLOCK_LEN];
 
         self.written += data.len();
         if data.len() <= LAST_BLOCK_MAX_DATA {
             // Only have one final block
-            self.init_final_block(data, &mut buffer);
-            self.transfer(&buffer).await;
+            self.init_final_block(data, &mut buffer)?;
+            self.transfer(&buffer).await?;
         } else {
             //End byte and padding won't fit in this block, submit this block and an extra one
-            self.init_final_data(data, &mut buffer);
-            self.transfer(&buffer).await;
-
+            self.init_final_data(data, &mut buffer)?;
+            self.transfer(&buffer).await?;
             buffer.fill(0);
-            self.init_final_len(&mut buffer);
-            self.transfer(&buffer).await;
+            self.init_final_len(&mut buffer)?;
+            self.transfer(&buffer).await?;
         }
 
         self.read_hash(hash);
+        Ok(())
     }
 
     /// Computes the hash of the given data
-    pub async fn hash(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) {
-        let full_blocks = data.len() / BLOCK_LEN;
+    pub async fn hash(mut self, data: &[u8], hash: &mut [u8; HASH_LEN]) -> Result<(), Error> {
+        let mut iter = data.chunks_exact(BLOCK_LEN);
 
-        if full_blocks > 0 {
-            self.submit_blocks(&data[0..full_blocks * BLOCK_LEN]).await;
+        for block in &mut iter {
+            self.submit_blocks(block).await?;
         }
-        self.finalize(&data[full_blocks * BLOCK_LEN..], hash).await;
+
+        self.finalize(iter.remainder(), hash).await
     }
 }
diff --git a/src/hashcrypt/mod.rs b/src/hashcrypt/mod.rs
@@ -12,6 +12,14 @@ use crate::{Peri, dma, interrupt, pac};
 /// Hasher module
 pub mod hasher;
 
+/// Error information type
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum Error {
+    /// configuration requested is not supported
+    UnsupportedConfiguration,
+}
+
 trait Sealed {}
 
 /// Asynchronous or blocking mode
