From 6eff51b65f57ffe554e9cc8682c1bc8ba7fc9bb3 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Thu, 26 Mar 2026 23:07:48 +0800 Subject: [PATCH] Avoid unsafe when decoding Also helps with big endian platforms like s390x Signed-off-by: Daniel Schaefer --- Cargo.lock | 21 ++++++++++ framework_lib/Cargo.toml | 1 + framework_lib/src/capsule.rs | 38 +++++++++++++---- framework_lib/src/ccgx/binary.rs | 24 +++++------ framework_lib/src/ccgx/mod.rs | 52 ++++++++++++----------- framework_lib/src/ec_binary.rs | 71 ++++++++++++++++---------------- 6 files changed, 126 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ece549f..fdf9dc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -411,6 +411,7 @@ dependencies = [ "windows-version", "winreg", "wmi", + "zerocopy", ] [[package]] @@ -1935,6 +1936,26 @@ dependencies = [ "synstructure", ] +[[package]] +name = "zerocopy" +version = "0.8.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "efbb2a062be311f2ba113ce66f697a4dc589f85e78a4aea276200804cea0ed87" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.8.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e8bc7269b54418e7aeeef514aa68f8690b8c0489a06b0136e5f57c4c5ccab89" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "zerofrom" version = "0.1.6" diff --git a/framework_lib/Cargo.toml b/framework_lib/Cargo.toml index 8c1e322..966b0b5 100644 --- a/framework_lib/Cargo.toml +++ b/framework_lib/Cargo.toml @@ -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"] } diff --git a/framework_lib/src/capsule.rs b/framework_lib/src/capsule.rs index d5eac38..6ccfe73 100644 --- a/framework_lib/src/capsule.rs +++ b/framework_lib/src/capsule.rs @@ -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 { - let header_len = std::mem::size_of::(); - 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::(); - 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::()..]; + 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::(); diff --git a/framework_lib/src/ccgx/binary.rs b/framework_lib/src/ccgx/binary.rs index 317a7c7..9206efb 100644 --- a/framework_lib/src/ccgx/binary.rs +++ b/framework_lib/src/ccgx/binary.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::(); - 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, diff --git a/framework_lib/src/ccgx/mod.rs b/framework_lib/src/ccgx/mod.rs index 4495cc7..65239e6 100644 --- a/framework_lib/src/ccgx/mod.rs +++ b/framework_lib/src/ccgx/mod.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 { fn parse_metadata_ccg3(buffer: &[u8]) -> Option<(u32, u32)> { let buffer = &buffer[CCG3_METADATA_OFFSET..]; - let metadata_len = std::mem::size_of::(); - 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,12 +284,13 @@ 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::(); - 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 } @@ -294,13 +298,11 @@ fn parse_metadata_cyacd(buffer: &[u8]) -> Option<(u32, u32)> { fn parse_metadata_cyacd2(buffer: &[u8]) -> Option<(u32, u32)> { let buffer = &buffer[CCG8_METADATA_OFFSET..]; - let metadata_len = std::mem::size_of::(); - 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 diff --git a/framework_lib/src/ec_binary.rs b/framework_lib/src/ec_binary.rs index 1519310..f656f7f 100644 --- a/framework_lib/src/ec_binary.rs +++ b/framework_lib/src/ec_binary.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 { .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 { } 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 { } 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),