getditto / safer_ffi

Write safer FFI code in Rust without polluting it with unsafe code
http://getditto.github.io/safer_ffi
MIT License
925 stars 40 forks source link

Please teach me what I am doing wrong stack/heap wise in terms of Rust FFI struct allocation/deallocation #115

Closed brandonros closed 2 years ago

brandonros commented 2 years ago

I'm sorry, I just stumbled across your library and I don't know where else to ask. I've tried "ManullyDrop", mem::forget, Box, all sorts of stuff.

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct PDU_MODULE_DATA {
    pub ModuleTypeId: UNUM32,
    pub hMod: UNUM32,
    pub pVendorModuleName: *const CHAR8,
    pub pVendorAdditionalInfo: *const CHAR8,
    pub ModuleStatus: T_PDU_STATUS,
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct PDU_MODULE_ITEM {
    pub ItemType: T_PDU_IT,
    pub NumEntries: UNUM32,
    pub pModuleData: *const PDU_MODULE_DATA,
}

pub fn convert_rust_string_to_c_string(input: String) -> *const i8 {
  let mut output = Vec::from(input);
  output.push(0x00); // push \0 null terminating C string byte
  let dropped_output = std::mem::ManuallyDrop::new(output);
  let slice = dropped_output.as_slice();
  return slice.as_ptr() as *const u8 as *const i8;
}

pub fn convert_module_data_rpc_to_ffi(input: RPC_PDU_MODULE_DATA) -> PDU_MODULE_DATA {
  let pVendorModuleName = convert_rust_string_to_c_string(input.VendorModuleName.clone());
  let pVendorAdditionalInfo = convert_rust_string_to_c_string(input.VendorAdditionalInfo.clone());
  let output = Box::new(PDU_MODULE_DATA {
    ModuleTypeId: input.ModuleTypeId,
    hMod: input.hMod,
    pVendorModuleName: pVendorModuleName,
    pVendorAdditionalInfo: pVendorAdditionalInfo,
    ModuleStatus: input.ModuleStatus
  });
  return *Box::leak(output);
}

pub fn convert_module_item_rpc_to_ffi(input: RPC_PDU_MODULE_ITEM) -> PDU_MODULE_ITEM {
  let mut moduleDataArray: Vec<PDU_MODULE_DATA> = Vec::new();
  for i in 0..input.NumEntries as usize {
    moduleDataArray.push(convert_module_data_rpc_to_ffi(input.ModuleData[i].clone()));
  }
  let output = Box::new(PDU_MODULE_ITEM {
    ItemType: input.ItemType,
    NumEntries: input.NumEntries,
    pModuleData: moduleDataArray.as_slice().as_ptr()
  });
  return *Box::leak(output);
}

What can I do to allocate a struct in a function, return it, and the allocator not like... just 100% destroy it and cause a bunch of segfaults/corruption?

danielhenrymantilla commented 2 years ago

Hey, sorry, I have been busy these past days. Yes, there are some things to do to clean up your code, I may write a detailed post within the following days.

Right now the main offender is the moduleDataArray which is dropped at the end of convert_module_item_rpc_to_ffi(). In general, rather than Box::leak and/or forget, try to look for into_raw-named methods in the standard library:

A key thing for these methods is that they consume ownership (conceptually relinquishing ownership) of their input, thereby guaranteeing a lack of drop / segfault etc. (you'd later have to give the ownership back to avoid memory leaks, but while focusing on crashes caused by use-after-frees, that is not the priority):

pub fn convert_module_item_rpc_to_ffi(input: RPC_PDU_MODULE_ITEM) -> PDU_MODULE_ITEM {
  let mut moduleDataArray: Vec<PDU_MODULE_DATA> = Vec::new();
  for i in 0..input.NumEntries as usize {
    moduleDataArray.push(convert_module_data_rpc_to_ffi(input.ModuleData[i].clone()));
  }
  let output = Box::new(PDU_MODULE_ITEM {
    ItemType: input.ItemType,
    NumEntries: input.NumEntries,
-   pModuleData: moduleDataArray.as_slice().as_ptr()
+   pModuleData: Box::into_raw(moduleDataArray.into_boxed_slice()) as *mut _,
  });
  return *Box::leak(output);
}
danielhenrymantilla commented 2 years ago

If you want to keep discussing about this, feel free to head over the Discussions section of the repo 🙂