From 595504b3715cd08be5adb2fc7023f2ac8fb843e7 Mon Sep 17 00:00:00 2001 From: Mark Poliakov Date: Thu, 2 Jan 2025 21:29:02 +0200 Subject: [PATCH] vfs: check mountpoint before instantiating a new filesystem --- kernel/driver/fs/ext2/src/lib.rs | 2 +- kernel/libk/src/vfs/ioctx.rs | 8 ++++++-- kernel/libk/src/vfs/node/mod.rs | 25 ++++++++++++------------- kernel/src/fs/mod.rs | 18 +++++++++++++----- kernel/src/syscall/imp/mod.rs | 4 +--- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/kernel/driver/fs/ext2/src/lib.rs b/kernel/driver/fs/ext2/src/lib.rs index fc6348df..ff76cebd 100644 --- a/kernel/driver/fs/ext2/src/lib.rs +++ b/kernel/driver/fs/ext2/src/lib.rs @@ -91,7 +91,7 @@ impl Filesystem for Ext2Fs { } impl Ext2Fs { - pub async fn create<'a, I: Iterator>>( + pub async fn create<'a, I: IntoIterator>>( device: Arc, options: I, ) -> Result { diff --git a/kernel/libk/src/vfs/ioctx.rs b/kernel/libk/src/vfs/ioctx.rs index 11c77ee0..53c4ab11 100644 --- a/kernel/libk/src/vfs/ioctx.rs +++ b/kernel/libk/src/vfs/ioctx.rs @@ -158,7 +158,11 @@ impl IoContext { /// Makes a directory at given path become a "mountpoint" for the given filesystem root. /// When accessed, the target directory will return contents of the filesystem root instead of /// its own. Both the `target` path and `fs_root` Node must be directories. - pub fn mount>(&mut self, target: P, fs_root: NodeRef) -> Result<(), Error> { + pub fn mount, F: FnOnce() -> Result>( + &mut self, + target: P, + create_fs: F, + ) -> Result<(), Error> { if !self.uid.is_root() { return Err(Error::PermissionDenied); } @@ -169,7 +173,7 @@ impl IoContext { } let target_node = self._find(self.root.clone(), target.trim_start_separators(), true)?; - target_node.set_mountpoint_target(fs_root) + target_node.mount(create_fs) } /// Locates a [crate::Node] at given path and opens it with requested access options. If no diff --git a/kernel/libk/src/vfs/node/mod.rs b/kernel/libk/src/vfs/node/mod.rs index 89e3b2ea..0f3b0d47 100644 --- a/kernel/libk/src/vfs/node/mod.rs +++ b/kernel/libk/src/vfs/node/mod.rs @@ -410,25 +410,24 @@ impl Node { } } - pub(crate) fn set_mountpoint_target(self: &NodeRef, target: NodeRef) -> Result<(), Error> { + pub(crate) fn mount Result>( + self: &NodeRef, + create_fs: F, + ) -> Result<(), Error> { let directory = self.as_directory()?; let mut mountpoint = directory.mountpoint.lock(); - let mut target_parent_lock = target.parent.lock(); - if mountpoint.is_some() { // TODO Busy todo!(); } - if target_parent_lock.is_some() { - // TODO mount a filesystem more than once? - return Err(Error::AlreadyExists); - } - if !target.is_directory() { - return Err(Error::NotADirectory); - } - mountpoint.replace(target.clone()); - target_parent_lock.replace(self.clone()); + let fs_root = create_fs()?; + let mut parent = fs_root.parent.lock(); + assert!(fs_root.is_directory()); + assert!(parent.is_none()); + + mountpoint.replace(fs_root.clone()); + parent.replace(self.clone()); Ok(()) } @@ -436,7 +435,7 @@ impl Node { pub(crate) fn as_directory(&self) -> Result<&DirectoryData, Error> { match &self.data { NodeImpl::Directory(dir) => Ok(dir), - _ => Err(Error::InvalidFile), + _ => Err(Error::NotADirectory), } } diff --git a/kernel/src/fs/mod.rs b/kernel/src/fs/mod.rs index bb8836bd..cb38b5dd 100644 --- a/kernel/src/fs/mod.rs +++ b/kernel/src/fs/mod.rs @@ -7,7 +7,7 @@ use ext2::Ext2Fs; use libk::{ block, fs::{devfs, sysfs}, - vfs::{self, register_root, IoContext, NodeRef}, + vfs::{self, register_root, FilesystemMountOption, IoContext, NodeRef}, }; use libk_mm::{ address::{PhysicalAddress, Virtualize}, @@ -55,8 +55,8 @@ unsafe impl BlockAllocator for FileBlockAllocator { } } -/// Constructs an instance of a filesystem for given set of [MountOptions] -pub fn create_filesystem(ioctx: &mut IoContext, options: &MountOptions) -> Result { +/// Mounts an instance of a filesystem for given set of [MountOptions]. +pub fn mount_filesystem(ioctx: &mut IoContext, options: &MountOptions) -> Result<(), Error> { let Some(fs_name) = options.filesystem else { log::warn!("TODO: mount without filesystem type/fs probing not yet implemented"); return Err(Error::NotImplemented); @@ -69,8 +69,17 @@ pub fn create_filesystem(ioctx: &mut IoContext, options: &MountOptions) -> Resul } ioctx.find(None, path, true) }); - + let target = options.target; let options = vfs::parse_mount_options(options.options); + + ioctx.mount(target, move || create_filesystem(fs_name, source, options)) +} + +fn create_filesystem<'a, I: IntoIterator>>( + fs_name: &str, + source: Option>, + options: I, +) -> Result { let root = match fs_name { "devfs" => devfs::root().clone(), "sysfs" => sysfs::root().clone(), @@ -86,4 +95,3 @@ pub fn create_filesystem(ioctx: &mut IoContext, options: &MountOptions) -> Resul Ok(root) } -// } diff --git a/kernel/src/syscall/imp/mod.rs b/kernel/src/syscall/imp/mod.rs index e30a6750..b4b9ef7d 100644 --- a/kernel/src/syscall/imp/mod.rs +++ b/kernel/src/syscall/imp/mod.rs @@ -47,9 +47,7 @@ pub(crate) fn mount(options: &MountOptions<'_>) -> Result<(), Error> { let process = thread.process(); run_with_io(&process, |mut io| { - let fs_root = fs::create_filesystem(io.ioctx_mut(), options)?; - io.ioctx_mut().mount(options.target, fs_root)?; - Ok(()) + fs::mount_filesystem(io.ioctx_mut(), options) }) }