diff options
Diffstat (limited to 'src/fam.rs')
| -rw-r--r-- | src/fam.rs | 240 |
1 files changed, 167 insertions, 73 deletions
@@ -50,6 +50,8 @@ impl fmt::Display for Error { /// * the implementer should be a POD /// * the implementor should contain a flexible array member of elements of type `Entry` /// * `Entry` should be a POD +/// * the implementor should ensures that the FAM length as returned by [`FamStruct::len()`] +/// always describes correctly the length of the flexible array member. /// /// Violating these may cause problems. /// @@ -99,7 +101,7 @@ impl fmt::Display for Error { /// self.len as usize /// } /// -/// fn set_len(&mut self, len: usize) { +/// unsafe fn set_len(&mut self, len: usize) { /// self.len = len as u32 /// } /// @@ -135,7 +137,12 @@ pub unsafe trait FamStruct { /// /// These type of structures contain a member that holds the FAM length. /// This method will set the value of that member. - fn set_len(&mut self, len: usize); + /// + /// # Safety + /// + /// The caller needs to ensure that `len` here reflects the correct number of entries of the + /// flexible array part of the struct. + unsafe fn set_len(&mut self, len: usize); /// Get max allowed FAM length /// @@ -169,9 +176,14 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { /// /// Get the capacity required by mem_allocator in order to hold /// the provided number of [`FamStruct::Entry`](trait.FamStruct.html#associatedtype.Entry). - fn mem_allocator_len(fam_len: usize) -> usize { - let wrapper_size_in_bytes = size_of::<T>() + fam_len * size_of::<T::Entry>(); - (wrapper_size_in_bytes + size_of::<T>() - 1) / size_of::<T>() + /// Returns `None` if the required length would overflow usize. + fn mem_allocator_len(fam_len: usize) -> Option<usize> { + let wrapper_size_in_bytes = + size_of::<T>().checked_add(fam_len.checked_mul(size_of::<T::Entry>())?)?; + + wrapper_size_in_bytes + .checked_add(size_of::<T>().checked_sub(1)?)? + .checked_div(size_of::<T>()) } /// Convert `mem_allocator` len to FAM len. @@ -206,7 +218,8 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { return Err(Error::SizeLimitExceeded); } let required_mem_allocator_capacity = - FamStructWrapper::<T>::mem_allocator_len(num_elements); + FamStructWrapper::<T>::mem_allocator_len(num_elements) + .ok_or(Error::SizeLimitExceeded)?; let mut mem_allocator = Vec::with_capacity(required_mem_allocator_capacity); mem_allocator.push(T::default()); @@ -214,7 +227,11 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { // SAFETY: Safe as long T follows the requirements of being POD. mem_allocator.push(unsafe { mem::zeroed() }) } - mem_allocator[0].set_len(num_elements); + // SAFETY: The flexible array part of the struct has `num_elements` capacity. We just + // initialized this in `mem_allocator`. + unsafe { + mem_allocator[0].set_len(num_elements); + } Ok(FamStructWrapper { mem_allocator }) } @@ -234,7 +251,8 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { let mut adapter = FamStructWrapper::<T>::new(entries.len())?; { - let wrapper_entries = adapter.as_mut_fam_struct().as_mut_slice(); + // SAFETY: We are not modifying the length of the FamStruct + let wrapper_entries = unsafe { adapter.as_mut_fam_struct().as_mut_slice() }; wrapper_entries.copy_from_slice(entries); } @@ -271,7 +289,12 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { } /// Get a mut reference to the actual [`FamStruct`](trait.FamStruct.html) instance. - pub fn as_mut_fam_struct(&mut self) -> &mut T { + /// + /// # Safety + /// + /// Callers must not use the reference returned to modify the `len` filed of the underlying + /// `FamStruct`. See also the top-level documentation of [`FamStruct`]. + pub unsafe fn as_mut_fam_struct(&mut self) -> &mut T { &mut self.mem_allocator[0] } @@ -294,7 +317,8 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { /// Modifying the container referenced by this pointer may cause its buffer /// to be reallocated, which would also make any pointers to it invalid. pub fn as_mut_fam_struct_ptr(&mut self) -> *mut T { - self.as_mut_fam_struct() + // SAFETY: We do not change the length of the underlying FamStruct. + unsafe { self.as_mut_fam_struct() } } /// Get the elements slice. @@ -304,7 +328,8 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { /// Get the mutable elements slice. pub fn as_mut_slice(&mut self) -> &mut [T::Entry] { - self.as_mut_fam_struct().as_mut_slice() + // SAFETY: We do not change the length of the underlying FamStruct. + unsafe { self.as_mut_fam_struct() }.as_mut_slice() } /// Get the number of elements of type `FamStruct::Entry` currently in the vec. @@ -326,17 +351,20 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { /// /// If the capacity is already reserved, this method doesn't do anything. /// If not this will trigger a reallocation of the underlying buffer. - fn reserve(&mut self, additional: usize) { + fn reserve(&mut self, additional: usize) -> Result<(), Error> { let desired_capacity = self.len() + additional; if desired_capacity <= self.capacity() { - return; + return Ok(()); } let current_mem_allocator_len = self.mem_allocator.len(); - let required_mem_allocator_len = FamStructWrapper::<T>::mem_allocator_len(desired_capacity); + let required_mem_allocator_len = FamStructWrapper::<T>::mem_allocator_len(desired_capacity) + .ok_or(Error::SizeLimitExceeded)?; let additional_mem_allocator_len = required_mem_allocator_len - current_mem_allocator_len; self.mem_allocator.reserve(additional_mem_allocator_len); + + Ok(()) } /// Update the length of the FamStructWrapper. @@ -352,7 +380,10 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { /// /// When len is greater than the max possible len it returns Error::SizeLimitExceeded. fn set_len(&mut self, len: usize) -> Result<(), Error> { - let additional_elements: isize = len as isize - self.len() as isize; + let additional_elements = isize::try_from(len) + .and_then(|len| isize::try_from(self.len()).map(|self_len| len - self_len)) + .map_err(|_| Error::SizeLimitExceeded)?; + // If len == self.len there's nothing to do. if additional_elements == 0 { return Ok(()); @@ -365,11 +396,12 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { return Err(Error::SizeLimitExceeded); } // Reserve additional capacity. - self.reserve(additional_elements as usize); + self.reserve(additional_elements as usize)?; } let current_mem_allocator_len = self.mem_allocator.len(); - let required_mem_allocator_len = FamStructWrapper::<T>::mem_allocator_len(len); + let required_mem_allocator_len = + FamStructWrapper::<T>::mem_allocator_len(len).ok_or(Error::SizeLimitExceeded)?; // Update the len of the `mem_allocator`. // SAFETY: This is safe since enough capacity has been reserved. unsafe { @@ -382,7 +414,11 @@ impl<T: Default + FamStruct> FamStructWrapper<T> { self.mem_allocator[i] = unsafe { mem::zeroed() } } // Update the len of the underlying `FamStruct`. - self.as_mut_fam_struct().set_len(len); + // SAFETY: We just adjusted the memory for the underlying `mem_allocator` to hold `len` + // entries. + unsafe { + self.as_mut_fam_struct().set_len(len); + } // If the len needs to be decreased, deallocate unnecessary memory if additional_elements < 0 { @@ -445,9 +481,9 @@ impl<T: Default + FamStruct + PartialEq> PartialEq for FamStructWrapper<T> { impl<T: Default + FamStruct> Clone for FamStructWrapper<T> { fn clone(&self) -> Self { // The number of entries (self.as_slice().len()) can't be > T::max_len() since `self` is a - // valid `FamStructWrapper`. + // valid `FamStructWrapper`. This makes the .unwrap() safe. let required_mem_allocator_capacity = - FamStructWrapper::<T>::mem_allocator_len(self.as_slice().len()); + FamStructWrapper::<T>::mem_allocator_len(self.as_slice().len()).unwrap(); let mut mem_allocator = Vec::with_capacity(required_mem_allocator_capacity); @@ -469,7 +505,7 @@ impl<T: Default + FamStruct> Clone for FamStructWrapper<T> { let mut adapter = FamStructWrapper { mem_allocator }; { - let wrapper_entries = adapter.as_mut_fam_struct().as_mut_slice(); + let wrapper_entries = adapter.as_mut_slice(); wrapper_entries.copy_from_slice(self.as_slice()); } adapter @@ -527,13 +563,23 @@ where { use serde::de::Error; - let header = seq + let header: X = seq .next_element()? .ok_or_else(|| de::Error::invalid_length(0, &self))?; let entries: Vec<X::Entry> = seq .next_element()? .ok_or_else(|| de::Error::invalid_length(1, &self))?; + if header.len() != entries.len() { + let msg = format!( + "Mismatch between length of FAM specified in FamStruct header ({}) \ + and actual size of FAM ({})", + header.len(), + entries.len() + ); + return Err(V::Error::custom(msg)); + } + let mut result: Self::Value = FamStructWrapper::from_entries(entries.as_slice()) .map_err(|e| V::Error::custom(format!("{:?}", e)))?; result.mem_allocator[0] = header; @@ -557,7 +603,7 @@ macro_rules! generate_fam_struct_impl { self.$field_name as usize } - fn set_len(&mut self, len: usize) { + unsafe fn set_len(&mut self, len: usize) { self.$field_name = len as $field_type; } @@ -581,6 +627,7 @@ macro_rules! generate_fam_struct_impl { #[cfg(test)] mod tests { #![allow(clippy::undocumented_unsafe_blocks)] + #[cfg(feature = "with-serde")] use serde_derive::{Deserialize, Serialize}; @@ -589,7 +636,7 @@ mod tests { const MAX_LEN: usize = 100; #[repr(C)] - #[derive(Default, PartialEq, Eq)] + #[derive(Default, Debug, PartialEq, Eq)] pub struct __IncompleteArrayField<T>(::std::marker::PhantomData<T>, [T; 0]); impl<T> __IncompleteArrayField<T> { #[inline] @@ -678,12 +725,30 @@ mod tests { let fam_len = pair.0; let mem_allocator_len = pair.1; assert_eq!( - mem_allocator_len, + Some(mem_allocator_len), MockFamStructWrapper::mem_allocator_len(fam_len) ); } } + #[repr(C)] + #[derive(Default, PartialEq)] + struct MockFamStructU8 { + pub len: u32, + pub padding: u32, + pub entries: __IncompleteArrayField<u8>, + } + generate_fam_struct_impl!(MockFamStructU8, u8, entries, u32, len, 100); + type MockFamStructWrapperU8 = FamStructWrapper<MockFamStructU8>; + #[test] + fn test_invalid_type_conversion() { + let mut adapter = MockFamStructWrapperU8::new(10).unwrap(); + assert!(matches!( + adapter.set_len(0xffff_ffff_ffff_ff00), + Err(Error::SizeLimitExceeded) + )); + } + #[test] fn test_wrapper_len() { for pair in MEM_ALLOCATOR_LEN_TO_FAM_LEN { @@ -785,7 +850,7 @@ mod tests { let num_elements = pair.0; let required_mem_allocator_len = pair.1; - adapter.reserve(num_elements); + adapter.reserve(num_elements).unwrap(); assert!(adapter.mem_allocator.capacity() >= required_mem_allocator_len); assert_eq!(0, adapter.len()); @@ -794,7 +859,7 @@ mod tests { // test that when the capacity is already reserved, the method doesn't do anything let current_capacity = adapter.capacity(); - adapter.reserve(current_capacity - 1); + adapter.reserve(current_capacity - 1).unwrap(); assert_eq!(current_capacity, adapter.capacity()); } @@ -831,7 +896,8 @@ mod tests { assert_eq!(adapter.as_slice()[i], i as u32); assert_eq!(adapter.len(), i + 1); assert!( - adapter.mem_allocator.capacity() >= MockFamStructWrapper::mem_allocator_len(i + 1) + adapter.mem_allocator.capacity() + >= MockFamStructWrapper::mem_allocator_len(i + 1).unwrap() ); } @@ -858,7 +924,7 @@ mod tests { assert_eq!(adapter.len(), num_retained_entries); assert!( adapter.mem_allocator.capacity() - >= MockFamStructWrapper::mem_allocator_len(num_retained_entries) + >= MockFamStructWrapper::mem_allocator_len(num_retained_entries).unwrap() ); } @@ -910,7 +976,7 @@ mod tests { assert_eq!(payload[0], 0xA5); assert_eq!(payload[1], 0x1e); } - assert_eq!(wrapper.as_mut_fam_struct().padding, 5); + assert_eq!(unsafe { wrapper.as_mut_fam_struct() }.padding, 5); let data = wrapper.into_raw(); assert_eq!(data[0].len, 2); assert_eq!(data[0].padding, 5); @@ -996,53 +1062,81 @@ mod tests { type FooFamStructWrapper = FamStructWrapper<Foo>; let mut wrapper = FooFamStructWrapper::new(0).unwrap(); - wrapper.as_mut_fam_struct().index = 1; - wrapper.as_mut_fam_struct().flags = 2; - wrapper.as_mut_fam_struct().length = 3; - wrapper.push(3).unwrap(); - wrapper.push(14).unwrap(); - assert_eq!(wrapper.as_slice().len(), 3 + 2); - assert_eq!(wrapper.as_slice()[3], 3); - assert_eq!(wrapper.as_slice()[3 + 1], 14); - - let mut wrapper2 = wrapper.clone(); - assert_eq!( - wrapper.as_mut_fam_struct().index, - wrapper2.as_mut_fam_struct().index - ); - assert_eq!( - wrapper.as_mut_fam_struct().length, - wrapper2.as_mut_fam_struct().length - ); - assert_eq!( - wrapper.as_mut_fam_struct().flags, - wrapper2.as_mut_fam_struct().flags - ); - assert_eq!(wrapper.as_slice(), wrapper2.as_slice()); - assert_eq!( - wrapper2.as_slice().len(), - wrapper2.as_mut_fam_struct().length as usize - ); - assert!(wrapper == wrapper2); + // SAFETY: We do play with length here, but that's just for testing purposes :) + unsafe { + wrapper.as_mut_fam_struct().index = 1; + wrapper.as_mut_fam_struct().flags = 2; + wrapper.as_mut_fam_struct().length = 3; + wrapper.push(3).unwrap(); + wrapper.push(14).unwrap(); + assert_eq!(wrapper.as_slice().len(), 3 + 2); + assert_eq!(wrapper.as_slice()[3], 3); + assert_eq!(wrapper.as_slice()[3 + 1], 14); + + let mut wrapper2 = wrapper.clone(); + assert_eq!( + wrapper.as_mut_fam_struct().index, + wrapper2.as_mut_fam_struct().index + ); + assert_eq!( + wrapper.as_mut_fam_struct().length, + wrapper2.as_mut_fam_struct().length + ); + assert_eq!( + wrapper.as_mut_fam_struct().flags, + wrapper2.as_mut_fam_struct().flags + ); + assert_eq!(wrapper.as_slice(), wrapper2.as_slice()); + assert_eq!( + wrapper2.as_slice().len(), + wrapper2.as_mut_fam_struct().length as usize + ); + assert!(wrapper == wrapper2); - wrapper.as_mut_fam_struct().index = 3; - assert!(wrapper != wrapper2); + wrapper.as_mut_fam_struct().index = 3; + assert!(wrapper != wrapper2); - wrapper.as_mut_fam_struct().length = 7; - assert!(wrapper != wrapper2); + wrapper.as_mut_fam_struct().length = 7; + assert!(wrapper != wrapper2); - wrapper.push(1).unwrap(); - assert_eq!(wrapper.as_mut_fam_struct().length, 8); - assert!(wrapper != wrapper2); + wrapper.push(1).unwrap(); + assert_eq!(wrapper.as_mut_fam_struct().length, 8); + assert!(wrapper != wrapper2); - let mut wrapper2 = wrapper.clone(); - assert!(wrapper == wrapper2); + let mut wrapper2 = wrapper.clone(); + assert!(wrapper == wrapper2); - // Dropping the original variable should not affect its clone. - drop(wrapper); - assert_eq!(wrapper2.as_mut_fam_struct().index, 3); - assert_eq!(wrapper2.as_mut_fam_struct().length, 8); - assert_eq!(wrapper2.as_mut_fam_struct().flags, 2); - assert_eq!(wrapper2.as_slice(), [0, 0, 0, 3, 14, 0, 0, 1]); + // Dropping the original variable should not affect its clone. + drop(wrapper); + assert_eq!(wrapper2.as_mut_fam_struct().index, 3); + assert_eq!(wrapper2.as_mut_fam_struct().length, 8); + assert_eq!(wrapper2.as_mut_fam_struct().flags, 2); + assert_eq!(wrapper2.as_slice(), [0, 0, 0, 3, 14, 0, 0, 1]); + } + } + + #[cfg(feature = "with-serde")] + #[test] + fn test_bad_deserialize() { + #[repr(C)] + #[derive(Default, Debug, PartialEq, Serialize, Deserialize)] + struct Foo { + pub len: u32, + pub padding: u32, + pub entries: __IncompleteArrayField<u32>, + } + + generate_fam_struct_impl!(Foo, u32, entries, u32, len, 100); + + let state = FamStructWrapper::<Foo>::new(0).unwrap(); + let mut bytes = bincode::serialize(&state).unwrap(); + + // The `len` field of the header is the first to be serialized. + // Writing at position 0 of the serialized data should change its value. + bytes[0] = 255; + + assert!( + matches!(bincode::deserialize::<FamStructWrapper<Foo>>(&bytes).map_err(|boxed| *boxed), Err(bincode::ErrorKind::Custom(s)) if s == *"Mismatch between length of FAM specified in FamStruct header (255) and actual size of FAM (0)") + ); } } |