From 38d455496b96ce82da82de6a903a06b6fc611ed3 Mon Sep 17 00:00:00 2001 From: Chen Chengjun Date: Fri, 16 Jan 2026 02:12:20 +0000 Subject: [PATCH] Modify all usages of the removed Path APIs --- kernel/src/device/pty/master.rs | 14 +++++----- kernel/src/fs/device.rs | 2 +- kernel/src/fs/overlayfs/fs.rs | 5 +++- kernel/src/fs/path/mod.rs | 5 +++- kernel/src/fs/procfs/pid/task/comm.rs | 5 ++-- kernel/src/fs/procfs/pid/task/mountinfo.rs | 3 +-- kernel/src/fs/sysfs/test.rs | 7 ++++- kernel/src/fs/utils/inode.rs | 10 -------- kernel/src/process/clone.rs | 13 +++++++--- kernel/src/process/execve.rs | 30 +++++++++++++++------- kernel/src/process/process/init_proc.rs | 11 +++++--- kernel/src/syscall/getcwd.rs | 25 ++++++++++++++---- kernel/src/syscall/readlink.rs | 20 +++++++++------ kernel/src/syscall/rename.rs | 8 +++--- 14 files changed, 101 insertions(+), 57 deletions(-) diff --git a/kernel/src/device/pty/master.rs b/kernel/src/device/pty/master.rs index 5bd3649c1..eb767d417 100644 --- a/kernel/src/device/pty/master.rs +++ b/kernel/src/device/pty/master.rs @@ -200,8 +200,13 @@ impl FileIo for PtyMaster { // TODO: Deal with `open()` flags. let slave = { + let fs_ref = thread_local.borrow_fs(); + let path_resolver = fs_ref.resolver().read(); + let slave_name = { - let devpts_path = super::DEV_PTS.get().unwrap().abs_path(); + let devpts_path = path_resolver + .make_abs_path(super::DEV_PTS.get().unwrap()) + .into_string(); format!("{}/{}", devpts_path, self.slave.index()) }; @@ -209,12 +214,7 @@ impl FileIo for PtyMaster { let inode_handle = { let open_args = OpenArgs::from_modes(AccessMode::O_RDWR, mkmod!(u+rw)); - thread_local - .borrow_fs() - .resolver() - .read() - .lookup(&fs_path)? - .open(open_args)? + path_resolver.lookup(&fs_path)?.open(open_args)? }; Arc::new(inode_handle) }; diff --git a/kernel/src/fs/device.rs b/kernel/src/fs/device.rs index 88f563dcb..214bcbadf 100644 --- a/kernel/src/fs/device.rs +++ b/kernel/src/fs/device.rs @@ -73,7 +73,7 @@ pub fn add_node( (relative_path, "") }; - match dev_path.lookup(next_name) { + match path_resolver.lookup_at_path(&dev_path, next_name) { Ok(next_path) => { if path_remain.is_empty() { return_errno_with_message!(Errno::EEXIST, "the device node already exists"); diff --git a/kernel/src/fs/overlayfs/fs.rs b/kernel/src/fs/overlayfs/fs.rs index aec5dc7c8..bb417d246 100644 --- a/kernel/src/fs/overlayfs/fs.rs +++ b/kernel/src/fs/overlayfs/fs.rs @@ -1489,6 +1489,9 @@ mod tests { let link = d1.create("link", InodeType::SymLink, mode).unwrap(); let link_str = "link_to_somewhere"; link.write_link(link_str).unwrap(); - assert_eq!(link.read_link().unwrap().to_string(), link_str.to_string()); + assert!(matches!( + link.read_link().unwrap(), + SymbolicLink::Plain(s) if s == link_str + )); } } diff --git a/kernel/src/fs/path/mod.rs b/kernel/src/fs/path/mod.rs index fe4df1c92..18b295d77 100644 --- a/kernel/src/fs/path/mod.rs +++ b/kernel/src/fs/path/mod.rs @@ -206,7 +206,10 @@ impl Path { return_errno_with_message!(Errno::ENOTDIR, "the path is not a directory"); } - if self.effective_parent().is_none() { + let fs_ref = ctx.thread_local.borrow_fs(); + let path_resolver = fs_ref.resolver().read(); + + if path_resolver.root() == self { return_errno_with_message!(Errno::EINVAL, "the root cannot be mounted on"); } diff --git a/kernel/src/fs/procfs/pid/task/comm.rs b/kernel/src/fs/procfs/pid/task/comm.rs index 52683dd5e..1285b7a11 100644 --- a/kernel/src/fs/procfs/pid/task/comm.rs +++ b/kernel/src/fs/procfs/pid/task/comm.rs @@ -33,9 +33,8 @@ impl FileOps for CommFileOps { return Ok(0); }; - let exe_path = vmar.process_vm().executable_file().abs_path(); - let last_component = exe_path.rsplit('/').next().unwrap_or(&exe_path); - let mut comm = last_component.as_bytes().to_vec(); + let executable_file_name = vmar.process_vm().executable_file().name(); + let mut comm = executable_file_name.as_bytes().to_vec(); comm.truncate(TASK_COMM_LEN - 1); comm.push(b'\n'); diff --git a/kernel/src/fs/procfs/pid/task/mountinfo.rs b/kernel/src/fs/procfs/pid/task/mountinfo.rs index f5fe9cbb6..d48eaa3c2 100644 --- a/kernel/src/fs/procfs/pid/task/mountinfo.rs +++ b/kernel/src/fs/procfs/pid/task/mountinfo.rs @@ -30,8 +30,7 @@ impl FileOps for MountInfoFileOps { let fs = posix_thread.read_fs(); let path_resolver = fs.resolver().read(); - let root_mount = path_resolver.root().mount_node(); - root_mount.read_mount_info(offset, writer) + path_resolver.read_mount_info(offset, writer) } } diff --git a/kernel/src/fs/sysfs/test.rs b/kernel/src/fs/sysfs/test.rs index 5bf27af72..a27155ec2 100644 --- a/kernel/src/fs/sysfs/test.rs +++ b/kernel/src/fs/sysfs/test.rs @@ -427,6 +427,8 @@ fn test_sysfs_write_attr() { #[ktest] fn test_sysfs_read_link() { + use crate::fs::utils::SymbolicLink; + let sysfs = init_sysfs_with_mock_tree(); let root_inode = sysfs.root_inode(); // Action: Lookup the sysfs symlink corresponding to a systree symlink node @@ -436,7 +438,10 @@ fn test_sysfs_read_link() { // the path provided by the underlying mock systree symlink node's target_path method. let target = link1_inode.read_link().expect("read_link failed"); - assert_eq!(target.to_string(), "../branch1/leaf1"); + assert!(matches!( + target, + SymbolicLink::Plain(s) if s == "../branch1/leaf1" + )); // read_link on non-symlink should fail (expect EINVAL as per inode.rs) let branch1_inode = root_inode.lookup("branch1").unwrap(); diff --git a/kernel/src/fs/utils/inode.rs b/kernel/src/fs/utils/inode.rs index c45e9edd5..6122019da 100644 --- a/kernel/src/fs/utils/inode.rs +++ b/kernel/src/fs/utils/inode.rs @@ -605,13 +605,3 @@ pub enum SymbolicLink { /// such as `/proc/[pid]/fd/[fd]` and `/proc/[pid]/exe`. Path(Path), } - -#[expect(clippy::to_string_trait_impl)] -impl ToString for SymbolicLink { - fn to_string(&self) -> String { - match self { - SymbolicLink::Plain(s) => s.clone(), - SymbolicLink::Path(path) => path.abs_path(), - } - } -} diff --git a/kernel/src/process/clone.rs b/kernel/src/process/clone.rs index 52d14ec7a..1d9b6be43 100644 --- a/kernel/src/process/clone.rs +++ b/kernel/src/process/clone.rs @@ -498,9 +498,16 @@ fn clone_child_process( let child = { let mut child_thread_builder = { - let child_thread_name = ThreadName::new_from_executable_path( - &child_vmar.process_vm().executable_file().abs_path(), - ); + let thread_name = { + let executable_path = child_vmar.process_vm().executable_file(); + thread_local + .borrow_fs() + .resolver() + .read() + .make_abs_path(executable_path) + .into_string() + }; + let child_thread_name = ThreadName::new_from_executable_path(&thread_name); let credentials = { let credentials = ctx.posix_thread.credentials(); diff --git a/kernel/src/process/execve.rs b/kernel/src/process/execve.rs index 5337cb805..0d067015a 100644 --- a/kernel/src/process/execve.rs +++ b/kernel/src/process/execve.rs @@ -10,7 +10,10 @@ use ostd::{ use super::process_vm::activate_vmar; use crate::{ - fs::{path::Path, utils::Inode}, + fs::{ + path::{Path, PathResolver}, + utils::Inode, + }, prelude::*, process::{ ContextUnshareAdminApi, Credentials, Process, @@ -40,16 +43,17 @@ pub fn do_execve( // of all strings to enforce a sensible overall limit. let argv = read_cstring_vec(argv_ptr_ptr, MAX_NR_STRING_ARGS, MAX_LEN_STRING_ARG, ctx)?; let envp = read_cstring_vec(envp_ptr_ptr, MAX_NR_STRING_ARGS, MAX_LEN_STRING_ARG, ctx)?; - debug!( - "filename: {:?}, argv = {:?}, envp = {:?}", - elf_file.abs_path(), - argv, - envp - ); let fs_ref = ctx.thread_local.borrow_fs(); let path_resolver = fs_ref.resolver().read(); + debug!( + "file path: {:?}, argv = {:?}, envp = {:?}", + path_resolver.make_abs_path(&elf_file).into_string(), + argv, + envp + ); + let elf_inode = elf_file.inode(); let program_to_load = ProgramToLoad::build_from_inode(elf_inode.clone(), &path_resolver, argv, envp)?; @@ -75,7 +79,14 @@ pub fn do_execve( // After this point, failures in subsequent operations are fatal: the process // state may be left inconsistent and it can never return to user mode. - let res = do_execve_no_return(ctx, user_context, elf_file, new_vmar, &elf_load_info); + let res = do_execve_no_return( + ctx, + user_context, + &path_resolver, + elf_file, + new_vmar, + &elf_load_info, + ); if res.is_err() { ctx.posix_thread @@ -128,6 +139,7 @@ fn read_cstring_vec( fn do_execve_no_return( ctx: &Context, user_context: &mut UserContext, + path_resolver: &PathResolver, elf_file: Path, new_vmar: Arc, elf_load_info: &ElfLoadInfo, @@ -165,7 +177,7 @@ fn do_execve_no_return( unshare_and_close_files(ctx); // Update the process's executable path and set the thread name - let executable_path = elf_file.abs_path(); + let executable_path = path_resolver.make_abs_path(&elf_file).into_string(); *posix_thread.thread_name().lock() = ThreadName::new_from_executable_path(&executable_path); // Unshare and reset signal dispositions to their default actions. diff --git a/kernel/src/process/process/init_proc.rs b/kernel/src/process/process/init_proc.rs index 48fbdf6c6..db55a59e1 100644 --- a/kernel/src/process/process/init_proc.rs +++ b/kernel/src/process/process/init_proc.rs @@ -106,18 +106,23 @@ fn create_init_task( ) -> Result> { let credentials = Credentials::new_root(); - let elf_load_info = { + let (elf_load_info, elf_abs_path) = { let path_resolver = fs.resolver().read(); + let program_to_load = ProgramToLoad::build_from_inode(elf_path.inode().clone(), &path_resolver, argv, envp)?; let vmar = process.lock_vmar(); - program_to_load.load_to_vmar(vmar.unwrap(), &path_resolver)? + let elf_load_info = program_to_load.load_to_vmar(vmar.unwrap(), &path_resolver)?; + let elf_abs_path = path_resolver.make_abs_path(&elf_path).into_string(); + + (elf_load_info, elf_abs_path) }; + let mut user_ctx = UserContext::default(); user_ctx.set_instruction_pointer(elf_load_info.entry_point as _); user_ctx.set_stack_pointer(elf_load_info.user_stack_top as _); - let thread_name = ThreadName::new_from_executable_path(&elf_path.abs_path()); + let thread_name = ThreadName::new_from_executable_path(&elf_abs_path); let thread_builder = PosixThreadBuilder::new(tid, thread_name, Box::new(user_ctx), credentials) .process(Arc::downgrade(process)) diff --git a/kernel/src/syscall/getcwd.rs b/kernel/src/syscall/getcwd.rs index ae92079cb..ebae7a523 100644 --- a/kernel/src/syscall/getcwd.rs +++ b/kernel/src/syscall/getcwd.rs @@ -3,14 +3,29 @@ use ostd::mm::VmIo; use super::SyscallReturn; -use crate::prelude::*; +use crate::{fs::path::AbsPathResult, prelude::*}; pub fn sys_getcwd(buf: Vaddr, len: usize, ctx: &Context) -> Result { - let dirent = ctx.thread_local.borrow_fs().resolver().read().cwd().clone(); - let name = dirent.abs_path(); - debug!("getcwd: {:?}", name); + let abs_path = { + let fs_ref = ctx.thread_local.borrow_fs(); + let path_resolver = fs_ref.resolver().read(); + path_resolver.make_abs_path(path_resolver.cwd()) + }; - let cwd = CString::new(name)?; + debug!("getcwd: {:?}", abs_path); + + // Linux will add a "(unreachable)" prefix to the path if the CWD is not reachable. + // However, according to POSIX, `getcwd()` should fail with ENOENT in this case. + // `glibc` treats the Linux's behavior as a bug and handles the Linux-specific prefix + // to conform to POSIX. Here follows Linux's behavior to keep compatibility. + // + // Reference: + let abs_path = match abs_path { + AbsPathResult::Reachable(s) => s, + AbsPathResult::Unreachable(s) => alloc::format!("(unreachable){}", s), + }; + + let cwd = CString::new(abs_path)?; let bytes = cwd.as_bytes_with_nul(); let write_len = len.min(bytes.len()); ctx.user_space().write_bytes(buf, &bytes[..write_len])?; diff --git a/kernel/src/syscall/readlink.rs b/kernel/src/syscall/readlink.rs index 8f697f491..ddd93c459 100644 --- a/kernel/src/syscall/readlink.rs +++ b/kernel/src/syscall/readlink.rs @@ -7,6 +7,7 @@ use crate::{ fs::{ file_table::FileDesc, path::{AT_FDCWD, FsPath}, + utils::SymbolicLink, }, prelude::*, syscall::constants::MAX_FILENAME_LEN, @@ -26,18 +27,21 @@ pub fn sys_readlinkat( dirfd, path_name, usr_buf_addr, usr_buf_len ); - let path = { + let link_path = { + let fs_ref = ctx.thread_local.borrow_fs(); + let path_resolver = fs_ref.resolver().read(); + let path_name = path_name.to_string_lossy(); let fs_path = FsPath::from_fd_and_path(dirfd, &path_name)?; - ctx.thread_local - .borrow_fs() - .resolver() - .read() - .lookup_no_follow(&fs_path)? + let path = path_resolver.lookup_no_follow(&fs_path)?; + + match path.inode().read_link()? { + SymbolicLink::Plain(s) => s, + SymbolicLink::Path(path) => path_resolver.make_abs_path(&path).into_string(), + } }; - let linkpath = path.inode().read_link()?.to_string(); - let bytes = linkpath.as_bytes(); + let bytes = link_path.as_bytes(); let write_len = bytes.len().min(usr_buf_len); user_space.write_bytes(usr_buf_addr, &bytes[..write_len])?; Ok(SyscallReturn::Return(write_len as _)) diff --git a/kernel/src/syscall/rename.rs b/kernel/src/syscall/rename.rs index abb2a9644..400241c9a 100644 --- a/kernel/src/syscall/rename.rs +++ b/kernel/src/syscall/rename.rs @@ -44,7 +44,7 @@ pub fn sys_renameat2( let old_fs_path = FsPath::from_fd_and_path(old_dirfd, old_parent_path_name)?; (path_resolver.lookup(&old_fs_path)?, old_name) }; - let old_path = old_dir_path.lookup(old_name)?; + let old_path = path_resolver.lookup_at_path(&old_dir_path, old_name)?; if old_path.type_() != InodeType::Dir && old_path_name.ends_with('/') { return_errno_with_message!(Errno::ENOTDIR, "the old path is not a directory"); } @@ -60,8 +60,10 @@ pub fn sys_renameat2( }; // Check the absolute path - let old_abs_path = old_path.abs_path(); - let new_abs_path = new_dir_path.abs_path() + "/" + new_name; + // FIXME: Using string prefix matching to check for path containment is incorrect. + // It doesn't handle path components like '..' or '.' properly. + let old_abs_path = path_resolver.make_abs_path(&old_path).into_string(); + let new_abs_path = path_resolver.make_abs_path(&new_dir_path).into_string() + "/" + new_name; if new_abs_path.starts_with(&old_abs_path) { if new_abs_path.len() == old_abs_path.len() { return Ok(SyscallReturn::Return(0));