Fix more syscall

Cargo.toml

@@ -16,5 +16,8 @@ bstr = "0.2.15"
 log = "0.4.14"
 env_logger = "0.8.3"
 
+[dev-dependencies]
+nc = "0.6.2"
+
 [build-dependencies]
 gcc = { git = "https://github.com/vincenthage/gcc-rs", branch = "master" }

src/errors.rs

@@ -20,6 +20,7 @@ pub struct Error {
     source: Option<Box<dyn std::error::Error>>,
 }
 
+#[allow(dead_code)]
 impl Error {
     /// Create an Error with a unknown errno
     pub fn unknown() -> Self {
@@ -77,6 +78,7 @@ impl Error {
     }
 }
 
+#[allow(dead_code)]
 impl Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "Error with {}({})", self.errno, self.errno as i32)?;
@@ -91,6 +93,7 @@ impl Display for Error {
     }
 }
 
+#[allow(dead_code)]
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut d = f.debug_struct("Error");
@@ -160,6 +163,7 @@ impl From<NixError> for Error {
 /// `Result<T,E>`, In addition, it also allows appending an `errno` value.
 ///
 /// [`anyhow::Context`]: https://docs.rs/anyhow/1.0.40/anyhow/trait.Context.html
+#[allow(dead_code)]
 pub trait WithContext<T> {
     fn errno(self, errno: Errno) -> Result<T>;
 
@@ -173,6 +177,7 @@ pub trait WithContext<T> {
         F: FnOnce() -> C;
 }
 
+#[allow(dead_code)]
 impl<T, E> WithContext<T> for result::Result<T, E>
 where
     Error: From<E>,

src/filesystem/binding.rs

@@ -99,7 +99,6 @@ impl Binding {
 mod tests {
     use super::Side::{Guest, Host};
     use super::*;
-    use crate::utils::tests::get_test_rootfs_path;
     use std::path::PathBuf;
 
     #[test]

src/filesystem/canonicalization.rs

@@ -17,7 +17,9 @@ impl Canonicalizer for FileSystem {
     /// symlinks. It checks that every path of the path exists.
     /// The result is a canonicalized path on the `Guest` side.
     ///
-    /// The final path is only deferenced if `deref_final` is true.
+    /// The final component can be a path that does not exist. The final
+    /// component is only deferenced if `deref_final` is true and path is
+    /// existing.
     ///
     /// # Paramters
     ///
@@ -28,6 +30,25 @@ impl Canonicalizer for FileSystem {
     ///
     /// guest_path_new: the canonicalized user_path, which is a path in the view
     /// of Guest
+    ///
+    /// # Note
+    ///
+    /// The current implementation performs a slight normalization on
+    /// `guest_path` in advance, which is caused by `Path::components()`
+    /// function. This means that `"/foo/bar/."` is equivalent to
+    /// `"/foo/bar/"` and also to `"/foo/bar"`.
+    ///
+    /// This means that the final component of `"/foo/bar/."` is `"bar"`, and we
+    /// will not check if `"/foo/bar/"` is a dir or not.
+    ///
+    /// # Error
+    ///
+    /// This function will return an error in the following situations:
+    ///
+    /// - The `guest_path` is a relative path.
+    /// - An error occurred while calling `Substitutor::substitute()` to convert
+    ///   to the host side path
+    /// - A non-final component in path is not a directory.
     fn canonicalize<P: AsRef<Path>>(&self, guest_path: P, deref_final: bool) -> Result<PathBuf> {
         let guest_path = guest_path.as_ref();
         // The `guest_path` must be absolute path
@@ -72,24 +93,24 @@ impl Canonicalizer for FileSystem {
 
                     let metadata = host_path.symlink_metadata();
                     // `metadata` is error if we cannot access this file or file is not exist.
-                    // However we can accept this path if dereference final component is not
-                    // required. Because Some syscall (e.g. mkdir, mknod) allow path not exist.
-                    if metadata.is_err() && is_last_component && !deref_final {
+                    // However, we can accept this path because some syscall (e.g. mkdir, mknod)
+                    // allow final component not exist.
+                    if is_last_component && metadata.is_err() {
                         continue;
                     }
+                    // We can continue if we are now on the last component and are explicitly asked
+                    // not to dereference 'user_path'.
+                    if is_last_component && !deref_final {
+                        continue;
+                    }
+
                     let file_type = metadata?.file_type();
 
                     // directory can always push
                     if file_type.is_dir() {
                         continue;
                     }
                     if file_type.is_symlink() {
-                        // we can continue if current path is symlink and is last component and
-                        // if we explicitly ask to not dereference 'user_path', as required by
-                        // kernel like `lstat(2)`
-                        if is_last_component && !deref_final {
-                            continue;
-                        }
                         // we need to deref
                         // TODO: add test for this
                         let link_value = host_path.read_link()?;
@@ -140,33 +161,56 @@ mod tests {
     fn test_canonicalize_invalid_path() {
         let fs = FileSystem::with_root(get_test_rootfs_path()).unwrap();
 
+        // A path with non-existing final component is accepted.
         assert_eq!(
-            fs.canonicalize("/impossible_path", true),
+            fs.canonicalize("/non_existing_path", true),
+            Ok("/non_existing_path".into())
+        );
+        assert_eq!(
+            fs.canonicalize("/non_existing_path", false),
+            Ok("/non_existing_path".into())
+        );
+        assert_eq!(
+            fs.canonicalize("/etc/non_existing_path", true),
+            Ok("/etc/non_existing_path".into())
+        );
+        assert_eq!(
+            fs.canonicalize("/etc/non_existing_path", false),
+            Ok("/etc/non_existing_path".into())
+        );
+        // Any non-final component in path should exist
+        assert_eq!(
+            fs.canonicalize("/etc/non_existing_path/non_existing_path", true),
             Err(Error::errno(Errno::ENOENT))
         );
         assert_eq!(
-            fs.canonicalize("/etc/impossible_path", true),
+            fs.canonicalize("/etc/non_existing_path/non_existing_path", false),
             Err(Error::errno(Errno::ENOENT))
         );
+        // Any non-final component in path should be directory
         assert_eq!(
-            fs.canonicalize("/etc/impossible_path", false),
-            Ok("/etc/impossible_path".into())
+            fs.canonicalize("/etc/passwd/non_existing_path", true),
+            Err(Error::errno(Errno::ENOTDIR))
+        );
+        assert_eq!(
+            fs.canonicalize("/etc/passwd/non_existing_path", false),
+            Err(Error::errno(Errno::ENOTDIR))
         );
     }
 
     #[test]
     fn test_canonicalize_path_traversal() {
         let fs = FileSystem::with_root(get_test_rootfs_path()).unwrap();
 
-        let path = PathBuf::from("/../impossible_path");
-        // should be failed, because ${rootfs}/impossible_path does not exist on host
+        let path = PathBuf::from("/../non_existing_path");
+        // should be ok, because ${rootfs}/non_existing_path exists on host
         assert_eq!(
-            fs.canonicalize(&path, true),
-            Err(Error::errno(Errno::ENOENT))
+            fs.canonicalize(&path, false),
+            Ok("/non_existing_path".into())
         );
         // should be ok, because ${rootfs}/bin exists on host
         let path = PathBuf::from("/../bin");
-        assert_eq!(fs.canonicalize(&path, false), Ok(PathBuf::from("/bin")));
+        assert_eq!(fs.canonicalize(&path, false), Ok("/bin".into()));
     }
     #[test]
     fn test_canonicalize_normal_path() {

src/filesystem/fs.rs

@@ -1,4 +1,3 @@
-use std::os::unix::prelude::OsStrExt;
 use std::path::{Path, PathBuf};
 
 use nix::sys::{self, stat::Mode};
@@ -60,6 +59,13 @@ impl FileSystem {
         let canonical_host_path = std::fs::canonicalize(host_path)?;
         // TODO: allow path not existed when glue is implemented
         let canonical_guest_path = self.canonicalize(guest_path.as_ref(), true)?;
+        // We need to ensure that the target path for the binding exists.
+        // Skip the check for "/" because "/" always exists.
+        if canonical_guest_path != Path::new("/") {
+            self.substitute(&canonical_guest_path, Side::Guest)?
+                .metadata()?;
+        }
+
         // Add a binding at the beginning of the list, so that we get the most recent
         // one when going through them in the `get_binding` method.
         self.bindings.insert(
@@ -149,8 +155,8 @@ impl FileSystem {
             ));
         }
 
-        let guest_path_canonical = self.canonicalize(&guest_path, true)?;
-        let host_path = self.substitute(&guest_path_canonical, Side::Guest)?;
+        let canonical_guest_path = self.canonicalize(&guest_path, true)?;
+        let host_path = self.substitute(&canonical_guest_path, Side::Guest)?;
 
         // To change cwd to a dir, the tracee must have execute (`x`) permission to this
         // dir, FIXME: This may be wrong, because we need to check if tracee has
@@ -160,7 +166,7 @@ impl FileSystem {
         }
         nix::unistd::access(&host_path, AccessFlags::X_OK)?;
 
-        self.cwd = guest_path_canonical;
+        self.cwd = canonical_guest_path;
         Ok(())
     }
 
@@ -170,7 +176,8 @@ impl FileSystem {
     #[inline]
     pub fn set_root<P: AsRef<Path>>(&mut self, host_path: P) -> Result<()> {
         let raw_root = host_path.as_ref();
-        // the `root` is host path, we use host side canonicalize() to canonicalize it
+        // the `root` is host path, we use host side canonicalize() to canonicalize it.
+        // std::fs::canonicalize() also throws an error if the path does not exist.
         let canonical_root = std::fs::canonicalize(raw_root)?;
         self.root = canonical_root.clone();
         self.add_binding(canonical_root, "/")?;
@@ -367,6 +374,10 @@ mod tests {
 
         let root_path = get_test_rootfs_path();
         let mut fs = FileSystem::with_root(root_path)?;
+        // we currently cannot bind to a non-existing guest path.
+        fs.add_binding("/etc", "/bin/non_existing_path")
+            .unwrap_err();
+        fs.add_binding("/non_existing_path", "/bin").unwrap_err();
         fs.add_binding("/etc", "/usr")?;
         fs.add_binding("/etc/../tmp/", "/home/../home")?;
         fs.add_binding("home", "/home").unwrap_err();

src/filesystem/mod.rs

@@ -1,8 +1,8 @@
 pub mod binding;
-mod canonicalization;
+pub mod canonicalization;
 mod fs;
 pub mod readers;
-mod substitution;
+pub mod substitution;
 pub mod temp;
 mod translation;
 pub mod validation;

src/filesystem/translation.rs

@@ -1,4 +1,5 @@
 use crate::errors::Result;
+
 use crate::filesystem::binding::Side::{Guest, Host};
 use crate::filesystem::canonicalization::Canonicalizer;
 use crate::filesystem::substitution::Substitutor;
@@ -40,8 +41,8 @@ impl Translator for FileSystem {
         guest_path: P,
         deref_final: bool,
     ) -> Result<PathBuf> {
-        let guest_path_canonical = self.canonicalize(&guest_path, deref_final)?;
-        let host_path = self.substitute(&guest_path_canonical, Guest)?;
+        let canonical_guest_path = self.canonicalize(&guest_path, deref_final)?;
+        let host_path = self.substitute(&canonical_guest_path, Guest)?;
         Ok(host_path)
     }
 

src/kernel/enter.rs

@@ -8,43 +8,41 @@ use crate::kernel::socket::*;
 use crate::kernel::standard::*;
 use crate::process::proot::InfoBag;
 use crate::process::tracee::Tracee;
-use crate::register::Current;
+use crate::register::Original;
 
 pub fn translate(info_bag: &InfoBag, tracee: &mut Tracee) -> Result<()> {
-    let sys_num = tracee.regs.get_sys_num(Current);
+    let sys_num = tracee.regs.get_sys_num(Original);
     let sys_type = syscall_group_from_sysnum(sys_num);
 
-    trace!("Syscall enter ({:?}, {:?})", sys_num, sys_type);
-
     match sys_type {
         Accept => accept::enter(),
         BindConnect => bind_connect::enter(),
         Brk => brk::enter(),
-        Chdir => chdir::enter(),
-        ChmodAccessMkNodAt => chmod_access_mknod_at::enter(),
-        DirLinkAttr => dir_link_attr::enter(),
+        Chdir => chdir::enter(tracee),
+        ChmodAccessMkNodAt => chmod_access_mknod_at::enter(tracee),
+        DirLinkAttr => dir_link_attr::enter(tracee),
         Execve => execve::enter(tracee, &info_bag.loader),
-        GetCwd => getcwd::enter(),
+        GetCwd => getcwd::enter(tracee),
         GetSockOrPeerName => get_sockorpeer_name::enter(),
         InotifyAddWatch => inotify_add_watch::enter(),
-        Link => link_rename::enter(),
-        LinkAt => link_at::enter(),
+        Link => link_rename::enter(tracee),
+        LinkAt => link_at::enter(tracee),
         Mount => mount::enter(),
         Open => open::enter(tracee),
         OpenAt => open_at::enter(tracee),
         PivotRoot => pivot_root::enter(),
         Ptrace => ptrace::enter(),
-        ReadLink => standard_syscall::enter(tracee),
-        ReadLinkAt => readlink_at::enter(),
-        Rename => link_rename::enter(),
-        RenameAt => rename_at::enter(),
+        ReadLink => dir_link_attr::enter(tracee),
+        ReadLinkAt => unlink_mkdir_at::enter(tracee),
+        Rename => link_rename::enter(tracee),
+        RenameAt => rename_at::enter(tracee),
         SocketCall => socketcall::enter(),
         StandardSyscall => standard_syscall::enter(tracee),
-        StatAt => stat_at::enter(),
-        SymLink => sym_link::enter(),
-        SymLinkAt => sym_link_at::enter(),
+        StatAt => stat_at::enter(tracee),
+        SymLink => sym_link::enter(tracee),
+        SymLinkAt => sym_link_at::enter(tracee),
         Wait => wait::enter(),
-        UnlinkMkdirAt => unlink_mkdir_at::enter(),
+        UnlinkMkdirAt => unlink_mkdir_at::enter(tracee),
         _ => Ok(()),
     }
 }