Fix dirfd crashing on macOS by hooking dirfd (metalbear-co#2133)

* Fix dirfd crashing on macOS by hooking dirfd

* fix dirfd

changelog.d/+dirfd-crash-handled.fixed.md

@@ -0,0 +1 @@
+Fix dirfd crashing on macOS

mirrord/layer/src/file/hooks.rs

@@ -260,6 +260,13 @@ pub(crate) unsafe extern "C" fn closedir_detour(dirp: *mut DIR) -> c_int {
         .unwrap_or_bypass_with(|_| FN_CLOSEDIR(dirp))
 }
 
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn dirfd_detour(dirp: *mut DIR) -> c_int {
+    OPEN_DIRS
+        .get_fd(dirp as usize)
+        .unwrap_or_bypass_with(|_| FN_DIRFD(dirp))
+}
+
 /// Equivalent to `open_detour`, **except** when `raw_path` specifies a relative path.
 ///
 /// If `fd == AT_FDCWD`, the current working directory is used, and the behavior is the same as
@@ -958,6 +965,8 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
         FN_CLOSEDIR
     );
 
+    replace!(hook_manager, "dirfd", dirfd_detour, FnDirfd, FN_DIRFD);
+
     replace!(hook_manager, "pread", pread_detour, FnPread, FN_PREAD);
     replace!(
         hook_manager,

mirrord/layer/src/file/open_dirs.rs

@@ -9,7 +9,7 @@ use std::{
 use dashmap::DashMap;
 use mirrord_protocol::file::{CloseDirRequest, DirEntryInternal, ReadDirRequest, ReadDirResponse};
 
-use super::{DirStreamFd, RemoteFd};
+use super::{DirStreamFd, LocalFd, RemoteFd, OPEN_FILES};
 use crate::{
     common,
     detour::{Bypass, Detour},
@@ -38,10 +38,10 @@ impl OpenDirs {
     ///
     /// * `local_dir_fd` - opaque identifier
     /// * `remote_fd` - descriptor of the remote directory (received from the agent)
-    pub fn insert(&self, local_dir_fd: DirStreamFd, remote_fd: RemoteFd) {
+    pub fn insert(&self, local_dir_fd: DirStreamFd, remote_fd: RemoteFd, base_fd: LocalFd) {
         self.inner.insert(
             local_dir_fd,
-            Mutex::new(OpenDir::new(local_dir_fd, remote_fd)).into(),
+            Mutex::new(OpenDir::new(local_dir_fd, remote_fd, base_fd)).into(),
         );
     }
 
@@ -58,6 +58,19 @@ impl OpenDirs {
         guard.read_r()
     }
 
+    /// Gets fd used to open dir [`DirStreamFd`].
+    pub fn get_fd(&self, local_dir_fd: DirStreamFd) -> Detour<LocalFd> {
+        let dir = self
+            .inner
+            .get(&local_dir_fd)
+            .ok_or(Bypass::LocalDirStreamNotFound(local_dir_fd))?
+            .clone();
+
+        let guard = dir.lock().expect("lock poisoned");
+
+        Detour::Success(guard.get_base_fd())
+    }
+
     /// Reads next entry from the open directory with the given [`DirStreamFd`] and copies the data
     /// into an internal buffer. Returns a raw pointer to that buffer.
     ///
@@ -128,7 +141,7 @@ impl OpenDirs {
 
         let mut guard = dir.lock().expect("lock poisoned");
         guard.closed = true;
-
+        OPEN_FILES.remove(&guard.base_fd);
         common::make_proxy_request_no_response(CloseDirRequest {
             remote_fd: guard.remote_fd,
         })?;
@@ -141,13 +154,15 @@ struct OpenDir {
     closed: bool,
     local_fd: DirStreamFd,
     remote_fd: RemoteFd,
+    // fd used for opening the dir originally
+    base_fd: LocalFd,
     dirent: libc::dirent,
     #[cfg(target_os = "linux")]
     dirent64: libc::dirent64,
 }
 
 impl OpenDir {
-    fn new(local_fd: DirStreamFd, remote_fd: RemoteFd) -> Self {
+    fn new(local_fd: DirStreamFd, remote_fd: RemoteFd, base_fd: LocalFd) -> Self {
         #[cfg(not(target_os = "macos"))]
         let dirent = libc::dirent {
             d_ino: 0,
@@ -171,6 +186,7 @@ impl OpenDir {
             closed: false,
             local_fd,
             remote_fd,
+            base_fd,
             dirent,
             #[cfg(target_os = "linux")]
             dirent64: libc::dirent64 {
@@ -195,6 +211,10 @@ impl OpenDir {
             })??;
         Detour::Success(direntry)
     }
+
+    fn get_base_fd(&self) -> LocalFd {
+        self.base_fd
+    }
 }
 
 /// Moves [`DirEntryInternal`] content to the given [`libc::dirent`].

mirrord/layer/src/file/ops.rs

@@ -223,10 +223,9 @@ pub(crate) fn fdopendir(fd: RawFd) -> Detour<usize> {
         common::make_proxy_request_with_response(open_dir_request)??;
 
     let local_dir_fd = create_local_fake_file(remote_dir_fd)?;
-    OPEN_DIRS.insert(local_dir_fd as usize, remote_dir_fd);
+    OPEN_DIRS.insert(local_dir_fd as usize, remote_dir_fd, fd);
 
-    // According to docs, when using fdopendir, the fd is now managed by OS - i.e closed
-    OPEN_FILES.remove(&fd);
+    // Let it stay in OPEN_FILES, as some functions might use it in comibination with dirfd
 
     Detour::Success(local_dir_fd as usize)
 }

mirrord/layer/tests/issue2001.rs

@@ -54,8 +54,6 @@ async fn test_issue2001(
         ))))
         .await;
 
-    intproxy.expect_file_close(10).await;
-
     assert_eq!(
         intproxy.recv().await,
         ClientMessage::FileRequest(FileRequest::ReadDir(ReadDirRequest { remote_fd: 11 })),