Add rmdir support (#2985)

* Add rmdir support

* Fix lint

* Fix rmdir.c

* Fix rmdir on go

* SYS_rmdir doesn't exist on aarch64

* Add unlink/unlinkat support

* Check protocol supported version on unlink/unlinkat requests

* Revert message_bus.send

* PR review

* Fix && fmt

* lint

* PR comments

changelog.d/2221.added.md

@@ -0,0 +1 @@
+Add rmdir / unlink / unlinkat support  

mirrord/agent/src/file.rs

@@ -5,13 +5,17 @@ use std::{
     io::{self, prelude::*, BufReader, SeekFrom},
     iter::{Enumerate, Peekable},
     ops::RangeInclusive,
-    os::unix::{fs::MetadataExt, prelude::FileExt},
+    os::{
+        fd::RawFd,
+        unix::{fs::MetadataExt, prelude::FileExt},
+    },
     path::{Path, PathBuf},
 };
 
 use faccess::{AccessMode, PathExt};
 use libc::DT_DIR;
 use mirrord_protocol::{file::*, FileRequest, FileResponse, RemoteResult, ResponseError};
+use nix::unistd::UnlinkatFlags;
 use tracing::{error, trace, Level};
 
 use crate::error::Result;
@@ -258,6 +262,17 @@ impl FileManager {
                 pathname,
                 mode,
             }) => Some(FileResponse::MakeDir(self.mkdirat(dirfd, &pathname, mode))),
+            FileRequest::RemoveDir(RemoveDirRequest { pathname }) => {
+                Some(FileResponse::RemoveDir(self.rmdir(&pathname)))
+            }
+            FileRequest::Unlink(UnlinkRequest { pathname }) => {
+                Some(FileResponse::Unlink(self.unlink(&pathname)))
+            }
+            FileRequest::UnlinkAt(UnlinkAtRequest {
+                dirfd,
+                pathname,
+                flags,
+            }) => Some(FileResponse::Unlink(self.unlinkat(dirfd, &pathname, flags))),
         })
     }
 
@@ -520,6 +535,55 @@ impl FileManager {
         }
     }
 
