From 54de00a3bca03e1ee415552062a877d361ace17b Mon Sep 17 00:00:00 2001 From: LI Qing Date: Tue, 19 Jul 2022 19:41:03 +0800 Subject: [PATCH] Fix the issue when path is suffixed by "/" --- src/libos/src/fs/file_ops/link.rs | 3 ++ src/libos/src/fs/file_ops/mkdir.rs | 2 +- src/libos/src/fs/file_ops/rename.rs | 10 +++-- src/libos/src/fs/file_ops/rmdir.rs | 2 +- src/libos/src/fs/file_ops/symlink.rs | 3 ++ src/libos/src/fs/file_ops/unlink.rs | 3 ++ src/libos/src/fs/fs_view.rs | 55 +++++++++++++++++++--------- src/libos/src/fs/mod.rs | 31 +++++++++++++--- src/libos/src/fs/rootfs.rs | 18 ++++++--- test/hostfs/main.c | 2 +- test/mkdir/main.c | 23 ++++++++++++ test/open/main.c | 25 ++++++++++++- test/rename/main.c | 23 +++++++++++- test/stat/main.c | 18 +++++++++ 14 files changed, 180 insertions(+), 38 deletions(-) diff --git a/src/libos/src/fs/file_ops/link.rs b/src/libos/src/fs/file_ops/link.rs index 499f5a10..518bece4 100644 --- a/src/libos/src/fs/file_ops/link.rs +++ b/src/libos/src/fs/file_ops/link.rs @@ -15,6 +15,9 @@ pub fn do_linkat(old_fs_path: &FsPath, new_fs_path: &FsPath, flags: LinkFlags) - let newpath = new_fs_path.to_abs_path()?; let (new_dir_path, new_file_name) = split_path(&newpath); + if new_file_name.ends_with("/") { + return_errno!(EISDIR, "new path is dir"); + } let (inode, new_dir_inode) = { let oldpath = old_fs_path.to_abs_path()?; let current = current!(); diff --git a/src/libos/src/fs/file_ops/mkdir.rs b/src/libos/src/fs/file_ops/mkdir.rs index 4e7e663d..55623965 100644 --- a/src/libos/src/fs/file_ops/mkdir.rs +++ b/src/libos/src/fs/file_ops/mkdir.rs @@ -4,7 +4,7 @@ pub fn do_mkdirat(fs_path: &FsPath, mode: FileMode) -> Result<()> { debug!("mkdirat: fs_path: {:?}, mode: {:#o}", fs_path, mode.bits()); let path = fs_path.to_abs_path()?; - let (dir_path, file_name) = split_path(&path); + let (dir_path, file_name) = split_path(&path.trim_end_matches('/')); let current = current!(); let inode = { let fs = current.fs().read().unwrap(); diff --git a/src/libos/src/fs/file_ops/rename.rs b/src/libos/src/fs/file_ops/rename.rs index 595f99b7..f46c8a42 100644 --- a/src/libos/src/fs/file_ops/rename.rs +++ b/src/libos/src/fs/file_ops/rename.rs @@ -19,19 +19,23 @@ pub fn do_renameat(old_fs_path: &FsPath, new_fs_path: &FsPath) -> Result<()> { let current = current!(); let fs = current.fs().read().unwrap(); - let (old_dir_path, old_file_name) = split_path(&oldpath); - let (new_dir_path, new_file_name) = split_path(&newpath); + // The source and target to be renamed could be dirs + let (old_dir_path, old_file_name) = split_path(&oldpath.trim_end_matches('/')); + let (new_dir_path, new_file_name) = split_path(&newpath.trim_end_matches('/')); let old_dir_inode = fs.lookup_inode(old_dir_path)?; let new_dir_inode = fs.lookup_inode(new_dir_path)?; let old_file_mode = { let old_file_inode = old_dir_inode.find(old_file_name)?; let metadata = old_file_inode.metadata()?; + // oldpath is directory, the old_file_inode should be directory + if oldpath.ends_with("/") && metadata.type_ != FileType::Dir { + return_errno!(ENOTDIR, "old path is not a directory"); + } FileMode::from_bits_truncate(metadata.mode) }; if old_file_mode.has_sticky_bit() { warn!("ignoring the sticky bit"); } - // TODO: support to modify file's absolute path old_dir_inode.move_(old_file_name, &new_dir_inode, new_file_name)?; Ok(()) } diff --git a/src/libos/src/fs/file_ops/rmdir.rs b/src/libos/src/fs/file_ops/rmdir.rs index def94478..574c480b 100644 --- a/src/libos/src/fs/file_ops/rmdir.rs +++ b/src/libos/src/fs/file_ops/rmdir.rs @@ -3,7 +3,7 @@ use super::*; pub fn do_rmdir(path: &str) -> Result<()> { debug!("rmdir: path: {:?}", path); - let (dir_path, file_name) = split_path(&path); + let (dir_path, file_name) = split_path(path.trim_end_matches('/')); let dir_inode = { let current = current!(); let fs = current.fs().read().unwrap(); diff --git a/src/libos/src/fs/file_ops/symlink.rs b/src/libos/src/fs/file_ops/symlink.rs index b90e33af..a37e219e 100644 --- a/src/libos/src/fs/file_ops/symlink.rs +++ b/src/libos/src/fs/file_ops/symlink.rs @@ -36,6 +36,9 @@ pub fn do_symlinkat(target: &str, link_path: &FsPath) -> Result { let link_path = link_path.to_abs_path()?; let (dir_path, link_name) = split_path(&link_path); + if link_name.ends_with('/') { + return_errno!(EISDIR, "link path is dir"); + } let dir_inode = { let current = current!(); let fs = current.fs().read().unwrap(); diff --git a/src/libos/src/fs/file_ops/unlink.rs b/src/libos/src/fs/file_ops/unlink.rs index 927bc0c2..cf7995ab 100644 --- a/src/libos/src/fs/file_ops/unlink.rs +++ b/src/libos/src/fs/file_ops/unlink.rs @@ -8,6 +8,9 @@ bitflags! { fn do_unlink(path: &str) -> Result<()> { let (dir_path, file_name) = split_path(&path); + if file_name.ends_with("/") { + return_errno!(EISDIR, "unlink on directory"); + } let dir_inode = { let current = current!(); let fs = current.fs().read().unwrap(); diff --git a/src/libos/src/fs/fs_view.rs b/src/libos/src/fs/fs_view.rs index c28a1a2e..8d029266 100644 --- a/src/libos/src/fs/fs_view.rs +++ b/src/libos/src/fs/fs_view.rs @@ -68,6 +68,12 @@ impl FsView { inode } Err(e) if e.errno() == ENOENT && creation_flags.can_create() => { + if creation_flags.must_be_directory() { + return_errno!(ENOTDIR, "cannot create directory"); + } + if path.ends_with("/") { + return_errno!(EISDIR, "path is a directory"); + } let (dir_path, file_name) = split_path(&path); let dir_inode = self.lookup_inode(dir_path)?; if !dir_inode.allow_write()? { @@ -94,8 +100,17 @@ impl FsView { inode } Err(e) if e.errno() == ENOENT && creation_flags.can_create() => { + if creation_flags.must_be_directory() { + return_errno!(ENOTDIR, "cannot create directory"); + } + if path.ends_with("/") { + return_errno!(EISDIR, "path is a directory"); + } let real_path = self.lookup_real_path(&path)?; let (dir_path, file_name) = split_path(&real_path); + if file_name.ends_with("/") { + return_errno!(EISDIR, "path refers to a directory"); + } let dir_inode = self.lookup_inode(dir_path)?; if !dir_inode.allow_write()? { return_errno!(EPERM, "file cannot be created"); @@ -111,31 +126,29 @@ impl FsView { /// Recursively lookup the real path of giving path, dereference symlinks pub fn lookup_real_path(&self, path: &str) -> Result { - let (dir_path, file_name) = split_path(&path); + let (dir_path, file_name) = split_path(path); let dir_inode = self.lookup_inode(dir_path)?; - match dir_inode.find(file_name) { + match dir_inode.find(file_name.trim_end_matches('/')) { // Handle symlink Ok(inode) if inode.metadata()?.type_ == FileType::SymLink => { let new_path = { let mut content = vec![0u8; PATH_MAX]; let len = inode.read_at(0, &mut content)?; - let path = std::str::from_utf8(&content[..len]) + let link_path = std::str::from_utf8(&content[..len]) .map_err(|_| errno!(ENOENT, "invalid symlink content"))?; - let path = String::from(path); - match path.chars().next() { - None => unreachable!(), + let link_path = String::from(link_path); + + let mut new_path = match link_path.chars().next() { + None => return_errno!(ENOENT, "empty symlink"), // absolute path - Some('/') => path, + Some('/') => link_path, // relative path - _ => { - let dir_path = if dir_path.ends_with("/") { - String::from(dir_path) - } else { - String::from(dir_path) + "/" - }; - dir_path + &path - } + _ => String::from(dir_path) + "/" + &link_path, + }; + if path.ends_with("/") && !new_path.ends_with("/") { + new_path += "/"; } + new_path }; self.lookup_real_path(&new_path) } @@ -147,7 +160,8 @@ impl FsView { } } - /// Lookup INode from the cwd of the process. If path is a symlink, do not dereference it + /// Lookup INode from the cwd of the process. + /// If last component is a symlink, do not dereference it pub fn lookup_inode_no_follow(&self, path: &str) -> Result> { debug!( "lookup_inode_no_follow: cwd: {:?}, path: {:?}", @@ -155,8 +169,13 @@ impl FsView { path ); let (dir_path, file_name) = split_path(&path); - let dir_inode = self.lookup_inode(dir_path)?; - Ok(dir_inode.lookup(file_name)?) + let inode = if file_name.ends_with("/") { + self.lookup_inode(path)? + } else { + let dir_inode = self.lookup_inode(dir_path)?; + dir_inode.lookup(file_name)? + }; + Ok(inode) } /// Lookup INode from the cwd of the process, dereference symlink diff --git a/src/libos/src/fs/mod.rs b/src/libos/src/fs/mod.rs index 9af110b5..8d752b8f 100644 --- a/src/libos/src/fs/mod.rs +++ b/src/libos/src/fs/mod.rs @@ -59,14 +59,33 @@ mod stdio; mod syscalls; mod timer_file; -/// Split a `path` str to `(base_path, file_name)` +/// Split a `path` to (`dir_path`, `file_name`). +/// +/// The `dir_path` must be a directory. +/// +/// The `file_name` is the last component. It can be suffixed by "/". +/// +/// Example: +/// +/// The path "/dir/file/" will be split to ("/dir", "file/"). fn split_path(path: &str) -> (&str, &str) { + let file_name = path + .split_inclusive('/') + .filter(|&x| x != "/") + .last() + .unwrap_or("."); + let mut split = path.trim_end_matches('/').rsplitn(2, '/'); - let file_name = split.next().unwrap(); - let mut dir_path = split.next().unwrap_or("."); - if dir_path == "" { - dir_path = "/"; - } + let dir_path = if split.next().unwrap().is_empty() { + "/" + } else { + let mut dir = split.next().unwrap_or(".").trim_end_matches('/'); + if dir.is_empty() { + dir = "/"; + } + dir + }; + (dir_path, file_name) } diff --git a/src/libos/src/fs/rootfs.rs b/src/libos/src/fs/rootfs.rs index e6793e1b..d7492493 100644 --- a/src/libos/src/fs/rootfs.rs +++ b/src/libos/src/fs/rootfs.rs @@ -80,8 +80,12 @@ pub fn umount_nonroot_fs( root.lookup_follow(abs_path, MAX_SYMLINKS)? } else { let (dir_path, file_name) = split_path(abs_path); - root.lookup_follow(dir_path, MAX_SYMLINKS)? - .lookup(file_name)? + if file_name.ends_with("/") { + root.lookup_follow(abs_path, MAX_SYMLINKS)? + } else { + root.lookup_follow(dir_path, MAX_SYMLINKS)? + .lookup(file_name)? + } }; mount_dir.downcast_ref::().unwrap().umount()?; @@ -173,9 +177,13 @@ pub fn mount_fs_at( parent_inode.lookup_follow(path, MAX_SYMLINKS)? } else { let (dir_path, file_name) = split_path(path); - parent_inode - .lookup_follow(dir_path, MAX_SYMLINKS)? - .lookup(file_name)? + if file_name.ends_with("/") { + parent_inode.lookup_follow(path, MAX_SYMLINKS)? + } else { + parent_inode + .lookup_follow(dir_path, MAX_SYMLINKS)? + .lookup(file_name)? + } }; mount_dir.downcast_ref::().unwrap().mount(fs)?; Ok(()) diff --git a/test/hostfs/main.c b/test/hostfs/main.c index c4db4988..d71fffaf 100644 --- a/test/hostfs/main.c +++ b/test/hostfs/main.c @@ -211,7 +211,7 @@ static int test_truncate() { } static int test_mkdir_then_rmdir() { - const char *dir_path = "/host/hostfs_dir"; + const char *dir_path = "/host/hostfs_dir/"; struct stat stat_buf; int dir_fd; diff --git a/test/mkdir/main.c b/test/mkdir/main.c index 87e60aa3..b901fd82 100644 --- a/test/mkdir/main.c +++ b/test/mkdir/main.c @@ -48,6 +48,24 @@ static int __test_mkdir(const char *dir_path) { return 0; } +static int __test_mkdir_with_suffix(const char *dir_path) { + struct stat stat_buf; + char new_dir_path[PATH_MAX] = { 0 }; + snprintf(new_dir_path, sizeof(new_dir_path), "%s/", dir_path); + mode_t mode = 00775; + + if (mkdir(new_dir_path, mode) < 0) { + THROW_ERROR("failed to mkdir"); + } + if (stat(new_dir_path, &stat_buf) < 0) { + THROW_ERROR("failed to stat dir"); + } + if (!S_ISDIR(stat_buf.st_mode)) { + THROW_ERROR("failed to check if it is dir"); + } + return 0; +} + static int __test_mkdirat(const char *dir_path) { struct stat stat_buf; mode_t mode = 00775; @@ -95,6 +113,10 @@ static int test_mkdir() { return test_mkdir_framework(__test_mkdir); } +static int test_mkdir_with_suffix() { + return test_mkdir_framework(__test_mkdir_with_suffix); +} + static int test_mkdirat() { return test_mkdir_framework(__test_mkdirat); } @@ -189,6 +211,7 @@ static int test_rmdir_via_unlinkat() { static test_case_t test_cases[] = { TEST_CASE(test_mkdir), + TEST_CASE(test_mkdir_with_suffix), TEST_CASE(test_mkdirat), TEST_CASE(test_chdir), TEST_CASE(test_rmdir_via_unlinkat), diff --git a/test/open/main.c b/test/open/main.c index 13484d05..0f8339eb 100644 --- a/test/open/main.c +++ b/test/open/main.c @@ -27,17 +27,33 @@ static int __test_open(const char *file_path, int flags, int mode) { THROW_ERROR("failed to open a file"); } close(fd); + + return 0; +} + +static int __test_open_file_as_dir(const char *file_path, int flags, int mode) { + char dir_path[PATH_MAX] = { 0 }; + snprintf(dir_path, sizeof(dir_path), "%s/", file_path); + + int fd = open(dir_path, flags, mode); + if (!(fd < 0 && errno == EISDIR)) { + THROW_ERROR("failed check open a file as dir"); + } + + if (__test_open(file_path, flags, mode) < 0) { + THROW_ERROR("failed to create file"); + } return 0; } static int __test_open_file_with_dir_flags(const char *file_path, int flags, int mode) { - flags = O_DIRECTORY | O_RDWR | O_CREAT; int fd = open(file_path, flags, mode); if (fd < 0) { - THROW_ERROR("failed to check creating file with O_DIRECTORY"); + THROW_ERROR("failed to open a file"); } close(fd); + flags = O_DIRECTORY | O_RDWR; fd = open(file_path, flags, mode); if (!(fd < 0 && errno == ENOTDIR)) { THROW_ERROR("open file with O_DIRECTORY should return ENOTDIR"); @@ -132,6 +148,10 @@ static int test_open() { return test_open_framework(__test_open); } +static int test_open_file_as_dir() { + return test_open_framework(__test_open_file_as_dir); +} + static int test_open_file_with_dir_flags() { return test_open_framework(__test_open_file_with_dir_flags); } @@ -158,6 +178,7 @@ static int test_creat() { static test_case_t test_cases[] = { TEST_CASE(test_open), + TEST_CASE(test_open_file_as_dir), TEST_CASE(test_open_file_with_dir_flags), TEST_CASE(test_open_dir_with_write_flags), TEST_CASE(test_openat_with_abs_path), diff --git a/test/rename/main.c b/test/rename/main.c index ed380b50..880ff67e 100644 --- a/test/rename/main.c +++ b/test/rename/main.c @@ -132,7 +132,7 @@ static int test_renameat() { } static int test_rename_dir() { - const char *old_dir = "/root/test_old_dir"; + const char *old_dir = "/root/test_old_dir/"; const char *new_dir = "/root/test_new_dir"; const char *file_name = "test_file.txt"; char file_buf[128] = { 0 }; @@ -203,6 +203,26 @@ static int test_rename_dir_to_subdir() { return 0; } +static int test_rename_file_as_dir() { + const char *old_file = "/root/test_old_file"; + const char *old_dir = "/root/test_old_file/"; + const char *new_dir = "/root/test_new_dir"; + + if (create_file_with_content(old_file, WRITE_MSG) < 0) { + THROW_ERROR("failed to create old file with content"); + } + + int ret = rename(old_dir, new_dir); + if (ret == 0 || errno != ENOTDIR) { + THROW_ERROR("failed to check rename file as dir"); + } + + if (unlink(old_file) < 0) { + THROW_ERROR("failed to remove file"); + } + return 0; +} + // ============================================================================ // Test suite main // ============================================================================ @@ -214,6 +234,7 @@ static test_case_t test_cases[] = { TEST_CASE(test_renameat), TEST_CASE(test_rename_dir), TEST_CASE(test_rename_dir_to_subdir), + TEST_CASE(test_rename_file_as_dir), }; int main(int argc, const char *argv[]) { diff --git a/test/stat/main.c b/test/stat/main.c index de21687a..8aab08d9 100644 --- a/test/stat/main.c +++ b/test/stat/main.c @@ -47,6 +47,19 @@ static int __test_stat(const char *file_path) { return 0; } +static int __test_stat_file_as_dir(const char *file_path) { + struct stat stat_buf; + char dir_path[PATH_MAX] = { 0 }; + snprintf(dir_path, sizeof(dir_path), "%s/", file_path); + + int ret = stat(dir_path, &stat_buf); + if (ret == 0 || errno != ENOTDIR) { + THROW_ERROR("failed to check stat file as dir"); + } + + return 0; +} + static int __test_fstat(const char *file_path) { struct stat stat_buf; int fd, ret; @@ -155,6 +168,10 @@ static int test_stat() { return test_stat_framework(__test_stat); } +static int test_stat_file_as_dir() { + return test_stat_framework(__test_stat_file_as_dir); +} + static int test_fstat() { return test_stat_framework(__test_fstat); } @@ -181,6 +198,7 @@ static int test_fstatat_with_dirfd() { static test_case_t test_cases[] = { TEST_CASE(test_stat), + TEST_CASE(test_stat_file_as_dir), TEST_CASE(test_fstat), TEST_CASE(test_lstat), TEST_CASE(test_fstatat_with_abs_path),