From 1422082be1629506e6c0088336a0d76874009c2b Mon Sep 17 00:00:00 2001 From: Jason White Date: Thu, 8 Feb 2024 20:50:36 -0800 Subject: [PATCH] Fix broken tests after toolchain update Summary: These tests could potentially do unaligned pointer reads because the test data is not guaranteed to be aligned. There is also now a debug assertion in `CString::from_vec_with_nul_unchecked` that ensures the NUL byte is at the end of the `Vec`. Names in dirents may be padded out to keep them aligned. Reviewed By: dtolnay Differential Revision: D53597638 fbshipit-source-id: ad6018564d6afd4e8f5de4390d63f64e6c5cadf7 --- detcore/src/dirents.rs | 42 ++++++++++++++++++----------------- detcore/src/syscalls/files.rs | 28 +++++++++++++---------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/detcore/src/dirents.rs b/detcore/src/dirents.rs index cbfc1d5..eea32df 100644 --- a/detcore/src/dirents.rs +++ b/detcore/src/dirents.rs @@ -9,41 +9,40 @@ //! Linux dirent64 helpers use std::cmp::Ordering; -use std::ffi::CString; use std::ptr; use serde::Deserialize; use serde::Serialize; #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] -pub struct Dirent64 { +pub struct Dirent64<'a> { pub(crate) ino: u64, pub(crate) off: i64, pub(crate) ty: u8, pub(crate) reclen: u16, /// null terminated c string, NB: multiple null bytes are expected! - pub(crate) name: CString, + pub(crate) name: &'a [u8], } // sort by name, but "." < ".." <= .. #[allow(clippy::non_canonical_partial_ord_impl)] -impl PartialOrd for Dirent64 { +impl<'a> PartialOrd for Dirent64<'a> { fn partial_cmp(&self, other: &Self) -> Option { - if self.name.as_bytes()[..2] == b".\0"[..] { + if self.name == b".\0" { Some(Ordering::Less) - } else if other.name.as_bytes()[..2] == b".\0"[..] { + } else if other.name == b".\0" { Some(Ordering::Greater) - } else if self.name.as_bytes()[..3] == b"..\0"[..] { + } else if self.name == b"..\0" { Some(Ordering::Less) - } else if other.name.as_bytes()[..3] == b"..\0"[..] { + } else if other.name == b"..\0" { Some(Ordering::Greater) } else { - self.name.partial_cmp(&other.name) + self.name.partial_cmp(other.name) } } } -impl Ord for Dirent64 { +impl<'a> Ord for Dirent64<'a> { fn cmp(&self, other: &Self) -> Ordering { self.partial_cmp(other).unwrap() } @@ -68,13 +67,12 @@ pub unsafe fn deserialize_dirents64(bytes: &[u8]) -> Vec { let ty = ptr::read(dirp.offset(j).cast::()); j += std::mem::size_of::() as isize; let name = std::slice::from_raw_parts(dirp.offset(j), 1 + reclen as usize - j as usize); - let name = Vec::from(name); let ent = Dirent64 { ino, off, reclen, ty, - name: CString::from_vec_with_nul_unchecked(name), + name, }; res.push(ent); k += reclen as isize; @@ -103,7 +101,7 @@ pub unsafe fn serialize_dirents64(dents: &[Dirent64], bytes: &mut [u8]) -> usize ptr::copy_nonoverlapping( ent.name.as_ptr() as *const u8, dirp.offset(j), - ent.name.as_bytes().len(), + ent.name.len(), ); k += reclen as isize; i += 1; @@ -128,7 +126,6 @@ pub unsafe fn deserialize_dirents(bytes: &[u8]) -> Vec { let reclen = ptr::read(dirp.offset(j).cast::()); j += std::mem::size_of::() as isize; let name = std::slice::from_raw_parts(dirp.offset(j), reclen as usize - j as usize); - let name = Vec::from(name); j = reclen as isize - 1; let ty = ptr::read(dirp.offset(j).cast::()); let ent = Dirent64 { @@ -136,7 +133,7 @@ pub unsafe fn deserialize_dirents(bytes: &[u8]) -> Vec { off, reclen, ty, - name: CString::from_vec_with_nul_unchecked(name), + name, }; res.push(ent); k += reclen as isize; @@ -163,7 +160,7 @@ pub unsafe fn serialize_dirents(dents: &[Dirent64], bytes: &mut [u8]) -> usize { ptr::copy_nonoverlapping( ent.name.as_ptr() as *const u8, dirp.offset(j), - ent.name.as_bytes().len(), + ent.name.len(), ); j = reclen as isize - 1; ptr::write(dirp.offset(j).cast::(), ent.ty); @@ -177,8 +174,11 @@ pub unsafe fn serialize_dirents(dents: &[Dirent64], bytes: &mut [u8]) -> usize { mod test { use super::*; + #[repr(align(8))] + struct Aligned(T); + // getdents from my homedir - const HOME_DIRENTS: &[u8] = &[ + const HOME_DIRENTS: &[u8] = &Aligned([ 0xf2, 0x5b, 0x79, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x2e, 0x00, 0x00, 0x00, 0x00, 0x04, 0x1f, 0x11, 0x79, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x2e, 0x2e, 0x00, @@ -390,10 +390,11 @@ mod test { 0x36, 0xf2, 0xde, 0x04, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x2e, 0x76, 0x69, 0x6d, 0x69, 0x6e, 0x66, 0x6f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, - ]; + ]) + .0; // getdents64 from my homedir - const HOME_DIRENTS64: &[u8] = &[ + const HOME_DIRENTS64: &[u8] = &Aligned([ 0xf2, 0x5b, 0x79, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x04, 0x2e, 0x00, 0x00, 0x00, 0x00, 0x1f, 0x11, 0x79, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x04, 0x2e, 0x2e, @@ -605,7 +606,8 @@ mod test { 0x57, 0xc8, 0xdb, 0x04, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x08, 0x2e, 0x6c, 0x65, 0x73, 0x73, 0x68, 0x73, 0x74, 0x00, 0x00, 0x00, 0x00, 0x00, - ]; + ]) + .0; #[test] fn dents_can_deserialize_serialize() { diff --git a/detcore/src/syscalls/files.rs b/detcore/src/syscalls/files.rs index 04bb8eb..ba63d13 100644 --- a/detcore/src/syscalls/files.rs +++ b/detcore/src/syscalls/files.rs @@ -966,24 +966,26 @@ impl Detcore { return Ok(0); } - let mut cached_bytes = vec![0; nb as usize]; - cached_bytes.reserve_exact(128); + let mut dents_bytes = vec![0; nb as usize]; + dents_bytes.reserve_exact(128); guest .memory() - .read_exact(dirent.cast(), cached_bytes.as_mut_slice())?; + .read_exact(dirent.cast(), dents_bytes.as_mut_slice())?; - let mut dents = unsafe { deserialize_dirents(&cached_bytes) }; + let mut dents = unsafe { deserialize_dirents(&dents_bytes) }; dents.sort(); for dent in &mut dents { let (d_ino, _) = determinize_inode(guest, dent.ino).await; dent.ino = d_ino; } - let _ = unsafe { serialize_dirents(&dents, cached_bytes.as_mut_slice()) }; + + let mut dents_bytes = vec![0; dents_bytes.len()]; + let _ = unsafe { serialize_dirents(&dents, &mut dents_bytes) }; guest .memory() - .write_exact(dirent.cast(), cached_bytes.as_slice())?; + .write_exact(dirent.cast(), dents_bytes.as_slice())?; Ok(nb) } @@ -1004,24 +1006,26 @@ impl Detcore { return Ok(0); } - let mut cached_bytes = vec![0; nb as usize]; - cached_bytes.reserve_exact(128); + let mut dents_bytes = vec![0; nb as usize]; + dents_bytes.reserve_exact(128); guest .memory() - .read_exact(dirent.cast(), cached_bytes.as_mut_slice())?; + .read_exact(dirent.cast(), dents_bytes.as_mut_slice())?; - let mut dents = unsafe { deserialize_dirents64(&cached_bytes) }; + let mut dents = unsafe { deserialize_dirents64(&dents_bytes) }; dents.sort(); for dent in &mut dents { let (d_ino, _) = determinize_inode(guest, dent.ino).await; dent.ino = d_ino; } - let _ = unsafe { serialize_dirents64(&dents, cached_bytes.as_mut_slice()) }; + + let mut dents_bytes = vec![0; dents_bytes.len()]; + let _ = unsafe { serialize_dirents64(&dents, &mut dents_bytes) }; guest .memory() - .write_exact(dirent.cast(), cached_bytes.as_slice())?; + .write_exact(dirent.cast(), dents_bytes.as_slice())?; Ok(nb) } }