diff --git a/kernel/driver/fs/ext2/src/dir.rs b/kernel/driver/fs/ext2/src/dir.rs
index a59282ec..dc7aa0da 100644
--- a/kernel/driver/fs/ext2/src/dir.rs
+++ b/kernel/driver/fs/ext2/src/dir.rs
@@ -3,6 +3,7 @@ use core::{
any::Any,
mem::{self, MaybeUninit},
str::FromStr,
+ sync::atomic::{AtomicBool, Ordering},
};
use alloc::sync::Arc;
@@ -21,6 +22,7 @@ use crate::{data::FsRequiredFeatures, file::RegularNode, inode::InodeAccess, Dir
pub struct DirectoryNode {
fs: Arc<Ext2Fs>,
+ locked: AtomicBool,
pub(crate) inode: InodeAccess,
}
@@ -260,6 +262,7 @@ impl DirectoryNode {
Node::directory(
Self {
fs: fs.clone(),
+ locked: AtomicBool::new(false),
inode,
},
NodeFlags::empty(),
@@ -276,6 +279,7 @@ impl DirectoryNode {
let ino = inode.ino();
let this = Self {
fs: fs.clone(),
+ locked: AtomicBool::new(false),
inode,
};
@@ -291,6 +295,11 @@ impl DirectoryNode {
async fn create_entry(&self, name: &str, ino: u32) -> Result<(), Error> {
assert!(name.len() < 255);
+ // This directory is in the process of being unlinked and cannot be written to
+ if self.locked.load(Ordering::Acquire) {
+ return Err(Error::DoesNotExist);
+ }
+
{
let mut holder = self.inode.cache().get_mut(ino).await?;
{
@@ -376,9 +385,55 @@ impl DirectoryNode {
Ok(())
}
- async fn remove_entry(&self, name: &str) -> Result<(), Error> {
- // TODO if unlinking a directory, check that it only contains . and ..
- assert!(name.len() < 255);
+ async fn prepare_for_removal(&self) -> Result<(), Error> {
+ // Check that the directory doesn't have any children besides . and .. and set the locked
+ // flag to prevent anyone from adding entries while the directory is being unlinked
+ // TODO find a better way to do this
+ let mut holder = self.inode.get_mut().await?;
+ {
+ let inode = holder.write();
+ let n = inode.blocks(&self.fs);
+
+ for i in 0..n {
+ self.fs
+ .with_inode_block(&inode, i as _, |block| {
+ let iter = DirentIter::new(&self.fs, block, 0);
+
+ for (_, name, _) in iter {
+ if name != b"." && name != b".." {
+ return Err(Error::IsADirectory);
+ }
+ }
+
+ Ok(())
+ })
+ .await?;
+ }
+
+ self.locked.store(true, Ordering::Release);
+ }
+ holder.put().await?;
+ Ok(())
+ }
+
+ async fn remove_entry(&self, child: &Node, name: &str) -> Result<(), Error> {
+ if name.len() >= 255 || name.contains('/') || name.contains('\0') {
+ return Err(Error::InvalidArgument);
+ }
+ assert_eq!(
+ Arc::as_ptr(&child.filesystem().unwrap()).addr(),
+ Arc::as_ptr(&self.fs).addr()
+ );
+
+ let child_ino = match child.data_as_any() {
+ data if let Some(dir) = data.downcast_ref::<DirectoryNode>() => {
+ // Check that the directory is not empty
+ dir.prepare_for_removal().await?;
+ dir.inode.ino()
+ }
+ data if let Some(file) = data.downcast_ref::<RegularNode>() => file.inode.ino(),
+ _ => return Err(Error::InvalidOperation),
+ };
let ino = {
let mut holder = self.inode.get_mut().await?;
@@ -409,6 +464,7 @@ impl DirectoryNode {
};
let ino = ino.ok_or(Error::DoesNotExist)?;
+ assert_eq!(ino, child_ino);
log::info!("ext2: unlinked ino #{ino}");
// Decrement the refcount on the inode
@@ -582,11 +638,11 @@ impl DirectoryImpl for DirectoryNode {
Ok(())
}
- fn unlink_node(&self, _parent: &NodeRef, name: &str) -> Result<(), Error> {
+ fn unlink_node(&self, _parent: &NodeRef, child: &NodeRef, name: &str) -> Result<(), Error> {
if self.fs.force_readonly {
return Err(Error::ReadOnly);
}
- block!(self.remove_entry(name).await)?
+ block!(self.remove_entry(child, name).await)?
}
fn read_entries(
diff --git a/kernel/libk/src/vfs/node/ops.rs b/kernel/libk/src/vfs/node/ops.rs
index 0f2083fc..b089e8dc 100644
--- a/kernel/libk/src/vfs/node/ops.rs
+++ b/kernel/libk/src/vfs/node/ops.rs
@@ -139,23 +139,27 @@ impl Node {
let directory = self.as_directory()?;
let child = self.lookup_or_load(name, check)?;
- if child.is_directory() {
- return Err(Error::IsADirectory);
- }
-
// Detach the node in the real filesystem
- match directory.imp.unlink_node(self, name) {
- Ok(_) | Err(Error::NotImplemented) => (),
+ // If the filesystem implements unlink_node(), then checking if it's okay to
+ // remove a directory node is done by the filesystem itself, if not, check the
+ // cache.
+ let need_nonempty_check = match directory.imp.unlink_node(self, &child, name) {
+ Ok(_) => false,
+ Err(Error::NotImplemented) => true,
Err(err) => return Err(err),
+ };
+
+ if need_nonempty_check && let Ok(child_dir) = child.as_directory() {
+ let child_children = child_dir.children.lock();
+ if !child_children.is_empty() {
+ return Err(Error::IsADirectory);
+ }
+ directory.remove_child(name);
+ } else {
+ // Just unlink the node from the cache tree
+ directory.remove_child(name);
}
- // Detach the node in the tree cache
- {
- let mut children = directory.children.lock();
- children.retain(|(name_, _)| name != name_);
- }
-
- // TODO child.destroy() or something?
Ok(())
}
@@ -177,7 +181,7 @@ impl Node {
let node = source.lookup_or_load(source_name, access)?;
- match source_dir.imp.unlink_node(source, source_name) {
+ match source_dir.imp.unlink_node(source, &node, source_name) {
Ok(()) | Err(Error::NotImplemented) => (),
Err(error) => {
log::warn!("Rename: unlink {source_name}: {error:?}");
diff --git a/kernel/libk/src/vfs/node/traits.rs b/kernel/libk/src/vfs/node/traits.rs
index 33e31dc4..f58546c3 100644
--- a/kernel/libk/src/vfs/node/traits.rs
+++ b/kernel/libk/src/vfs/node/traits.rs
@@ -37,7 +37,6 @@ pub trait CommonImpl: Send + Sync {
}
/// Regular file interface
-#[allow(unused)]
pub trait RegularImpl: CommonImpl {
/// Opens the file for reading/writing and returns a `(start position, instance data)` tuple
fn open(
@@ -82,7 +81,6 @@ pub trait RegularImpl: CommonImpl {
}
/// Directory implementation
-#[allow(unused)]
pub trait DirectoryImpl: CommonImpl {
/// Opens a directory for reading its entries. Returns [DirectoryOpenPosition] to specify the
/// starting position.
@@ -111,7 +109,7 @@ pub trait DirectoryImpl: CommonImpl {
}
/// Removes an entry of the directory with given name
- fn unlink_node(&self, parent: &NodeRef, name: &str) -> Result<(), Error> {
+ fn unlink_node(&self, parent: &NodeRef, child: &NodeRef, name: &str) -> Result<(), Error> {
Err(Error::NotImplemented)
}
@@ -127,7 +125,6 @@ pub trait DirectoryImpl: CommonImpl {
}
/// Symbolic link interface
-#[allow(unused)]
pub trait SymlinkImpl: CommonImpl {
// /// Returns the target node (if such is available directly) of the link
// fn target(&self, node: &NodeRef) -> Result<NodeRef, Error> {