From 374f8fb5fbd698c2e5719076c82224fe06542d18 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com> Date: Thu, 29 Sep 2022 14:58:49 +0100 Subject: Rust soundness fixes (#7518) * Rust soundness fixes * Second pass * Make init_from_table unsafe * Remove SafeSliceAccess * Clippy * Remove create_vector_of_strings * More clippy * Remove deprecated root type accessors * More soundness fixes * Fix EndianScalar for bool * Add TriviallyTransmutable * Add debug assertions * Review comments * Review feedback --- .../other_name_space/from_include_generated.rs | 20 +++++------ .../my_game/other_name_space/table_b_generated.rs | 11 +++--- .../my_game/other_name_space/unused_generated.rs | 40 +++++++++------------- tests/include_test1/table_a_generated.rs | 11 +++--- 4 files changed, 39 insertions(+), 43 deletions(-) (limited to 'tests/include_test1') diff --git a/tests/include_test1/my_game/other_name_space/from_include_generated.rs b/tests/include_test1/my_game/other_name_space/from_include_generated.rs index 3c5165d1..70cb407c 100644 --- a/tests/include_test1/my_game/other_name_space/from_include_generated.rs +++ b/tests/include_test1/my_game/other_name_space/from_include_generated.rs @@ -51,10 +51,8 @@ impl core::fmt::Debug for FromInclude { impl<'a> flatbuffers::Follow<'a> for FromInclude { type Inner = Self; #[inline] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { - let b = unsafe { - flatbuffers::read_scalar_at::(buf, loc) - }; + unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + let b = flatbuffers::read_scalar_at::(buf, loc); Self(b) } } @@ -62,21 +60,21 @@ impl<'a> flatbuffers::Follow<'a> for FromInclude { impl flatbuffers::Push for FromInclude { type Output = FromInclude; #[inline] - fn push(&self, dst: &mut [u8], _rest: &[u8]) { - unsafe { flatbuffers::emplace_scalar::(dst, self.0); } + unsafe fn push(&self, dst: &mut [u8], _written_len: usize) { + flatbuffers::emplace_scalar::(dst, self.0); } } impl flatbuffers::EndianScalar for FromInclude { + type Scalar = i64; #[inline] - fn to_little_endian(self) -> Self { - let b = i64::to_le(self.0); - Self(b) + fn to_little_endian(self) -> i64 { + self.0.to_le() } #[inline] #[allow(clippy::wrong_self_convention)] - fn from_little_endian(self) -> Self { - let b = i64::from_le(self.0); + fn from_little_endian(v: i64) -> Self { + let b = i64::from_le(v); Self(b) } } diff --git a/tests/include_test1/my_game/other_name_space/table_b_generated.rs b/tests/include_test1/my_game/other_name_space/table_b_generated.rs index da7b9378..5652195b 100644 --- a/tests/include_test1/my_game/other_name_space/table_b_generated.rs +++ b/tests/include_test1/my_game/other_name_space/table_b_generated.rs @@ -19,8 +19,8 @@ pub struct TableB<'a> { impl<'a> flatbuffers::Follow<'a> for TableB<'a> { type Inner = TableB<'a>; #[inline] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { - Self { _tab: flatbuffers::Table { buf, loc } } + unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + Self { _tab: flatbuffers::Table::new(buf, loc) } } } @@ -32,7 +32,7 @@ impl<'a> TableB<'a> { } #[inline] - pub fn init_from_table(table: flatbuffers::Table<'a>) -> Self { + pub unsafe fn init_from_table(table: flatbuffers::Table<'a>) -> Self { TableB { _tab: table } } #[allow(unused_mut)] @@ -56,7 +56,10 @@ impl<'a> TableB<'a> { #[inline] pub fn a(&self) -> Option> { - self._tab.get::>(TableB::VT_A, None) + // Safety: + // Created from valid Table for this object + // which contains a valid value in this slot + unsafe { self._tab.get::>(TableB::VT_A, None)} } } diff --git a/tests/include_test1/my_game/other_name_space/unused_generated.rs b/tests/include_test1/my_game/other_name_space/unused_generated.rs index 1e4ad9c7..69be4869 100644 --- a/tests/include_test1/my_game/other_name_space/unused_generated.rs +++ b/tests/include_test1/my_game/other_name_space/unused_generated.rs @@ -27,39 +27,25 @@ impl core::fmt::Debug for Unused { } impl flatbuffers::SimpleToVerifyInSlice for Unused {} -impl flatbuffers::SafeSliceAccess for Unused {} impl<'a> flatbuffers::Follow<'a> for Unused { type Inner = &'a Unused; #[inline] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { <&'a Unused>::follow(buf, loc) } } impl<'a> flatbuffers::Follow<'a> for &'a Unused { type Inner = &'a Unused; #[inline] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { flatbuffers::follow_cast_ref::(buf, loc) } } impl<'b> flatbuffers::Push for Unused { type Output = Unused; #[inline] - fn push(&self, dst: &mut [u8], _rest: &[u8]) { - let src = unsafe { - ::core::slice::from_raw_parts(self as *const Unused as *const u8, Self::size()) - }; - dst.copy_from_slice(src); - } -} -impl<'b> flatbuffers::Push for &'b Unused { - type Output = Unused; - - #[inline] - fn push(&self, dst: &mut [u8], _rest: &[u8]) { - let src = unsafe { - ::core::slice::from_raw_parts(*self as *const Unused as *const u8, Self::size()) - }; + unsafe fn push(&self, dst: &mut [u8], _written_len: usize) { + let src = ::core::slice::from_raw_parts(self as *const Unused as *const u8, Self::size()); dst.copy_from_slice(src); } } @@ -89,24 +75,30 @@ impl<'a> Unused { } pub fn a(&self) -> i32 { - let mut mem = core::mem::MaybeUninit::::uninit(); - unsafe { + let mut mem = core::mem::MaybeUninit::<::Scalar>::uninit(); + // Safety: + // Created from a valid Table for this object + // Which contains a valid value in this slot + EndianScalar::from_little_endian(unsafe { core::ptr::copy_nonoverlapping( self.0[0..].as_ptr(), mem.as_mut_ptr() as *mut u8, - core::mem::size_of::(), + core::mem::size_of::<::Scalar>(), ); mem.assume_init() - }.from_little_endian() + }) } pub fn set_a(&mut self, x: i32) { let x_le = x.to_little_endian(); + // Safety: + // Created from a valid Table for this object + // Which contains a valid value in this slot unsafe { core::ptr::copy_nonoverlapping( - &x_le as *const i32 as *const u8, + &x_le as *const _ as *const u8, self.0[0..].as_mut_ptr(), - core::mem::size_of::(), + core::mem::size_of::<::Scalar>(), ); } } diff --git a/tests/include_test1/table_a_generated.rs b/tests/include_test1/table_a_generated.rs index 0e6a78de..fd979b09 100644 --- a/tests/include_test1/table_a_generated.rs +++ b/tests/include_test1/table_a_generated.rs @@ -19,8 +19,8 @@ pub struct TableA<'a> { impl<'a> flatbuffers::Follow<'a> for TableA<'a> { type Inner = TableA<'a>; #[inline] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { - Self { _tab: flatbuffers::Table { buf, loc } } + unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + Self { _tab: flatbuffers::Table::new(buf, loc) } } } @@ -32,7 +32,7 @@ impl<'a> TableA<'a> { } #[inline] - pub fn init_from_table(table: flatbuffers::Table<'a>) -> Self { + pub unsafe fn init_from_table(table: flatbuffers::Table<'a>) -> Self { TableA { _tab: table } } #[allow(unused_mut)] @@ -56,7 +56,10 @@ impl<'a> TableA<'a> { #[inline] pub fn b(&self) -> Option> { - self._tab.get::>(TableA::VT_B, None) + // Safety: + // Created from valid Table for this object + // which contains a valid value in this slot + unsafe { self._tab.get::>(TableA::VT_B, None)} } } -- cgit v1.2.3