From 8769b78bf3573f349ff0fd06d9e0e0757f343acf Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Fri, 13 Jun 2025 14:27:00 -0400 Subject: [PATCH] arch: x86_64: Require that types to be checksummed are ByteValued Checksumming a type that has padding would use the padding in the checksum, which is definitely wrong and is often unsound. Signed-off-by: Demi Marie Obenour --- arch/src/x86_64/mpspec.rs | 64 ++++++++++++++++++++++++++++++++++++++ arch/src/x86_64/mptable.rs | 2 +- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/arch/src/x86_64/mpspec.rs b/arch/src/x86_64/mpspec.rs index 8e3eb976b..dacda287c 100644 --- a/arch/src/x86_64/mpspec.rs +++ b/arch/src/x86_64/mpspec.rs @@ -3,6 +3,8 @@ // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE-BSD-3-Clause file. +#![allow(non_camel_case_types)] +use vm_memory::ByteValued; pub const MP_PROCESSOR: ::std::os::raw::c_uint = 0; pub const MP_BUS: ::std::os::raw::c_uint = 1; @@ -29,6 +31,13 @@ pub struct mpf_intel { pub feature5: ::std::os::raw::c_uchar, } +const _: () = assert!(::core::mem::size_of::() == 16); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpf_intel {} + #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct mpc_table { @@ -45,6 +54,19 @@ pub struct mpc_table { pub reserved: ::std::os::raw::c_uint, } +const _: () = { + assert!(::core::mem::size_of::() == 4 + 2 + 1 + 1 + 8 + 12 + 4 + 2 + 2 + 4 + 4); + assert!(::core::mem::size_of::<::std::os::raw::c_uint>() == 4); + assert!(::core::mem::size_of::<::std::os::raw::c_ushort>() == 2); + assert!(::core::mem::size_of::<::std::os::raw::c_uchar>() == 1); +}; + +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_table {} + #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct mpc_cpu { @@ -57,6 +79,13 @@ pub struct mpc_cpu { pub reserved: [::std::os::raw::c_uint; 2usize], } +const _: () = assert!(::core::mem::size_of::() == 20); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_cpu {} + #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct mpc_bus { @@ -65,6 +94,13 @@ pub struct mpc_bus { pub bustype: [::std::os::raw::c_uchar; 6usize], } +const _: () = assert!(::core::mem::size_of::() == 8); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_bus {} + #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct mpc_ioapic { @@ -75,6 +111,13 @@ pub struct mpc_ioapic { pub apicaddr: ::std::os::raw::c_uint, } +const _: () = assert!(::core::mem::size_of::() == 8); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_ioapic {} + #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct mpc_intsrc { @@ -87,6 +130,13 @@ pub struct mpc_intsrc { pub dstirq: ::std::os::raw::c_uchar, } +const _: () = assert!(::core::mem::size_of::() == 8); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_intsrc {} + pub const MP_IRQ_SOURCE_TYPES_MP_INT: ::std::os::raw::c_uint = 0; pub const MP_IRQ_SOURCE_TYPES_MP_NMI: ::std::os::raw::c_uint = 1; pub const MP_IRQ_SOURCE_TYPES_MP_EXT_INT: ::std::os::raw::c_uint = 3; @@ -103,6 +153,13 @@ pub struct mpc_lintsrc { pub destapiclint: ::std::os::raw::c_uchar, } +const _: () = assert!(::core::mem::size_of::() == 8); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_lintsrc {} + #[repr(C)] #[derive(Debug, Default, Copy, Clone)] pub struct mpc_oemtable { @@ -112,3 +169,10 @@ pub struct mpc_oemtable { pub checksum: ::std::os::raw::c_uchar, pub mpc: [::std::os::raw::c_uchar; 8usize], } + +const _: () = assert!(::core::mem::size_of::() == 16); +// SAFETY: all members of this struct are plain integers +// and the sum of their sizes is the size of the struct, so +// padding and reserved values are not possible as there +// would be nowhere for them to exist. +unsafe impl ByteValued for mpc_oemtable {} diff --git a/arch/src/x86_64/mptable.rs b/arch/src/x86_64/mptable.rs index a40d7fd9a..aaf6f1ddd 100644 --- a/arch/src/x86_64/mptable.rs +++ b/arch/src/x86_64/mptable.rs @@ -106,7 +106,7 @@ const CPU_STEPPING: u32 = 0x600; const CPU_FEATURE_APIC: u32 = 0x200; const CPU_FEATURE_FPU: u32 = 0x001; -fn compute_checksum(v: &T) -> u8 { +fn compute_checksum(v: &T) -> u8 { // SAFETY: we are only reading the bytes within the size of the `T` reference `v`. let v_slice = unsafe { slice::from_raw_parts(v as *const T as *const u8, mem::size_of::()) }; let mut checksum: u8 = 0;