fix(sandbox): fix close after unlink

If an unlink takes place while the file is open, this would make the
subsequent close fail, as soon as the file object goes out of scope
and gets dropped. We introduced a HashSet that temporarily caches
such unlinked file descriptors.

Author: n0toose

src/hypercall.rs

@@ -87,7 +87,7 @@ pub fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut Uhy
 			let host_path_c_string = CString::new(host_path.as_bytes()).unwrap();
 			sysunlink.ret = unsafe { libc::unlink(host_path_c_string.as_c_str().as_ptr()) };
 			if sysunlink.ret >= 0 {
-				file_map.remove_guest_path(guest_path);
+				file_map.unlink_guest_path(guest_path);
 			}
 		} else {
 			error!("The kernel requested to unlink() an unknown path ({guest_path}): Rejecting...");
@@ -144,10 +144,10 @@ pub fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFile
 
 /// Handles an close syscall by closing the file on the host.
 pub fn close(sysclose: &mut CloseParams, file_map: &mut UhyveFileMap) {
-	if file_map.get_fd_presence(sysclose.fd.into_raw_fd()) {
+	if file_map.is_fd_closable(sysclose.fd.into_raw_fd()) {
 		if sysclose.ret > 2 {
 			unsafe { sysclose.ret = libc::close(sysclose.fd) }
-			file_map.remove_fd(sysclose.fd);
+			file_map.close_fd(sysclose.fd);
 		} else {
 			// Ignore closes of stdin, stdout and stderr that would
 			// otherwise affect Uhyve
@@ -160,7 +160,7 @@ pub fn close(sysclose: &mut CloseParams, file_map: &mut UhyveFileMap) {
 
 /// Handles an read syscall on the host.
 pub fn read(mem: &MmapMemory, sysread: &mut ReadParams, file_map: &mut UhyveFileMap) {
-	if file_map.get_fd_presence(sysread.fd.into_raw_fd()) {
+	if file_map.is_fd_present(sysread.fd.into_raw_fd()) {
 		unsafe {
 			let bytes_read = libc::read(
 				sysread.fd,
@@ -207,7 +207,7 @@ pub fn write<B: VirtualizationBackend>(
 					})?
 			};
 			return parent_vm.serial_output(bytes);
-		} else if !file_map.get_fd_presence(syswrite.fd.into_raw_fd()) {
+		} else if !file_map.is_fd_present(syswrite.fd.into_raw_fd()) {
 			// We don't write anything if the file descriptor is not available,
 			// but this is OK for now, as we have no means of returning an error codee
 			// and writes are not necessarily guaranteed to write anything.
@@ -243,7 +243,7 @@ pub fn write<B: VirtualizationBackend>(
 
 /// Handles an write syscall on the host.
 pub fn lseek(syslseek: &mut LseekParams, file_map: &mut UhyveFileMap) {
-	if file_map.get_fd_presence(syslseek.fd.into_raw_fd()) {
+	if file_map.is_fd_present(syslseek.fd.into_raw_fd()) {
 		unsafe {
 			syslseek.offset =
 				libc::lseek(syslseek.fd, syslseek.offset as i64, syslseek.whence) as isize;

src/isolation/filemap.rs

@@ -1,5 +1,5 @@
 use std::{
-	collections::HashMap,
+	collections::{HashMap, HashSet},
 	ffi::{CString, OsString},
 	fs::canonicalize,
 	io::ErrorKind,
@@ -20,6 +20,7 @@ pub struct UhyveFileMap {
 	files: HashMap<String, OsString>,
 	tempdir: TempDir,
 	fdmap: BiHashMap<RawFd, String>,
+	unlinkedfd: HashSet<RawFd>,
 }
 
 impl UhyveFileMap {
@@ -36,6 +37,7 @@ impl UhyveFileMap {
 				.collect(),
 			tempdir: create_temp_dir(),
 			fdmap: BiHashMap::new(),
+			unlinkedfd: HashSet::new(),
 		}
 	}
 
@@ -123,8 +125,20 @@ impl UhyveFileMap {
 		ret
 	}
 
-	pub fn get_fd_presence(&mut self, fd: RawFd) -> bool {
-		debug!("get_fd_presence: {:#?}", &self.fdmap);
+	/// Checks whether the fd is mapped to a guest path or belongs
+	/// to an unlinked file.
+	///
+	/// * `fd` - The opened guest path's file descriptor.
+	pub fn is_fd_closable(&mut self, fd: RawFd) -> bool {
+		debug!("is_fd_closable: {:#?}", &self.fdmap);
+		self.is_fd_present(fd) || self.unlinkedfd.contains(&fd)
+	}
+
+	/// Checks whether the fd is mapped to a guest path.
+	///
+	/// * `fd` - The opened guest path's file descriptor.
+	pub fn is_fd_present(&mut self, fd: RawFd) -> bool {
+		debug!("is_fd_present: {:#?}", &self.fdmap);
 		if (fd >= 0 && self.fdmap.contains_left(&fd)) || (0..=2).contains(&fd) {
 			return true;
 		}
@@ -136,10 +150,28 @@ impl UhyveFileMap {
 	/// * `fd` - The opened guest path's file descriptor.
 	/// * `guest_path` - The guest path.
 	pub fn insert_fd_path(&mut self, fd: RawFd, guest_path: &str) {
-		debug!("insert_fd_path: {:#?}", &self.fdmap);
 		if fd > 2 {
 			self.fdmap.insert(fd, guest_path.into());
 		}
+		debug!("insert_fd_path: {:#?}", &self.fdmap);
+	}
+
+	/// Removes an fd from UhyveFileMap. This is only used by [crate::hypercall::close],
+	/// under the expectation that a new temporary file will be created if the guest
+	/// attempts to open a file of the same path again.
+	///
+	/// * `fd` - The file descriptor of the file being removed.
+	pub fn close_fd(&mut self, fd: RawFd) {
+		debug!("close_fd: {:#?}", &self.fdmap);
+		// The file descriptor in fdclosed is supposedly still alive.
+		// It is safe to assume that the host OS will not assign the
+		// same file descriptor to another opened file, until _after_
+		// the file has been closed.
+		if let Some(&fd) = self.unlinkedfd.get(&fd) {
+			self.unlinkedfd.remove(&fd);
+		} else {
+			self.remove_fd(fd);
+		}
 	}
 
 	/// Removes an fd from UhyveFileMap. This is only used by [crate::hypercall::close],
@@ -159,10 +191,13 @@ impl UhyveFileMap {
 	/// attempts to open a file of the same path again.
 	///
 	/// * `guest_path` - The path of the file being removed.
-	pub fn remove_guest_path(&mut self, guest_path: &str) {
-		debug!("remove_guest_path: {:#?}", guest_path);
+	pub fn unlink_guest_path(&mut self, guest_path: &str) {
+		debug!("unlink_guest_path: {:#?}", &guest_path);
+		if let Some(fd) = self.fdmap.get_by_right(guest_path) {
+			self.unlinkedfd.insert(*fd);
+			self.fdmap.remove_by_right(guest_path);
+		}
 		self.files.remove(guest_path);
-		self.fdmap.remove_by_right(guest_path);
 	}
 }