+    #[tracing::instrument(level = Level::TRACE, skip(self))]
+    pub(crate) fn rmdir(&mut self, path: &Path) -> RemoteResult<()> {
+        let path = resolve_path(path, &self.root_path)?;
+
+        std::fs::remove_dir(path.as_path()).map_err(ResponseError::from)
+    }
+
+    #[tracing::instrument(level = Level::TRACE, skip(self))]
+    pub(crate) fn unlink(&mut self, path: &Path) -> RemoteResult<()> {
+        let path = resolve_path(path, &self.root_path)?;
+
+        nix::unistd::unlink(path.as_path())
+            .map_err(|error| ResponseError::from(std::io::Error::from_raw_os_error(error as i32)))
+    }
+
+    #[tracing::instrument(level = Level::TRACE, skip(self))]
+    pub(crate) fn unlinkat(
+        &mut self,
+        dirfd: Option<u64>,
+        path: &Path,
+        flags: u32,
+    ) -> RemoteResult<()> {
+        let path = match dirfd {
+            Some(dirfd) => {
+                let relative_dir = self
+                    .open_files
+                    .get(&dirfd)
+                    .ok_or(ResponseError::NotFound(dirfd))?;
+
+                if let RemoteFile::Directory(relative_dir) = relative_dir {
+                    relative_dir.join(path)
+                } else {
+                    return Err(ResponseError::NotDirectory(dirfd));
+                }
+            }
+            None => resolve_path(path, &self.root_path)?,
+        };
+
+        let flags = match flags {
+            0 => UnlinkatFlags::RemoveDir,
+            _ => UnlinkatFlags::NoRemoveDir,
+        };
+
+        let fd: Option<RawFd> = dirfd.map(|fd| fd as RawFd);
+
+        nix::unistd::unlinkat(fd, path.as_path(), flags)
+            .map_err(|error| ResponseError::from(std::io::Error::from_raw_os_error(error as i32)))
+    }
+
     pub(crate) fn seek(&mut self, fd: u64, seek_from: SeekFrom) -> RemoteResult<SeekFileResponse> {
         trace!(
             "FileManager::seek -> fd {:#?} | seek_from {:#?}",

mirrord/intproxy/protocol/src/lib.rs

@@ -324,6 +324,27 @@ impl_request!(
     res_path = ProxyToLayerMessage::File => FileResponse::MakeDir,
 );
 
+impl_request!(
+    req = RemoveDirRequest,
+    res = RemoteResult<()>,
+    req_path = LayerToProxyMessage::File => FileRequest::RemoveDir,
+    res_path = ProxyToLayerMessage::File => FileResponse::RemoveDir,
+);
+
+impl_request!(
+    req = UnlinkRequest,
+    res = RemoteResult<()>,
+    req_path = LayerToProxyMessage::File => FileRequest::Unlink,
+    res_path = ProxyToLayerMessage::File => FileResponse::Unlink,
+);
+
+impl_request!(
+    req = UnlinkAtRequest,
+    res = RemoteResult<()>,
+    req_path = LayerToProxyMessage::File => FileRequest::UnlinkAt,
+    res_path = ProxyToLayerMessage::File => FileResponse::Unlink,
+);
+
 impl_request!(
     req = SeekFileRequest,
     res = RemoteResult<SeekFileResponse>,

mirrord/intproxy/src/proxies/files.rs

@@ -6,7 +6,7 @@ use mirrord_protocol::{
     file::{
         CloseDirRequest, CloseFileRequest, DirEntryInternal, ReadDirBatchRequest, ReadDirResponse,
         ReadFileResponse, ReadLimitedFileRequest, SeekFromInternal, MKDIR_VERSION,
-        READDIR_BATCH_VERSION, READLINK_VERSION,
+        READDIR_BATCH_VERSION, READLINK_VERSION, RMDIR_VERSION,
     },
     ClientMessage, DaemonMessage, ErrorKindInternal, FileRequest, FileResponse, RemoteIOError,
     ResponseError,
@@ -253,6 +253,31 @@ impl FilesProxy {
         self.protocol_version.replace(version);
     }
 
+    /// Checks if the mirrord protocol version supports this [`FileRequest`].
+    fn is_request_supported(&self, request: &FileRequest) -> Result<(), FileResponse> {
+        let protocol_version = self.protocol_version.as_ref();
+
+        match request {
+            FileRequest::ReadLink(..)
+                if protocol_version.is_some_and(|version| !READLINK_VERSION.matches(version)) =>
+            {
+                Err(FileResponse::ReadLink(Err(ResponseError::NotImplemented)))
+            }
+            FileRequest::MakeDir(..) | FileRequest::MakeDirAt(..)
+                if protocol_version.is_some_and(|version| !MKDIR_VERSION.matches(version)) =>
+            {
+                Err(FileResponse::MakeDir(Err(ResponseError::NotImplemented)))
+            }
+            FileRequest::RemoveDir(..) | FileRequest::Unlink(..) | FileRequest::UnlinkAt(..)
+                if protocol_version
+                    .is_some_and(|version: &Version| !RMDIR_VERSION.matches(version)) =>
+            {
+                Err(FileResponse::RemoveDir(Err(ResponseError::NotImplemented)))
+            }
+            _ => Ok(()),
+        }
+    }
+
     // #[tracing::instrument(level = Level::TRACE, skip(message_bus))]
     async fn file_request(
         &mut self,
@@ -261,6 +286,18 @@ impl FilesProxy {
         message_id: MessageId,
         message_bus: &mut MessageBus<Self>,
     ) {
+        // Not supported in old `mirrord-protocol` versions.
+        if let Err(response) = self.is_request_supported(&request) {
+            message_bus
+                .send(ToLayer {
+                    message_id,
+                    layer_id,
+                    message: ProxyToLayerMessage::File(response),
+                })
+                .await;
+            return;
+        }
+
         match request {
             // Should trigger remote close only when the fd is closed in all layer instances.
             FileRequest::Close(close) => {
@@ -454,31 +491,6 @@ impl FilesProxy {
                 }
             },
 
-            // Not supported in old `mirrord-protocol` versions.
-            req @ FileRequest::ReadLink(..) => {
-                let supported = self
-                    .protocol_version
-                    .as_ref()
-                    .is_some_and(|version| READLINK_VERSION.matches(version));
-
-                if supported {
-                    self.request_queue.push_back(message_id, layer_id);
-                    message_bus
-                        .send(ProxyMessage::ToAgent(ClientMessage::FileRequest(req)))
-                        .await;
-                } else {
-                    message_bus
-                        .send(ToLayer {
-                            message_id,
-                            message: ProxyToLayerMessage::File(FileResponse::ReadLink(Err(
-                                ResponseError::NotImplemented,
-                            ))),
-                            layer_id,
-                        })
-                        .await;
-                }
-            }
-
             // Should only be sent from intproxy, not from the layer.
             FileRequest::ReadDirBatch(..) => {
                 unreachable!("ReadDirBatch request is never sent from the layer");
@@ -522,30 +534,6 @@ impl FilesProxy {
                     .await;
             }
 
-            FileRequest::MakeDir(_) | FileRequest::MakeDirAt(_) => {
-                let supported = self
-                    .protocol_version
-                    .as_ref()
-                    .is_some_and(|version| MKDIR_VERSION.matches(version));
-
-                if supported {
-                    self.request_queue.push_back(message_id, layer_id);
-                    message_bus
-                        .send(ProxyMessage::ToAgent(ClientMessage::FileRequest(request)))
-                        .await;
-                } else {
-                    let file_response = FileResponse::MakeDir(Err(ResponseError::NotImplemented));
-
-                    message_bus
-                        .send(ToLayer {
-                            message_id,
-                            message: ProxyToLayerMessage::File(file_response),
-                            layer_id,
-                        })
-                        .await;
-                }
-            }
-
             // Doesn't require any special logic.
             other => {
                 self.request_queue.push_back(message_id, layer_id);

mirrord/layer/src/file/hooks.rs

@@ -1088,6 +1088,43 @@ pub(crate) unsafe extern "C" fn mkdirat_detour(
         })
 }
 
+/// Hook for `libc::rmdir`.
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn rmdir_detour(pathname: *const c_char) -> c_int {
+    rmdir(pathname.checked_into())
+        .map(|()| 0)
+        .unwrap_or_bypass_with(|bypass| {
+            let raw_path = update_ptr_from_bypass(pathname, &bypass);
+            FN_RMDIR(raw_path)
+        })
+}
+
+/// Hook for `libc::unlink`.
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn unlink_detour(pathname: *const c_char) -> c_int {
+    unlink(pathname.checked_into())
+        .map(|()| 0)
+        .unwrap_or_bypass_with(|bypass| {
+            let raw_path = update_ptr_from_bypass(pathname, &bypass);
+            FN_UNLINK(raw_path)
+        })
+}
+
+/// Hook for `libc::unlinkat`.
+#[hook_guard_fn]
+pub(crate) unsafe extern "C" fn unlinkat_detour(
+    dirfd: c_int,
+    pathname: *const c_char,
+    flags: u32,
+) -> c_int {
+    unlinkat(dirfd, pathname.checked_into(), flags)
+        .map(|()| 0)
+        .unwrap_or_bypass_with(|bypass| {
+            let raw_path = update_ptr_from_bypass(pathname, &bypass);
+            FN_UNLINKAT(dirfd, raw_path, flags)
+        })
+}
+
 /// 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);
@@ -1163,7 +1200,6 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
     );
 
     replace!(hook_manager, "mkdir", mkdir_detour, FnMkdir, FN_MKDIR);
-
     replace!(
         hook_manager,
         "mkdirat",
@@ -1172,6 +1208,17 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
         FN_MKDIRAT
     );
 
+    replace!(hook_manager, "rmdir", rmdir_detour, FnRmdir, FN_RMDIR);
+
+    replace!(hook_manager, "unlink", unlink_detour, FnUnlink, FN_UNLINK);
+    replace!(
+        hook_manager,
+        "unlinkat",
+        unlinkat_detour,
+        FnUnlinkat,
+        FN_UNLINKAT
+    );
+
     replace!(hook_manager, "lseek", lseek_detour, FnLseek, FN_LSEEK);
 
     replace!(hook_manager, "write", write_detour, FnWrite, FN_WRITE);