Avoid unsafe when decoding

Also helps with big endian platforms like s390x

Signed-off-by: Daniel Schaefer <dhs@frame.work>

Author: JohnAZoidberg

Cargo.lock

@@ -36,6 +36,7 @@ no-std-compat = { version = "0.4.1", features = [ "alloc" ] }
 hidapi = { version = "2.6.3", features = [ "windows-native" ], optional = true }
 rusb = { version = "0.9.4", optional = true }
 guid-create = { version = "0.5.0", default-features = false }
+zerocopy = { version = "0.8", default-features = false, features = ["derive"] }
 
 [target.'cfg(target_os = "uefi")'.dependencies]
 uefi = { version = "0.36.1", features = ["alloc", "global_allocator", "panic_handler", "logger"] }

framework_lib/Cargo.toml

@@ -11,7 +11,7 @@
 use std::prelude::v1::*;
 
 use core::prelude::rust_2021::derive;
-use guid_create::CGuid;
+use guid_create::{CGuid, GUID};
 #[cfg(not(feature = "uefi"))]
 use std::fs::File;
 #[cfg(not(feature = "uefi"))]
@@ -112,10 +112,22 @@ fn print_capsule_flags(flags: u32) {
     }
 }
 
+fn read_capsule_header(data: &[u8]) -> EfiCapsuleHeader {
+    EfiCapsuleHeader {
+        capsule_guid: CGuid::from(GUID::build_from_components(
+            u32::from_le_bytes(data[0..4].try_into().unwrap()),
+            u16::from_le_bytes(data[4..6].try_into().unwrap()),
+            u16::from_le_bytes(data[6..8].try_into().unwrap()),
+            data[8..16].try_into().unwrap(),
+        )),
+        header_size: u32::from_le_bytes(data[16..20].try_into().unwrap()),
+        flags: u32::from_le_bytes(data[20..24].try_into().unwrap()),
+        capsule_image_size: u32::from_le_bytes(data[24..28].try_into().unwrap()),
+    }
+}
+
 pub fn parse_capsule_header(data: &[u8]) -> Option<EfiCapsuleHeader> {
-    let header_len = std::mem::size_of::<EfiCapsuleHeader>();
-    let header: EfiCapsuleHeader =
-        unsafe { std::ptr::read(data[0..header_len].as_ptr() as *const _) };
+    let header = read_capsule_header(data);
     if header.is_valid(data) {
         Some(header)
     } else {
@@ -141,10 +153,20 @@ pub fn print_capsule_header(header: &EfiCapsuleHeader) {
 }
 
 pub fn parse_ux_header(data: &[u8]) -> DisplayCapsule {
-    let header_len = std::mem::size_of::<DisplayCapsule>();
-    let header: DisplayCapsule =
-        unsafe { std::ptr::read(data[0..header_len].as_ptr() as *const _) };
-    header
+    let capsule_header = read_capsule_header(data);
+    let p = &data[std::mem::size_of::<EfiCapsuleHeader>()..];
+    DisplayCapsule {
+        capsule_header,
+        image_payload: DisplayPayload {
+            version: p[0],
+            checksum: p[1],
+            image_type: p[2],
+            reserved: p[3],
+            mode: u32::from_le_bytes(p[4..8].try_into().unwrap()),
+            offset_x: u32::from_le_bytes(p[8..12].try_into().unwrap()),
+            offset_y: u32::from_le_bytes(p[12..16].try_into().unwrap()),
+        },
+    }
 }
 pub fn print_ux_header(header: &DisplayCapsule) {
     let header_len = std::mem::size_of::<DisplayCapsule>();

framework_lib/src/capsule.rs

@@ -32,6 +32,8 @@ use alloc::vec::Vec;
 use core::prelude::rust_2021::derive;
 
 use crate::ccgx::{AppVersion, BaseVersion};
+use zerocopy::byteorder::little_endian::{U16, U32};
+use zerocopy::{FromBytes, KnownLayout};
 
 use super::*;
 
@@ -45,12 +47,12 @@ const SMALL_ROW: usize = 0x80;
 const LARGE_ROW: usize = 0x100;
 
 #[repr(C, packed)]
-#[derive(Debug, Copy, Clone)]
+#[derive(FromBytes, KnownLayout, Debug, Copy, Clone)]
 struct VersionInfo {
-    base_version: u32,
-    app_version: u32,
-    silicon_id: u16,
-    silicon_family: u16,
+    base_version: U32,
+    app_version: U32,
+    silicon_id: U16,
+    silicon_family: U16,
 }
 
 pub const CCG5_PD_LEN: usize = 0x20_000;
@@ -147,15 +149,13 @@ fn read_version(
     trace!("First row of firmware: {:X?}", data);
     let data = &data[FW_VERSION_OFFSET..];
 
-    let version_len = std::mem::size_of::<VersionInfo>();
-    let version_info: VersionInfo =
-        unsafe { std::ptr::read(data[0..version_len].as_ptr() as *const _) };
+    let (version_info, _) = VersionInfo::read_from_prefix(data).ok()?;
 
-    let base_version = BaseVersion::from(version_info.base_version);
-    let app_version = AppVersion::from(version_info.app_version);
+    let base_version = BaseVersion::from(version_info.base_version.get());
+    let app_version = AppVersion::from(version_info.app_version.get());
 
-    let fw_silicon_id = version_info.silicon_id;
-    let fw_silicon_family = version_info.silicon_family;
+    let fw_silicon_id = version_info.silicon_id.get();
+    let fw_silicon_family = version_info.silicon_family.get();
 
     Some(PdFirmware {
         silicon_id: fw_silicon_id,

framework_lib/src/ccgx/binary.rs

@@ -9,6 +9,8 @@ use alloc::vec::Vec;
 use core::prelude::rust_2021::derive;
 use num_derive::FromPrimitive;
 use std::fmt;
+use zerocopy::byteorder::little_endian::{U16, U32};
+use zerocopy::{FromBytes, KnownLayout};
 
 use crate::chromium_ec::{CrosEc, EcResult};
 use crate::smbios;
@@ -33,18 +35,18 @@ const METADATA_MAGIC: u16 = u16::from_le_bytes([b'Y', b'C']); // CY (Cypress)
 const CCG8_METADATA_MAGIC: u16 = u16::from_le_bytes([b'F', b'I']); // IF (Infineon)
 
 #[repr(C, packed)]
-#[derive(Debug, Copy, Clone)]
+#[derive(FromBytes, KnownLayout, Debug, Copy, Clone)]
 struct CyAcdMetadata {
     /// Offset 00: Single Byte FW Checksum
     _fw_checksum: u8,
     /// Offset 01: FW Entry Address
     _fw_entry: u32,
     /// Offset 05: Last Flash row of Bootloader or previous firmware
-    boot_last_row: u16,
+    boot_last_row: U16,
     /// Offset 07: Reserved
     _reserved1: [u8; 2],
     /// Offset 09: Size of Firmware
-    fw_size: u32,
+    fw_size: U32,
     /// Offset 0D: Reserved
     _reserved2: [u8; 3],
     /// Offset 10: Creator specific field
@@ -56,7 +58,7 @@ struct CyAcdMetadata {
     /// Offset 14: Creator specific field
     _boot_app_id: u16,
     /// Offset 16: Metadata Valid field. Valid if contains "CY"
-    metadata_valid: u16,
+    metadata_valid: U16,
     /// Offset 18: Creator specific field
     _fw_version: u32,
     /// Offset 1C: Boot sequence number field. Boot-loader will load the valid
@@ -67,12 +69,12 @@ struct CyAcdMetadata {
 
 // TODO: Would be nice to check the checksums
 #[repr(C, packed)]
-#[derive(Debug, Copy, Clone)]
+#[derive(FromBytes, KnownLayout, Debug, Copy, Clone)]
 struct CyAcd2Metadata {
     /// Offset 00: App Firmware Start
-    fw_start: u32,
+    fw_start: U32,
     /// Offset 04: App Firmware Size
-    fw_size: u32,
+    fw_size: U32,
     /// Offset 08: Boot wait time
     _boot_app_id: u16,
     /// Offset 0A: Last Flash row of Bootloader or previous firmware
@@ -89,9 +91,9 @@ struct CyAcd2Metadata {
     /// Offset 18: Reserved
     _reserved_1: [u32; 15],
     /// Offset 54: Version of the metadata structure
-    metadata_version: u16,
+    metadata_version: U16,
     /// Offset 56: Metadata Valid field. Valid if contains ASCII "IF"
-    metadata_valid: u16,
+    metadata_valid: U16,
     /// Offset 58: App Fw CRC32 checksum
     _fw_crc32: u32,
     /// Offset 5C: Reserved
@@ -267,12 +269,13 @@ pub fn get_pd_controller_versions(ec: &CrosEc) -> EcResult<PdVersions> {
 
 fn parse_metadata_ccg3(buffer: &[u8]) -> Option<(u32, u32)> {
     let buffer = &buffer[CCG3_METADATA_OFFSET..];
-    let metadata_len = std::mem::size_of::<CyAcdMetadata>();
-    let metadata: CyAcdMetadata =
-        unsafe { std::ptr::read(buffer[0..metadata_len].as_ptr() as *const _) };
+    let (metadata, _) = CyAcdMetadata::read_from_prefix(buffer).ok()?;
     trace!("Metadata: {:X?}", metadata);
-    if metadata.metadata_valid == METADATA_MAGIC {
-        Some((1 + metadata.boot_last_row as u32, metadata.fw_size))
+    if metadata.metadata_valid.get() == METADATA_MAGIC {
+        Some((
+            1 + metadata.boot_last_row.get() as u32,
+            metadata.fw_size.get(),
+        ))
     } else {
         None
     }
@@ -281,26 +284,25 @@ fn parse_metadata_ccg3(buffer: &[u8]) -> Option<(u32, u32)> {
 //fn parse_metadata(buffer: &[u8; 256]) -> Option<(u32, u32)> {
 fn parse_metadata_cyacd(buffer: &[u8]) -> Option<(u32, u32)> {
     let buffer = &buffer[METADATA_OFFSET..];
-    let metadata_len = std::mem::size_of::<CyAcdMetadata>();
-    let metadata: CyAcdMetadata =
-        unsafe { std::ptr::read(buffer[0..metadata_len].as_ptr() as *const _) };
+    let (metadata, _) = CyAcdMetadata::read_from_prefix(buffer).ok()?;
     trace!("Metadata: {:X?}", metadata);
-    if metadata.metadata_valid == METADATA_MAGIC {
-        Some((1 + metadata.boot_last_row as u32, metadata.fw_size))
+    if metadata.metadata_valid.get() == METADATA_MAGIC {
+        Some((
+            1 + metadata.boot_last_row.get() as u32,
+            metadata.fw_size.get(),
+        ))
     } else {
         None
     }
 }
 
 fn parse_metadata_cyacd2(buffer: &[u8]) -> Option<(u32, u32)> {
     let buffer = &buffer[CCG8_METADATA_OFFSET..];
-    let metadata_len = std::mem::size_of::<CyAcd2Metadata>();
-    let metadata: CyAcd2Metadata =
-        unsafe { std::ptr::read(buffer[0..metadata_len].as_ptr() as *const _) };
+    let (metadata, _) = CyAcd2Metadata::read_from_prefix(buffer).ok()?;
     trace!("Metadata: {:X?}", metadata);
-    if metadata.metadata_valid == CCG8_METADATA_MAGIC {
-        if metadata.metadata_version == 1 {
-            Some((metadata.fw_start, metadata.fw_size))
+    if metadata.metadata_valid.get() == CCG8_METADATA_MAGIC {
+        if metadata.metadata_version.get() == 1 {
+            Some((metadata.fw_start.get(), metadata.fw_size.get()))
         } else {
             println!("Unknown CCG8 metadata version");
             None

framework_lib/src/ccgx/mod.rs

@@ -16,19 +16,21 @@ const EC_RW_VER_OFFSET_ZEPHYR: usize = 0x40140;
 pub const EC_LEN: usize = 0x8_0000;
 
 use regex;
+use zerocopy::byteorder::little_endian::U32;
+use zerocopy::{FromBytes, KnownLayout};
 
 #[cfg(feature = "uefi")]
 use core::prelude::rust_2021::derive;
 
 // Defined in EC code as `struct image_data` in include/cros_version.h
-#[derive(Clone, Copy, Debug)]
+#[derive(FromBytes, KnownLayout, Clone, Copy, Debug)]
 #[repr(C, packed)]
 struct _ImageVersionData {
-    cookie1: u32,
+    cookie1: U32,
     version: [u8; 32],
-    size: u32,
-    rollback_version: u32,
-    cookie2: u32,
+    size: U32,
+    rollback_version: U32,
+    cookie2: U32,
 }
 /// Version Information about an EC FW binary
 #[derive(Debug, PartialEq)]
@@ -79,8 +81,8 @@ fn parse_ec_version(data: &_ImageVersionData) -> Option<ImageVersionData> {
         .trim_end_matches(char::from(0));
     Some(ImageVersionData {
         version: version.to_string(),
-        size: data.size,
-        rollback_version: data.rollback_version,
+        size: data.size.get(),
+        rollback_version: data.rollback_version.get(),
         details: parse_ec_version_str(version)?,
     })
 }
@@ -151,18 +153,17 @@ pub fn read_ec_version(data: &[u8], ro: bool) -> Option<ImageVersionData> {
     } else {
         EC_RW_VER_OFFSET
     };
-    if data.len() < offset + core::mem::size_of::<_ImageVersionData>() {
-        return None;
-    }
-    let v: _ImageVersionData = unsafe { std::ptr::read(data[offset..].as_ptr() as *const _) };
-    if v.cookie1 != CROS_EC_IMAGE_DATA_COOKIE1 {
-        debug!("Failed to find legacy Cookie 1. Found: {:X?}", {
-            v.cookie1
-        });
-    } else if v.cookie2 != CROS_EC_IMAGE_DATA_COOKIE2 {
-        debug!("Failed to find legacy Cookie 2. Found: {:X?}", {
-            v.cookie2
-        });
+    let (v, _) = _ImageVersionData::read_from_prefix(data.get(offset..)?).ok()?;
+    if v.cookie1.get() != CROS_EC_IMAGE_DATA_COOKIE1 {
+        debug!(
+            "Failed to find legacy Cookie 1. Found: {:X?}",
+            v.cookie1.get()
+        );
+    } else if v.cookie2.get() != CROS_EC_IMAGE_DATA_COOKIE2 {
+        debug!(
+            "Failed to find legacy Cookie 2. Found: {:X?}",
+            v.cookie2.get()
+        );
     } else {
         return parse_ec_version(&v);
     }
@@ -173,19 +174,17 @@ pub fn read_ec_version(data: &[u8], ro: bool) -> Option<ImageVersionData> {
     } else {
         EC_RW_VER_OFFSET_ZEPHYR
     };
-    if data.len() < offset_zephyr + core::mem::size_of::<_ImageVersionData>() {
-        return None;
-    }
-    let v: _ImageVersionData =
-        unsafe { std::ptr::read(data[offset_zephyr..].as_ptr() as *const _) };
-    if v.cookie1 != CROS_EC_IMAGE_DATA_COOKIE1 {
-        debug!("Failed to find Zephyr Cookie 1. Found: {:X?}", {
-            v.cookie1
-        });
-    } else if v.cookie2 != CROS_EC_IMAGE_DATA_COOKIE2 {
-        debug!("Failed to find Zephyr Cookie 2. Found: {:X?}", {
-            v.cookie2
-        });
+    let (v, _) = _ImageVersionData::read_from_prefix(data.get(offset_zephyr..)?).ok()?;
+    if v.cookie1.get() != CROS_EC_IMAGE_DATA_COOKIE1 {
+        debug!(
+            "Failed to find Zephyr Cookie 1. Found: {:X?}",
+            v.cookie1.get()
+        );
+    } else if v.cookie2.get() != CROS_EC_IMAGE_DATA_COOKIE2 {
+        debug!(
+            "Failed to find Zephyr Cookie 2. Found: {:X?}",
+            v.cookie2.get()
+        );
     } else {
         return parse_ec_version(&v);
     }
@@ -204,11 +203,11 @@ mod tests {
     fn can_parse() {
         let ver_chars: &[u8] = b"hx30_v0.0.1-7a61a89\0\0\0\0\0\0\0\0\0\0\0\0\0";
         let data = _ImageVersionData {
-            cookie1: CROS_EC_IMAGE_DATA_COOKIE1,
+            cookie1: U32::new(CROS_EC_IMAGE_DATA_COOKIE1),
             version: ver_chars.try_into().unwrap(),
-            size: 2868,
-            rollback_version: 0,
-            cookie2: CROS_EC_IMAGE_DATA_COOKIE1,
+            size: U32::new(2868),
+            rollback_version: U32::new(0),
+            cookie2: U32::new(CROS_EC_IMAGE_DATA_COOKIE1),
         };
         debug_assert_eq!(
             parse_ec_version(&data),