Skip to content

Commit

Permalink
Fix pointer arithmetic around FileOperationInMirrordBinTempDir
Browse files Browse the repository at this point in the history
  • Loading branch information
gememma committed Jun 14, 2024
1 parent 9b34df6 commit d39cb61
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 19 deletions.
9 changes: 5 additions & 4 deletions mirrord/layer/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@ use std::{ffi::CStr, fmt::Debug, path::PathBuf};
use libc::c_char;
use mirrord_intproxy_protocol::{IsLayerRequest, IsLayerRequestWithResponse, MessageId};
use mirrord_protocol::file::OpenOptionsInternal;
<<<<<<< HEAD
=======
#[cfg(target_os = "macos")]
use mirrord_sip::{MIRRORD_TEMP_BIN_DIR_CANONIC_PATHBUF, MIRRORD_TEMP_BIN_DIR_PATH_BUF};
>>>>>>> b6d57201 (Fix type errors in non-SIP crates)
use tracing::warn;

use crate::{
Expand Down Expand Up @@ -113,7 +110,11 @@ impl CheckedInto<PathBuf> for *const c_char {
// `stripped_path` is a reference to a later character in the same string as
// `path_str`, `stripped_path.as_ptr()` returns a pointer to a later index
// in the same string owned by the caller (the hooked program).
Detour::Bypass(Bypass::FileOperationInMirrordBinTempDir(stripped_path))
let prefix_len = path_str.len() - stripped_path.to_string_lossy().len();
Detour::Bypass(Bypass::FileOperationInMirrordBinTempDir((
prefix_len,
stripped_path,
)))
} else {
Detour::Success(path_str) // strip is None, path not in temp dir.
}
Expand Down
6 changes: 4 additions & 2 deletions mirrord/layer/src/detour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ pub(crate) enum Bypass {
CStrConversion,

/// We hooked a file operation on a path in mirrord's bin directory. So do the operation
/// locally, but on the original path, not the one in mirrord's dir.
/// locally, but on the original path, not the one in mirrord's dir. The offset from the full
/// path to the original path is needed for pointer arithmetic, as a pointer to the String is
/// no longer possible.
#[cfg(target_os = "macos")]
FileOperationInMirrordBinTempDir(PathBuf),
FileOperationInMirrordBinTempDir((usize, PathBuf)),

/// File [`PathBuf`] should be ignored (used for tests).
IgnoredFile(PathBuf),
Expand Down
16 changes: 5 additions & 11 deletions mirrord/layer/src/exec_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,9 @@ use std::{

use libc::{c_char, c_int, pid_t};
use mirrord_layer_macro::{hook_fn, hook_guard_fn};
<<<<<<< HEAD
use mirrord_sip::{sip_patch, SipError, MIRRORD_PATCH_DIR};
=======
use mirrord_sip::{
sip_patch, SipError, MIRRORD_TEMP_BIN_DIR_CANONIC_PATHBUF, MIRRORD_TEMP_BIN_DIR_PATH_BUF,
};
>>>>>>> b6d57201 (Fix type errors in non-SIP crates)
use null_terminated::Nul;
use tracing::{trace, warn};

Expand Down Expand Up @@ -322,16 +318,14 @@ pub(crate) unsafe extern "C" fn _nsget_executable_path_detour(
let res = FN__NSGET_EXECUTABLE_PATH(path, buflen);
if res == 0 {
let path_buf_detour = CheckedInto::<PathBuf>::checked_into(path as *const c_char);
if let Bypass(FileOperationInMirrordBinTempDir(path_buf)) = path_buf_detour {
let later_ptr: *const i8 = path_buf.as_os_str().as_encoded_bytes().as_ptr() as _;
if let Bypass(FileOperationInMirrordBinTempDir((prefix_len, path_buf))) = path_buf_detour {
// SAFETY: If we're here, the original function was passed this pointer and was
// successful, so this pointer must be valid.
let old_len = *buflen;

// SAFETY: `later_ptr` is a pointer to a later char in the same buffer.
let prefix_len = later_ptr.offset_from(path);
let later_ptr = path.wrapping_add(prefix_len);

let stripped_len = old_len - prefix_len as u32;
let stripped_len = path_buf.to_string_lossy().len() as u32;

// SAFETY:
// - can read `stripped_len` bytes from `path_cstring` because it's its length.
Expand Down Expand Up @@ -366,9 +360,9 @@ pub(crate) unsafe extern "C" fn dlopen_detour(
// we hold the guard manually for tracing/internal code
let guard = crate::detour::DetourGuard::new();
let detour: Detour<PathBuf> = raw_path.checked_into();
let raw_path = if let Bypass(FileOperationInMirrordBinTempDir(path_buf)) = detour {
let raw_path = if let Bypass(FileOperationInMirrordBinTempDir((prefix_len, _))) = detour {
trace!("dlopen called with a path inside our patch dir, switching with fixed pointer.");
path_buf.as_os_str().as_encoded_bytes().as_ptr() as _
raw_path.wrapping_add(prefix_len)
} else {
trace!("dlopen called on path {detour:?}.");
raw_path
Expand Down
6 changes: 4 additions & 2 deletions mirrord/layer/src/file/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ fn update_ptr_from_bypass(ptr: *const c_char, bypass: Bypass) -> *const c_char {
// inside mirrord's temp bin dir. The detour has returned us the original path of the file
// (stripped mirrord's dir path), so now we carry out the operation locally, on the stripped
// path.
Bypass::FileOperationInMirrordBinTempDir(path_buf) => {
path_buf.as_os_str().as_encoded_bytes().as_ptr() as _
Bypass::FileOperationInMirrordBinTempDir((prefix_len, path_buf)) => {
// add length of path to strip to ptr to obtain new pointer
dbg!("{:?}", path_buf.clone());
ptr.wrapping_add(prefix_len)
}
_ => ptr,
}
Expand Down

0 comments on commit d39cb61

Please sign in to comment.