Missing readdir and readdir64 detours (metalbear-co#2003)

* Implemented readdir and readdir64 detours

* Changelog entry

* Making clippy happy

* Fixed macos target

* Make macos happy

* macos clippy finally happy?

* Integration test added

* ...

* rebase fix

* ......

* ..............

* test broken on macos

.github/workflows/ci.yaml

@@ -300,6 +300,9 @@ jobs:
       - run: |
           cd mirrord/layer/tests/apps/issue1899
           cargo build
+      - run: |
+          cd mirrord/layer/tests/apps/issue2001
+          cargo build
       - run: ./scripts/build_c_apps.sh
       - run: cargo test --target x86_64-unknown-linux-gnu -p mirrord-layer
       - name: mirrord protocol UT
@@ -393,6 +396,9 @@ jobs:
       - run: |
           cd mirrord/layer/tests/apps/issue1899
           cargo build
+      - run: |
+          cd mirrord/layer/tests/apps/issue2001
+          cargo build
       - run: ./scripts/build_c_apps.sh
       # For the `java_temurin_sip` test.
       - uses: sdkman/sdkman-action@b1f9b696c79148b66d3d3a06f7ea801820318d0f

Cargo.toml

@@ -10,6 +10,7 @@ members = [
     "mirrord/layer/tests/apps/issue1776",
     "mirrord/layer/tests/apps/issue1776portnot53",
     "mirrord/layer/tests/apps/issue1899",
+    "mirrord/layer/tests/apps/issue2001",
     "sample/rust",
     "medschool",
     "tests",

changelog.d/2001.fixed.md

@@ -0,0 +1 @@
+Implemented missing hooks for `readdir` and `readdir64`.

mirrord/layer/src/file.rs

@@ -40,6 +40,7 @@ use crate::{
 
 pub(crate) mod filter;
 pub(crate) mod hooks;
+pub(crate) mod open_dirs;
 pub(crate) mod ops;
 
 type LocalFd = RawFd;
@@ -58,9 +59,6 @@ pub(crate) struct DirStream {
 pub(crate) static OPEN_FILES: LazyLock<DashMap<LocalFd, Arc<ops::RemoteFile>>> =
     LazyLock::new(|| DashMap::with_capacity(4));
 
-pub(crate) static OPEN_DIRS: LazyLock<DashMap<DirStreamFd, RemoteFd>> =
-    LazyLock::new(|| DashMap::with_capacity(4));
-
 /// Extension trait for [`OpenOptionsInternal`], used to convert between `libc`-ish open options and
 /// Rust's [`std::fs::OpenOptions`]
 pub(crate) trait OpenOptionsInternalExt {

mirrord/layer/src/file/hooks.rs

@@ -5,21 +5,20 @@ use core::ffi::{c_size_t, c_ssize_t};
 ///
 /// NOTICE: If a file operation fails, it might be because it depends on some `libc` function
 /// that is not being hooked (`strace` the program to check).
-use std::{ffi::CString, os::unix::io::RawFd, ptr, slice, time::Duration};
+use std::{os::unix::io::RawFd, ptr, slice, time::Duration};
 
-#[cfg(target_os = "linux")]
 use errno::{set_errno, Errno};
 use libc::{
     self, c_char, c_int, c_void, dirent, off_t, size_t, ssize_t, stat, statfs, AT_EACCESS,
-    AT_FDCWD, DIR,
+    AT_FDCWD, DIR, EINVAL,
 };
 #[cfg(target_os = "linux")]
-use libc::{dirent64, stat64, EBADF, EINVAL, ENOENT, ENOTDIR};
+use libc::{dirent64, stat64, EBADF, ENOENT, ENOTDIR};
 #[cfg(not(all(target_os = "macos", target_arch = "aarch64")))]
 use libc::{O_DIRECTORY, O_RDONLY};
 use mirrord_layer_macro::{hook_fn, hook_guard_fn};
 use mirrord_protocol::file::{
-    DirEntryInternal, FsMetadataInternal, MetadataInternal, ReadFileResponse, WriteFileResponse,
+    FsMetadataInternal, MetadataInternal, ReadFileResponse, WriteFileResponse,
 };
 #[cfg(target_os = "linux")]
 use mirrord_protocol::ResponseError::{NotDirectory, NotFound};
@@ -28,7 +27,7 @@ use tracing::trace;
 #[cfg(target_os = "linux")]
 use tracing::{error, info, warn};
 
-use super::{ops::*, OpenOptionsInternalExt};
+use super::{open_dirs, ops::*, OpenOptionsInternalExt};
 #[cfg(not(all(target_os = "macos", target_arch = "aarch64")))]
 use crate::close_layer_fd;
 #[cfg(target_os = "macos")]
@@ -39,7 +38,10 @@ use crate::{
     common::CheckedInto,
     detour::{Detour, DetourGuard},
     error::HookError,
-    file::ops::{access, lseek, open, read, write},
+    file::{
+        open_dirs::OPEN_DIRS,
+        ops::{access, lseek, open, read, write},
+    },
     hooks::HookManager,
     replace,
 };
@@ -165,81 +167,21 @@ pub(crate) unsafe extern "C" fn fdopendir_detour(fd: RawFd) -> usize {
     fdopendir(fd).unwrap_or_bypass_with(|_| FN_FDOPENDIR(fd))
 }
 
-/// Assign DirEntryInternal to dirent
-unsafe fn assign_direntry(
-    in_entry: DirEntryInternal,
-    out_entry: *mut dirent,
-    getdents: bool,
-) -> Result<(), HookError> {
-    (*out_entry).d_ino = in_entry.inode;
-    (*out_entry).d_reclen = if getdents {
-        // The structs written by the kernel for the getdents syscall do not have a fixed size.
-        in_entry.get_d_reclen64()
-    } else {
-        std::mem::size_of::<dirent>() as u16
-    };
-    (*out_entry).d_type = in_entry.file_type;
-
-    let dir_name = CString::new(in_entry.name)?;
-    let dir_name_bytes = dir_name.as_bytes_with_nul();
-    (*out_entry)
-        .d_name
-        .get_mut(..dir_name_bytes.len())
-        .expect("directory name length exceeds limit")
-        .copy_from_slice(bytemuck::cast_slice(dir_name_bytes));
-
-    #[cfg(target_os = "macos")]
-    {
-        (*out_entry).d_seekoff = in_entry.position;
-        // name length should be without null
-        (*out_entry).d_namlen = dir_name.to_bytes().len() as u16;
-    }
-
-    #[cfg(target_os = "linux")]
-    {
-        (*out_entry).d_off = in_entry.position as i64;
-    }
-    Ok(())
-}
-
-#[cfg(target_os = "linux")]
-unsafe fn assign_direntry64(
-    in_entry: DirEntryInternal,
-    out_entry: *mut dirent64,
-    getdents: bool,
-) -> Result<(), HookError> {
-    (*out_entry).d_ino = in_entry.inode;
-    (*out_entry).d_reclen = if getdents {
-        // The structs written by the kernel for the getdents syscall do not have a fixed size.
-        in_entry.get_d_reclen64()
-    } else {
-        std::mem::size_of::<dirent64>() as u16
-    };
-    (*out_entry).d_type = in_entry.file_type;
-
-    let dir_name = CString::new(in_entry.name)?;
-    let dir_name_bytes = dir_name.as_bytes_with_nul();
-    (*out_entry)
-        .d_name
-        .get_mut(..dir_name_bytes.len())
-        .expect("directory name length exceeds limit")
-        .copy_from_slice(bytemuck::cast_slice(dir_name_bytes));
-
-    (*out_entry).d_off = in_entry.position as i64;
-
-    Ok(())
-}
-
 #[hook_guard_fn]
 pub(crate) unsafe extern "C" fn readdir_r_detour(
     dirp: *mut DIR,
     entry: *mut dirent,
     result: *mut *mut dirent,
 ) -> c_int {
-    readdir_r(dirp as usize)
+    let Some(entry_ref) = entry.as_mut() else {
+        return EINVAL;
+    };
+
+    OPEN_DIRS
+        .read_r(dirp as usize)
         .map(|resp| {
             if let Some(direntry) = resp {
-                match assign_direntry(direntry, entry, false) {
+                match open_dirs::assign_direntry(direntry, entry_ref, false) {
                     Err(e) => return c_int::from(e),
                     Ok(()) => {
                         *result = entry;
@@ -262,10 +204,15 @@ pub(crate) unsafe extern "C" fn readdir64_r_detour(
     entry: *mut dirent64,
     result: *mut *mut dirent64,
 ) -> c_int {
-    readdir_r(dirp as usize)
+    let Some(entry_ref) = entry.as_mut() else {
+        return EINVAL;
+    };
+
+    OPEN_DIRS
+        .read_r(dirp as usize)
         .map(|resp| {
             if let Some(direntry) = resp {
-                match assign_direntry64(direntry, entry, false) {
+                match open_dirs::assign_direntry64(direntry, entry_ref, false) {
                     Err(e) => return c_int::from(e),
                     Ok(()) => {
                         *result = entry;
@@ -281,9 +228,36 @@ pub(crate) unsafe extern "C" fn readdir64_r_detour(
         .unwrap_or_bypass_with(|_| FN_READDIR64_R(dirp, entry, result))
 }
 
+#[cfg(target_os = "linux")]
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn readdir64_detour(dirp: *mut DIR) -> usize {
+    match OPEN_DIRS.read64(dirp as usize) {
+        Detour::Success(entry) => entry as usize,
+        Detour::Bypass(..) => FN_READDIR64(dirp),
+        Detour::Error(e) => {
+            set_errno(Errno(e.into()));
+            std::ptr::null::<dirent64>() as usize
+        }
+    }
+}
+
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn readdir_detour(dirp: *mut DIR) -> usize {
+    match OPEN_DIRS.read(dirp as usize) {
+        Detour::Success(entry) => entry as usize,
+        Detour::Bypass(..) => FN_READDIR(dirp),
+        Detour::Error(e) => {
+            set_errno(Errno(e.into()));
+            std::ptr::null::<dirent>() as usize
+        }
+    }
+}
+
 #[hook_guard_fn]
 pub(crate) unsafe extern "C" fn closedir_detour(dirp: *mut DIR) -> c_int {
-    closedir(dirp as usize).unwrap_or_bypass_with(|_| FN_CLOSEDIR(dirp))
+    OPEN_DIRS
+        .close(dirp as usize)
+        .unwrap_or_bypass_with(|_| FN_CLOSEDIR(dirp))
 }
 
 /// Equivalent to `open_detour`, **except** when `raw_path` specifies a relative path.
@@ -359,7 +333,13 @@ pub(crate) unsafe extern "C" fn getdents64_detour(
                     set_errno(Errno(EINVAL));
                     return -1;
                 }
-                match assign_direntry(dent, next, true) {
+
+                let Some(next_ref) = next.as_mut() else {
+                    set_errno(Errno(EINVAL));
+                    return -1;
+                };
+
+                match open_dirs::assign_direntry(dent, next_ref, true) {
                     Err(e) => {
                         error!(
                             "Error while trying to write remote dir entry to local buffer: {e:?}"
@@ -1070,6 +1050,21 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
             FnReaddir64_r,
             FN_READDIR64_R
         );
+        #[cfg(target_os = "linux")]
+        replace!(
+            hook_manager,
+            "readdir64",
+            readdir64_detour,
+            FnReaddir64,
+            FN_READDIR64
+        );
+        replace!(
+            hook_manager,
+            "readdir",
+            readdir_detour,
+            FnReaddir,
+            FN_READDIR
+        );
         // aarch + macOS hooks fail
         // because macOs internally calls this with pointer authentication
         // and we don't compile to arm64e yet, so it breaks.
@@ -1130,6 +1125,13 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
             FnReaddir_r,
             FN_READDIR_R
         );
+        replace!(
+            hook_manager,
+            "readdir$INODE64",
+            readdir_detour,
+            FnReaddir,
+            FN_READDIR
+        );
         replace!(
             hook_manager,
             "opendir$INODE64",