From 367e7e7b61fdf86ce2cab7122ecc9b3c9642aa9a Mon Sep 17 00:00:00 2001 From: Rowan Hart Date: Thu, 28 Sep 2023 09:11:45 -0700 Subject: [PATCH] Fix Box::into_raw and add checks (#27) --- simics-api/src/safe/base/attr_value.rs | 12 +++++++++--- simics-api/src/safe/base/conf_object.rs | 6 ++++++ simics-api/src/safe/simulator/callbacks.rs | 22 +++++++++++++++++----- simics-fuzz/src/fuzzer/mod.rs | 16 +++++++++++++--- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/simics-api/src/safe/base/attr_value.rs b/simics-api/src/safe/base/attr_value.rs index 513f8893..059bd54a 100644 --- a/simics-api/src/safe/base/attr_value.rs +++ b/simics-api/src/safe/base/attr_value.rs @@ -148,11 +148,17 @@ pub fn make_attr_object(obj: *mut ConfObject) -> Result { /// size. The data will be moved into a [`Box`], which will be converted to a raw pointer. pub fn make_attr_data_adopt(data: T) -> Result { let data = Box::new(data); - let data_ptr = Box::into_raw(data); + let data_raw = Box::into_raw(data); + + debug_assert!( + std::mem::size_of_val(&data_raw) == std::mem::size_of::<*mut std::ffi::c_void>(), + "Pointer is not convertible to *mut c_void" + ); + let data_size = u32::try_from(size_of::<*mut T>())?; ensure!( - !(data_ptr.is_null() && data_size == 0), + !(data_raw.is_null() && data_size == 0), "NULL data requires zero size" ); @@ -160,7 +166,7 @@ pub fn make_attr_data_adopt(data: T) -> Result { private_kind: AttrKind::Data.try_into()?, private_size: u32::try_from(data_size)?, private_u: attr_value__bindgen_ty_1 { - data: data_ptr as *mut u8, + data: data_raw as *mut u8, }, }) } diff --git a/simics-api/src/safe/base/conf_object.rs b/simics-api/src/safe/base/conf_object.rs index 2deb1dba..11f32a64 100644 --- a/simics-api/src/safe/base/conf_object.rs +++ b/simics-api/src/safe/base/conf_object.rs @@ -68,6 +68,12 @@ where // Note: This allocates and never frees. This is *required* by SIMICS and it is an error to // free this pointer let iface_raw = Box::into_raw(iface_box); + + debug_assert!( + std::mem::size_of_val(&iface_raw) == std::mem::size_of::<*mut std::ffi::c_void>(), + "Pointer is not convertible to *mut c_void" + ); + let status = unsafe { SIM_register_interface(cls.into(), name_raw, iface_raw as *mut _) }; if status != 0 { diff --git a/simics-api/src/safe/simulator/callbacks.rs b/simics-api/src/safe/simulator/callbacks.rs index d5acb5fd..9a179734 100644 --- a/simics-api/src/safe/simulator/callbacks.rs +++ b/simics-api/src/safe/simulator/callbacks.rs @@ -7,17 +7,29 @@ use simics_api_sys::SIM_run_alone; extern "C" fn run_alone_handler(cb: *mut c_void) where - F: FnMut(), + F: FnOnce() + 'static, { - let mut closure: Box = unsafe { Box::from_raw(cb as *mut F) }; + let closure: Box> = unsafe { Box::from_raw(cb as *mut Box) }; closure() } pub fn run_alone(cb: F) where - F: FnMut(), + F: FnOnce() + 'static, { let cb = Box::new(cb); - let cb = Box::into_raw(cb); - unsafe { SIM_run_alone(Some(run_alone_handler::), cb as *mut _ as *mut c_void) } + let cb_box = Box::new(cb); + let cb_raw = Box::into_raw(cb_box); + + debug_assert!( + std::mem::size_of_val(&cb_raw) == std::mem::size_of::<*mut std::ffi::c_void>(), + "Pointer is not convertible to *mut c_void" + ); + + unsafe { + SIM_run_alone( + Some(run_alone_handler::), + cb_raw as *mut _ as *mut c_void, + ) + } } diff --git a/simics-fuzz/src/fuzzer/mod.rs b/simics-fuzz/src/fuzzer/mod.rs index c7184638..1ff94668 100644 --- a/simics-fuzz/src/fuzzer/mod.rs +++ b/simics-fuzz/src/fuzzer/mod.rs @@ -309,12 +309,22 @@ impl SimicsFuzzer { let tx = Box::new(make_attr_data_adopt(tx)?); let rx = Box::new(make_attr_data_adopt(rx)?); - let tx = Box::into_raw(tx); - let rx = Box::into_raw(rx); + let tx_raw = Box::into_raw(tx); + let rx_raw = Box::into_raw(rx); + + debug_assert!( + std::mem::size_of_val(&tx_raw) == std::mem::size_of::<*mut std::ffi::c_void>(), + "Pointer is not convertible to *mut c_void" + ); + + debug_assert!( + std::mem::size_of_val(&rx_raw) == std::mem::size_of::<*mut std::ffi::c_void>(), + "Pointer is not convertible to *mut c_void" + ); info!("Setting up channels"); - (unsafe { *tsffs_interface }.add_channels)(tsffs, tx, rx); + (unsafe { *tsffs_interface }.add_channels)(tsffs, tx_raw, rx_raw); info!("Set channel for object");