add preadv and readv to fix erlang file reading (#2179)

* add preadv and readv to fix erlang file reading

* changelog

* ..

* cr

* lint

* Add readv test.

* Handle null pointer being passed to us.

* fix testS

* CR

---------

Co-authored-by: meowjesty <[email protected]>

changelog.d/2178.fixed.md

@@ -0,0 +1 @@
+add preadv and readv to fix erlang file reading

mirrord/layer/src/file/hooks.rs

@@ -9,7 +9,7 @@ use std::{ffi::CString, os::unix::io::RawFd, ptr, slice, time::Duration};
 
 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,
+    self, c_char, c_int, c_void, dirent, iovec, off_t, size_t, ssize_t, stat, statfs, AT_EACCESS,
     AT_FDCWD, DIR, EINVAL, O_DIRECTORY, O_RDONLY,
 };
 #[cfg(target_os = "linux")]
@@ -985,6 +985,76 @@ unsafe extern "C" fn realpath_darwin_extsn_detour(
 //     result
 // }
 
+fn vec_to_iovec(bytes: &[u8], iovecs: &[iovec]) {
+    let mut copied = 0;
+    let mut iov_index = 0;
+
+    while copied < bytes.len() {
+        let iov = &iovecs.get(iov_index).expect("ioevec out of bounds");
+        let read_ptr = unsafe { bytes.as_ptr().add(copied) };
+        let copy_amount = std::cmp::min(bytes.len(), iov.iov_len);
+        let out_buffer = iov.iov_base.cast();
+        unsafe { ptr::copy(read_ptr, out_buffer, copy_amount) };
+        copied += copy_amount;
+        // we trust iov_index to be in correct size since we checked it before
+        iov_index += 1;
+    }
+}
+
+/// Hook for `libc::readv`.
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn readv_detour(
+    fd: RawFd,
+    iovecs: *const iovec,
+    iovec_count: c_int,
+) -> ssize_t {
+    if iovec_count < 0 {
+        return FN_READV(fd, iovecs, iovec_count);
+    }
+
+    let iovs = (!iovecs.is_null()).then(|| slice::from_raw_parts(iovecs, iovec_count as usize));
+
+    readv(iovs)
+        .and_then(|(iovs, read_size)| Detour::Success((read(fd, read_size)?, iovs)))
+        .map(|(read_file, iovs)| {
+            let ReadFileResponse { bytes, .. } = read_file;
+
+            vec_to_iovec(&bytes, iovs);
+            // WARN: Must be careful when it comes to `EOF`, incorrect handling may appear as the
+            // `read` call being repeated.
+            ssize_t::try_from(bytes.len()).unwrap()
+        })
+        .unwrap_or_bypass_with(|_| FN_READV(fd, iovecs, iovec_count))
+}
+
+/// Hook for `libc::readv`.
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn preadv_detour(
+    fd: RawFd,
+    iovecs: *const iovec,
+    iovec_count: c_int,
+    offset: off_t,
+) -> ssize_t {
+    if iovec_count < 0 {
+        return FN_PREADV(fd, iovecs, iovec_count, offset);
+    }
+
+    let iovs = (!iovecs.is_null()).then(|| slice::from_raw_parts(iovecs, iovec_count as usize));
+
+    readv(iovs)
+        .and_then(|(iovs, read_size)| Detour::Success((pread(fd, read_size, offset as u64)?, iovs)))
+        .map(|(read_file, iovs)| {
+            let ReadFileResponse { bytes, .. } = read_file;
+
+            vec_to_iovec(&bytes, iovs);
+
+            // WARN: Must be careful when it comes to `EOF`, incorrect handling may appear as the
+            // `read` call being repeated.
+            ssize_t::try_from(bytes.len()).unwrap()
+        })
+        .unwrap_or_bypass_with(|_| FN_PREADV(fd, iovecs, iovec_count, offset))
+}
+
 /// Convenience function to setup file hooks (`x_detour`) with `frida_gum`.
 pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
     replace!(hook_manager, "open", open_detour, FnOpen, FN_OPEN);
@@ -1041,6 +1111,8 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
     replace!(hook_manager, "dirfd", dirfd_detour, FnDirfd, FN_DIRFD);
 
     replace!(hook_manager, "pread", pread_detour, FnPread, FN_PREAD);
+    replace!(hook_manager, "readv", readv_detour, FnReadv, FN_READV);
+    replace!(hook_manager, "preadv", preadv_detour, FnPreadv, FN_PREADV);
     replace!(
         hook_manager,
         "_pread$NOCANCEL",

mirrord/layer/src/file/ops.rs

@@ -1,7 +1,7 @@
 use core::ffi::CStr;
 use std::{env, ffi::CString, io::SeekFrom, os::unix::io::RawFd, path::PathBuf};
 
-use libc::{c_int, unlink, AT_FDCWD, FILE};
+use libc::{c_int, iovec, unlink, AT_FDCWD, FILE};
 use mirrord_protocol::file::{
     OpenFileRequest, OpenFileResponse, OpenOptionsInternal, ReadFileResponse, SeekFileResponse,
     WriteFileResponse, XstatFsResponse, XstatResponse,
@@ -275,6 +275,15 @@ pub(crate) fn read(local_fd: RawFd, read_amount: u64) -> Detour<ReadFileResponse
     get_remote_fd(local_fd).and_then(|remote_fd| RemoteFile::remote_read(remote_fd, read_amount))
 }
 
+/// Helper for dealing with a potential null pointer being passed to `*const iovec` from
+/// `readv_detour` and `preadv_detour`.
+pub(crate) fn readv(iovs: Option<&[iovec]>) -> Detour<(&[iovec], u64)> {
+    let iovs = iovs?;
+    let read_size: u64 = iovs.iter().fold(0, |sum, iov| sum + iov.iov_len as u64);
+
+    Detour::Success((iovs, read_size))
+}
+
 #[tracing::instrument(level = "trace")]
 pub(crate) fn pread(local_fd: RawFd, buffer_size: u64, offset: u64) -> Detour<ReadFileResponse> {
     // We're only interested in files that are paired with mirrord-agent.

mirrord/layer/tests/apps/issue2178/issue2178.c

@@ -0,0 +1,60 @@
+#include <stdio.h>
+#include <fcntl.h>
+#include <sys/uio.h>
+#include <unistd.h>
+#include <string.h>
+#include <signal.h>
+
+
+/// Opens a test file, then tries to read (part) of it with `readv`.
+int main(int argc, char *argv[]) {
+  printf("test issue 2178: START");
+
+  char first[4], second[8];
+
+  struct iovec iov[2];
+
+  int fd = open("/app/test.txt", O_RDONLY);
+  if (fd == -1) {
+    printf("test issue 2178: FAILED");
+    perror("open");
+    return 1;
+  }
+
+  memset(second, 0, 8);
+
+  iov[0].iov_base = first;
+  iov[0].iov_len = sizeof(first);
+  iov[1].iov_base = second;
+  iov[1].iov_len = sizeof(second);
+
+  ssize_t result = readv(fd, iov, 2);
+  if (result == -1) {
+    printf("test issue 2178: FAILED");
+    perror("readv");
+    return 1;
+  }
+
+
+  if (memcmp(first, "abcd", 4) != 0) {
+    printf("test issue 2178: FAILED");
+    printf("first: %s", first);
+    return 1;
+  }
+
+  if (memcmp(second, "efgh\x00\x00\x00\x00", 8) != 0) {
+    printf("test issue 2178: FAILED");
+    printf("second: %s", second);
+    return 1;
+  }
+
+  ssize_t null_result = readv(fd, NULL, 2);
+  if (null_result == -1) {
+    printf("test issue 2178: expected `readv: Bad address`");
+  }
+
+  close(fd);
+
+  printf("test issue 2178: SUCCESS");
+  return 0;
+}

mirrord/layer/tests/common/mod.rs

@@ -652,6 +652,7 @@ pub enum Application {
     Fork,
     OpenFile,
     CIssue2055,
+    CIssue2178,
     RustIssue2058,
     Realpath,
     // For running applications with the executable and arguments determined at runtime.
@@ -795,6 +796,11 @@ impl Application {
                 env!("CARGO_MANIFEST_DIR"),
                 "tests/apps/gethostbyname/out.c_test_app",
             ),
+            Application::CIssue2178 => format!(
+                "{}/{}",
+                env!("CARGO_MANIFEST_DIR"),
+                "tests/apps/issue2178/out.c_test_app",
+            ),
             Application::RustIssue2058 => String::from("tests/apps/issue2058/target/issue2058"),
             Application::DynamicApp(exe, _) => exe.clone(),
         }
@@ -887,7 +893,8 @@ impl Application {
             | Application::Go20DirBypass
             | Application::RustIssue2058
             | Application::OpenFile
-            | Application::CIssue2055 => vec![],
+            | Application::CIssue2055
+            | Application::CIssue2178 => vec![],
             Application::RustOutgoingUdp => ["--udp", RUST_OUTGOING_LOCAL, RUST_OUTGOING_PEERS]
                 .into_iter()
                 .map(Into::into)
@@ -955,6 +962,7 @@ impl Application {
             | Application::RustRecvFrom
             | Application::OpenFile
             | Application::CIssue2055
+            | Application::CIssue2178
             | Application::DynamicApp(..) => unimplemented!("shouldn't get here"),
             Application::PythonSelfConnect => 1337,
             Application::RustIssue2058 => 1234,

mirrord/layer/tests/issue2178.rs

@@ -0,0 +1,37 @@
+#![feature(assert_matches)]
+use std::{path::PathBuf, time::Duration};
+
+use rstest::rstest;
+
+mod common;
+pub use common::*;
+
+#[rstest]
+#[tokio::test]
+#[timeout(Duration::from_secs(60))]
+async fn test_issue2178(
+    #[values(Application::CIssue2178)] application: Application,
+    dylib_path: &PathBuf,
+) {
+    let (mut test_process, mut intproxy) = application
+        .start_process_with_layer(dylib_path, Default::default(), None)
+        .await;
+
+    println!("waiting for file request.");
+
+    intproxy
+        .expect_file_open_with_read_flag("/app/test.txt", 3)
+        .await;
+    assert_eq!(intproxy.expect_only_file_read(3).await, 12);
+    let file_data = "abcdefgh".as_bytes().to_vec();
+    intproxy.answer_file_read(file_data).await;
+    intproxy.expect_file_close(3).await;
+
+    test_process.wait_assert_success().await;
+    test_process
+        .assert_stdout_contains("test issue 2178: START")
+        .await;
+    test_process
+        .assert_stdout_contains("test issue 2178: SUCCESS")
+        .await;
+}