From ad8c784009bfa0934815fcf7383c1ff1e2cb8cc1 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 24 Feb 2020 19:50:25 -0600 Subject: [PATCH] Return length from write_os_str_to_c_str --- src/helpers.rs | 57 ++++++++++++++++++++---------------------------- src/shims/env.rs | 2 +- src/shims/fs.rs | 12 ++++++---- 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 24adc72fa87..53facf89dce 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -474,35 +474,42 @@ fn bytes_to_os_str<'tcx, 'a>(bytes: &'a [u8]) -> InterpResult<'tcx, &'a OsStr> { } /// Helper function to write an OsStr as a null-terminated sequence of bytes, which is what - /// the Unix APIs usually handle. This function returns `Ok(false)` without trying to write if - /// `size` is not large enough to fit the contents of `os_string` plus a null terminator. It - /// returns `Ok(true)` if the writing process was successful. + /// the Unix APIs usually handle. This function returns `Ok((false, length))` without trying + /// to write if `size` is not large enough to fit the contents of `os_string` plus a null + /// terminator. It returns `Ok((true, length))` if the writing process was successful. The + /// string length returned does not include the null terminator. fn write_os_str_to_c_str( &mut self, os_str: &OsStr, scalar: Scalar, size: u64, - ) -> InterpResult<'tcx, bool> { + ) -> InterpResult<'tcx, (bool, u64)> { + #[cfg(target_os = "unix")] + fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> { + std::os::unix::ffi::OsStringExt::into_bytes(os_str) + } + #[cfg(not(target_os = "unix"))] + fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> { + // On non-unix platforms the best we can do to transform bytes from/to OS strings is to do the + // intermediate transformation into strings. Which invalidates non-utf8 paths that are actually + // valid. + os_str + .to_str() + .map(|s| s.as_bytes()) + .ok_or_else(|| err_unsup_format!("{:?} is not a valid utf-8 string", os_str).into()) + } + let bytes = os_str_to_bytes(os_str)?; // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null // terminator to memory using the `ptr` pointer would cause an out-of-bounds access. - if size <= bytes.len() as u64 { - return Ok(false); + let string_length = bytes.len() as u64; + if size <= string_length { + return Ok((false, string_length)); } self.eval_context_mut() .memory .write_bytes(scalar, bytes.iter().copied().chain(iter::once(0u8)))?; - Ok(true) - } - - /// Helper function to determine how long an OsStr would be as a C string, not including the - /// null terminator. - fn os_str_length_as_c_str( - &mut self, - os_str: &OsStr, - ) -> InterpResult<'tcx, usize> { - let bytes = os_str_to_bytes(os_str)?; - Ok(bytes.len()) + Ok((true, string_length)) } fn alloc_os_str_as_c_str( @@ -520,22 +527,6 @@ fn alloc_os_str_as_c_str( } } -#[cfg(target_os = "unix")] -fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> { - std::os::unix::ffi::OsStringExt::into_bytes(os_str) -} - -#[cfg(not(target_os = "unix"))] -fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> { - // On non-unix platforms the best we can do to transform bytes from/to OS strings is to do the - // intermediate transformation into strings. Which invalidates non-utf8 paths that are actually - // valid. - os_str - .to_str() - .map(|s| s.as_bytes()) - .ok_or_else(|| err_unsup_format!("{:?} is not a valid utf-8 string", os_str).into()) -} - pub fn immty_from_int_checked<'tcx>( int: impl Into, layout: TyLayout<'tcx>, diff --git a/src/shims/env.rs b/src/shims/env.rs index 96174010441..3fd895576e5 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -124,7 +124,7 @@ fn getcwd( // If we cannot get the current directory, we return null match env::current_dir() { Ok(cwd) => { - if this.write_os_str_to_c_str(&OsString::from(cwd), buf, size)? { + if this.write_os_str_to_c_str(&OsString::from(cwd), buf, size)?.0 { return Ok(buf); } let erange = this.eval_libc("ERANGE")?; diff --git a/src/shims/fs.rs b/src/shims/fs.rs index a3ac59bdf6e..cbdaf9a65b7 100644 --- a/src/shims/fs.rs +++ b/src/shims/fs.rs @@ -906,7 +906,10 @@ fn linux_readdir64_r( let name_place = this.mplace_field(entry_place, 4)?; let file_name = dir_entry.file_name(); - let name_fits = this.write_os_str_to_c_str(&file_name, name_place.ptr, name_place.layout.size.bytes())?; + let (name_fits, _) = this.write_os_str_to_c_str( + &file_name, name_place.ptr, + name_place.layout.size.bytes(), + )?; if !name_fits { throw_unsup_format!("A directory entry had a name too large to fit in libc::dirent64"); } @@ -990,7 +993,10 @@ fn macos_readdir_r( let name_place = this.mplace_field(entry_place, 5)?; let file_name = dir_entry.file_name(); - let name_fits = this.write_os_str_to_c_str(&file_name, name_place.ptr, name_place.layout.size.bytes())?; + let (name_fits, file_name_len) = this.write_os_str_to_c_str( + &file_name, name_place.ptr, + name_place.layout.size.bytes(), + )?; if !name_fits { throw_unsup_format!("A directory entry had a name too large to fit in libc::dirent"); } @@ -1008,8 +1014,6 @@ fn macos_readdir_r( #[cfg(not(unix))] let ino = 0; - let file_name_len = this.os_str_length_as_c_str(&file_name)? as u128; - let file_type = this.file_type_to_d_type(dir_entry.file_type())? as u128; let imms = [ -- 2.44.0