diff --git a/admin/travis-build.sh b/admin/travis-build.sh index d1a03a0..5e86176 100755 --- a/admin/travis-build.sh +++ b/admin/travis-build.sh @@ -91,16 +91,10 @@ do_rust() { TestProfiling_FileProfiles TestProfiling_BadConfiguration TestReadOnly_Attributes - TestReadWrite_Remove TestReadWrite_InodeReassignedAfterRecreation - TestReadWrite_FstatOnDeletedNode - TestReadWrite_FtruncateOnDeletedFile TestReadWrite_NestedMappingsInheritDirectoryProperties TestReadWrite_RenameFile TestReadWrite_MoveFile - TestReadWrite_FchmodOnDeletedNode - TestReadWrite_FchownOnDeletedNode - TestReadWrite_FutimesOnDeletedNode TestReconfiguration_Streams TestReconfiguration_Steps TestReconfiguration_Unmap diff --git a/src/lib.rs b/src/lib.rs index e5bccc2..1025a4e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -482,9 +482,17 @@ impl fuse::Filesystem for SandboxFS { panic!("Required RW operation not yet implemented"); } - fn rmdir(&mut self, _req: &fuse::Request, _parent: u64, _name: &OsStr, - _reply: fuse::ReplyEmpty) { - panic!("Required RW operation not yet implemented"); + fn rmdir(&mut self, _req: &fuse::Request, parent: u64, name: &OsStr, reply: fuse::ReplyEmpty) { + let dir_node = self.find_node(parent); + if !dir_node.writable() { + reply.error(Errno::EPERM as i32); + return; + } + + match dir_node.rmdir(name) { + Ok(_) => reply.ok(), + Err(e) => reply.error(e.errno_as_i32()), + } } fn setattr(&mut self, _req: &fuse::Request, inode: u64, mode: Option, uid: Option, @@ -538,9 +546,17 @@ impl fuse::Filesystem for SandboxFS { } } - fn unlink(&mut self, _req: &fuse::Request, _parent: u64, _name: &OsStr, - _reply: fuse::ReplyEmpty) { - panic!("Required RW operation not yet implemented"); + fn unlink(&mut self, _req: &fuse::Request, parent: u64, name: &OsStr, reply: fuse::ReplyEmpty) { + let dir_node = self.find_node(parent); + if !dir_node.writable() { + reply.error(Errno::EPERM as i32); + return; + } + + match dir_node.unlink(name) { + Ok(_) => reply.ok(), + Err(e) => reply.error(e.errno_as_i32()), + } } fn write(&mut self, _req: &fuse::Request, _inode: u64, fh: u64, offset: i64, data: &[u8], diff --git a/src/nodes/dir.rs b/src/nodes/dir.rs index 5fb3fe6..8f37d08 100644 --- a/src/nodes/dir.rs +++ b/src/nodes/dir.rs @@ -254,12 +254,12 @@ impl Dir { if let Some(path) = &state.underlying_path { let fs_attr = fs::symlink_metadata(path)?; if !fs_attr.is_dir() { - warn!("Path {:?} backing a directory node is no longer a directory; got {:?}", - path, fs_attr.file_type()); + warn!("Path {} backing a directory node is no longer a directory; got {:?}", + path.display(), fs_attr.file_type()); return Err(KernelError::from_errno(errno::Errno::EIO)); } state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr); - }; + } Ok(state.attr) } @@ -348,6 +348,22 @@ impl Dir { } } } + + /// Common implementation for the `rmdir` and `unlink` operations. + /// + /// The behavior of these operations differs only in the syscall we invoke to delete the + /// underlying entry, which is passed in as the `remove` parameter. + fn remove_any(&self, name: &OsStr, remove: R) -> NodeResult<()> + where R: Fn(&PathBuf) -> io::Result<()> { + let mut state = self.state.lock().unwrap(); + let path = Dir::get_writable_path(&mut state, name)?; + + remove(&path)?; + + state.children[name].node.delete(); + state.children.remove(name); + Ok(()) + } } impl Node for Dir { @@ -363,6 +379,14 @@ impl Node for Dir { fuse::FileType::Directory } + fn delete(&self) { + let mut state = self.state.lock().unwrap(); + assert!( + state.underlying_path.is_some(), + "Delete already called or trying to delete an explicit mapping"); + state.underlying_path = None; + } + fn map(&self, components: &[Component], underlying_path: &Path, writable: bool, ids: &IdGenerator, cache: &Cache) -> Result<(), Error> { let (name, remainder) = split_components(components); @@ -494,14 +518,14 @@ impl Node for Dir { })) } + fn rmdir(&self, name: &OsStr) -> NodeResult<()> { + // TODO(jmmv): Figure out how to remove the redundant closure. + #[cfg_attr(feature = "cargo-clippy", allow(redundant_closure))] + self.remove_any(name, |p| fs::remove_dir(p)) + } + fn setattr(&self, delta: &AttrDelta) -> NodeResult { let mut state = self.state.lock().unwrap(); - - // setattr only gets called on writable nodes, which must have an underlying path. This - // assertion will go away when we have the ability to delete nodes as those may still - // exist in memory but have no corresponding underlying path. - debug_assert!(state.underlying_path.is_some()); - state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; Ok(state.attr) } @@ -515,4 +539,10 @@ impl Node for Dir { Dir::post_create_lookup(self.writable, &mut state, &path, name, fuse::FileType::Symlink, ids, cache) } + + fn unlink(&self, name: &OsStr) -> NodeResult<()> { + // TODO(jmmv): Figure out how to remove the redundant closure. + #[cfg_attr(feature = "cargo-clippy", allow(redundant_closure))] + self.remove_any(name, |p| fs::remove_file(p)) + } } diff --git a/src/nodes/file.rs b/src/nodes/file.rs index 9b19f0d..deb1676 100644 --- a/src/nodes/file.rs +++ b/src/nodes/file.rs @@ -58,7 +58,7 @@ pub struct File { /// Holds the mutable data of a file node. struct MutableFile { - underlying_path: PathBuf, + underlying_path: Option, attr: fuse::FileAttr, } @@ -85,8 +85,8 @@ impl File { let attr = conv::attr_fs_to_fuse(underlying_path, inode, &fs_attr); let state = MutableFile { - underlying_path: PathBuf::from(underlying_path), - attr, + underlying_path: Some(PathBuf::from(underlying_path)), + attr: attr, }; Arc::new(File { inode, writable, state: Mutex::from(state) }) @@ -94,13 +94,15 @@ impl File { /// Same as `getattr` but with the node already locked. fn getattr_locked(inode: u64, state: &mut MutableFile) -> NodeResult { - let fs_attr = fs::symlink_metadata(&state.underlying_path)?; - if !File::supports_type(fs_attr.file_type()) { - warn!("Path {:?} backing a file node is no longer a file; got {:?}", - &state.underlying_path, fs_attr.file_type()); - return Err(KernelError::from_errno(errno::Errno::EIO)); + if let Some(path) = &state.underlying_path { + let fs_attr = fs::symlink_metadata(path)?; + if !File::supports_type(fs_attr.file_type()) { + warn!("Path {} backing a file node is no longer a file; got {:?}", + path.display(), fs_attr.file_type()); + return Err(KernelError::from_errno(errno::Errno::EIO)); + } + state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr); } - state.attr = conv::attr_fs_to_fuse(&state.underlying_path, inode, &fs_attr); Ok(state.attr) } @@ -120,6 +122,14 @@ impl Node for File { state.attr.kind } + fn delete(&self) { + let mut state = self.state.lock().unwrap(); + assert!( + state.underlying_path.is_some(), + "Delete already called or trying to delete an explicit mapping"); + state.underlying_path = None; + } + fn getattr(&self) -> NodeResult { let mut state = self.state.lock().unwrap(); File::getattr_locked(self.inode, &mut state) @@ -129,13 +139,15 @@ impl Node for File { let state = self.state.lock().unwrap(); let options = conv::flags_to_openoptions(flags, self.writable)?; - let file = options.open(&state.underlying_path)?; + let path = state.underlying_path.as_ref().expect( + "Don't know how to handle a request to reopen a deleted file"); + let file = options.open(path)?; Ok(Arc::from(file)) } fn setattr(&self, delta: &AttrDelta) -> NodeResult { let mut state = self.state.lock().unwrap(); - state.attr = setattr(Some(&state.underlying_path), &state.attr, delta)?; + state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; Ok(state.attr) } } diff --git a/src/nodes/mod.rs b/src/nodes/mod.rs index a72bd9a..d9ed116 100644 --- a/src/nodes/mod.rs +++ b/src/nodes/mod.rs @@ -282,6 +282,12 @@ pub trait Node { /// `Symlink`s (on which we don't allow type changes at all), it's OK. fn file_type_cached(&self) -> fuse::FileType; + /// Marks the node as deleted (though the in-memory representation remains). + /// + /// The implication of this is that a node loses its backing underlying path once this method + /// is called. + fn delete(&self); + /// Maps a path onto a node and creates intermediate components as immutable directories. /// /// `_components` is the path to map, broken down into components, and relative to the current @@ -358,6 +364,11 @@ pub trait Node { panic!("Not implemented"); } + /// Deletes the empty directory `_name`. + fn rmdir(&self, _name: &OsStr) -> NodeResult<()> { + panic!("Not implemented"); + } + /// Sets one or more properties of the node's metadata. fn setattr(&self, _delta: &AttrDelta) -> NodeResult; @@ -372,4 +383,9 @@ pub trait Node { -> NodeResult<(Arc, fuse::FileAttr)> { panic!("Not implemented") } + + /// Deletes the file `_name`. + fn unlink(&self, _name: &OsStr) -> NodeResult<()> { + panic!("Not implemented"); + } } diff --git a/src/nodes/symlink.rs b/src/nodes/symlink.rs index 0e58eae..199c91d 100644 --- a/src/nodes/symlink.rs +++ b/src/nodes/symlink.rs @@ -30,7 +30,7 @@ pub struct Symlink { /// Holds the mutable data of a symlink node. struct MutableSymlink { - underlying_path: PathBuf, + underlying_path: Option, attr: fuse::FileAttr, } @@ -52,8 +52,8 @@ impl Symlink { let attr = conv::attr_fs_to_fuse(underlying_path, inode, &fs_attr); let state = MutableSymlink { - underlying_path: PathBuf::from(underlying_path), - attr, + underlying_path: Some(PathBuf::from(underlying_path)), + attr: attr, }; Arc::new(Symlink { inode, writable, state: Mutex::from(state) }) @@ -61,13 +61,15 @@ impl Symlink { /// Same as `getattr` but with the node already locked. fn getattr_locked(inode: u64, state: &mut MutableSymlink) -> NodeResult { - let fs_attr = fs::symlink_metadata(&state.underlying_path)?; - if !fs_attr.file_type().is_symlink() { - warn!("Path {:?} backing a symlink node is no longer a symlink; got {:?}", - &state.underlying_path, fs_attr.file_type()); - return Err(KernelError::from_errno(errno::Errno::EIO)); + if let Some(path) = &state.underlying_path { + let fs_attr = fs::symlink_metadata(path)?; + if !fs_attr.file_type().is_symlink() { + warn!("Path {} backing a symlink node is no longer a symlink; got {:?}", + path.display(), fs_attr.file_type()); + return Err(KernelError::from_errno(errno::Errno::EIO)); + } + state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr); } - state.attr = conv::attr_fs_to_fuse(&state.underlying_path, inode, &fs_attr); Ok(state.attr) } @@ -86,6 +88,14 @@ impl Node for Symlink { fuse::FileType::Symlink } + fn delete(&self) { + let mut state = self.state.lock().unwrap(); + assert!( + state.underlying_path.is_some(), + "Delete already called or trying to delete an explicit mapping"); + state.underlying_path = None; + } + fn getattr(&self) -> NodeResult { let mut state = self.state.lock().unwrap(); Symlink::getattr_locked(self.inode, &mut state) @@ -94,12 +104,14 @@ impl Node for Symlink { fn readlink(&self) -> NodeResult { let state = self.state.lock().unwrap(); - Ok(fs::read_link(&state.underlying_path)?) + let path = state.underlying_path.as_ref().expect( + "There is no known API to get the target of a deleted symlink"); + Ok(fs::read_link(path)?) } fn setattr(&self, delta: &AttrDelta) -> NodeResult { let mut state = self.state.lock().unwrap(); - state.attr = setattr(Some(&state.underlying_path), &state.attr, delta)?; + state.attr = setattr(state.underlying_path.as_ref(), &state.attr, delta)?; Ok(state.attr) } }