feat(sandbox): add fd sandbox

Cargo.toml

@@ -65,6 +65,7 @@ vm-fdt = "0.3"
 tempfile = "3.15.0"
 uuid = { version = "1.11.0", features = ["fast-rng", "v4"]}
 clean-path = "0.2.1"
+bimap = "0.6.3"
 
 [target.'cfg(target_os = "linux")'.dependencies]
 kvm-bindings = "0.10"

src/hypercall.rs

@@ -1,7 +1,7 @@
 use std::{
 	ffi::{CStr, CString, OsStr},
 	io::{self, Error, ErrorKind},
-	os::unix::ffi::OsStrExt,
+	os::{fd::IntoRawFd, unix::ffi::OsStrExt},
 };
 
 use uhyve_interface::{parameters::*, GuestPhysAddr, Hypercall, HypercallAddress, MAX_ARGC_ENVC};
@@ -86,6 +86,9 @@ pub fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut Uhy
 			// As host_path_c_string is a valid CString, this implementation is presumed to be safe.
 			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.unlink_guest_path(guest_path);
+			}
 		} else {
 			error!("The kernel requested to unlink() an unknown path ({guest_path}): Rejecting...");
 			sysunlink.ret = -ENOENT;
@@ -98,7 +101,6 @@ pub fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut Uhy
 
 /// Handles an open syscall by opening a file on the host.
 pub fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFileMap) {
-	// TODO: Keep track of file descriptors internally, just in case the kernel doesn't close them.
 	let requested_path_ptr = mem.host_address(sysopen.name).unwrap() as *const i8;
 	let mut flags = sysopen.flags & ALLOWED_OPEN_FLAGS;
 	if let Ok(guest_path) = unsafe { CStr::from_ptr(requested_path_ptr) }.to_str() {
@@ -116,6 +118,10 @@ pub fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFile
 
 			sysopen.ret =
 				unsafe { libc::open(host_path_c_string.as_c_str().as_ptr(), flags, sysopen.mode) };
+
+			if sysopen.ret >= 0 {
+				file_map.insert_fd_path(sysopen.ret, guest_path);
+			}
 		} else {
 			debug!("Attempting to open a temp file for {:#?}...", guest_path);
 			// Existing files that already exist should be in the file map, not here.
@@ -126,6 +132,9 @@ pub fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFile
 			let host_path_c_string = file_map.create_temporary_file(guest_path);
 			let new_host_path = host_path_c_string.as_c_str().as_ptr();
 			sysopen.ret = unsafe { libc::open(new_host_path, flags, sysopen.mode) };
+			if sysopen.ret >= 0 {
+				file_map.insert_fd_path(sysopen.ret.into_raw_fd(), guest_path);
+			}
 		}
 	} else {
 		error!("The kernel requested to open() a path that is not valid UTF-8. Rejecting...");
@@ -134,33 +143,47 @@ 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) {
-	unsafe {
-		sysclose.ret = libc::close(sysclose.fd);
+pub fn close(sysclose: &mut CloseParams, file_map: &mut UhyveFileMap) {
+	if file_map.is_fd_closable(sysclose.fd.into_raw_fd()) {
+		if sysclose.fd > 2 {
+			unsafe { sysclose.ret = libc::close(sysclose.fd) }
+			file_map.close_fd(sysclose.fd);
+		} else {
+			// Ignore closes of stdin, stdout and stderr that would
+			// otherwise affect Uhyve
+			sysclose.ret = 0
+		}
+	} else {
+		sysclose.ret = -EBADF
 	}
 }
 
 /// Handles an read syscall on the host.
-pub fn read(mem: &MmapMemory, sysread: &mut ReadParams) {
-	unsafe {
-		let bytes_read = libc::read(
-			sysread.fd,
-			mem.host_address(virt_to_phys(sysread.buf, mem, BOOT_PML4).unwrap())
-				.unwrap() as *mut libc::c_void,
-			sysread.len,
-		);
-		if bytes_read >= 0 {
-			sysread.ret = bytes_read;
-		} else {
-			sysread.ret = -1;
+pub fn read(mem: &MmapMemory, sysread: &mut ReadParams, file_map: &mut UhyveFileMap) {
+	if file_map.is_fd_present(sysread.fd.into_raw_fd()) {
+		unsafe {
+			let bytes_read = libc::read(
+				sysread.fd,
+				mem.host_address(virt_to_phys(sysread.buf, mem, BOOT_PML4).unwrap())
+					.unwrap() as *mut libc::c_void,
+				sysread.len,
+			);
+			if bytes_read >= 0 {
+				sysread.ret = bytes_read;
+			} else {
+				sysread.ret = -1
+			}
 		}
+	} else {
+		sysread.ret = -EBADF as isize;
 	}
 }
 
 /// Handles an write syscall on the host.
 pub fn write<B: VirtualizationBackend>(
 	parent_vm: &UhyveVm<B>,
 	syswrite: &WriteParams,
+	file_map: &mut UhyveFileMap,
 ) -> io::Result<()> {
 	let mut bytes_written: usize = 0;
 	while bytes_written != syswrite.len {
@@ -171,8 +194,7 @@ pub fn write<B: VirtualizationBackend>(
 		)
 		.unwrap();
 
-		if syswrite.fd == 1 {
-			// fd 0 is stdout
+		if syswrite.fd == 1 || syswrite.fd == 2 {
 			let bytes = unsafe {
 				parent_vm
 					.mem
@@ -185,6 +207,11 @@ pub fn write<B: VirtualizationBackend>(
 					})?
 			};
 			return parent_vm.serial_output(bytes);
+		} 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.
+			return Ok(());
 		}
 
 		unsafe {
@@ -215,10 +242,16 @@ pub fn write<B: VirtualizationBackend>(
 }
 
 /// Handles an write syscall on the host.
-pub fn lseek(syslseek: &mut LseekParams) {
-	unsafe {
-		syslseek.offset =
-			libc::lseek(syslseek.fd, syslseek.offset as i64, syslseek.whence) as isize;
+pub fn lseek(syslseek: &mut LseekParams, file_map: &mut UhyveFileMap) {
+	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;
+		}
+	} else {
+		// TODO: Return -EBADF to the ret field, as soon as it is implemented for LseekParams
+		warn!("lseek attempted to use an unknown file descriptor");
+		syslseek.offset = -1
 	}
 }
 

src/isolation/filemap.rs

@@ -1,12 +1,16 @@
 use std::{
-	collections::HashMap,
+	collections::{HashMap, HashSet},
 	ffi::{CString, OsString},
-	fs::canonicalize,
+	fs::{canonicalize, File},
 	io::ErrorKind,
-	os::unix::ffi::OsStrExt,
+	os::{
+		fd::{FromRawFd, RawFd},
+		unix::ffi::OsStrExt,
+	},
 	path::{absolute, PathBuf},
 };
 
+use bimap::BiHashMap;
 use clean_path::clean;
 use tempfile::TempDir;
 use uuid::Uuid;
@@ -18,6 +22,8 @@ use crate::isolation::tempdir::create_temp_dir;
 pub struct UhyveFileMap {
 	files: HashMap<String, OsString>,
 	tempdir: TempDir,
+	fdmap: BiHashMap<RawFd, String>,
+	unlinkedfd: HashSet<RawFd>,
 }
 
 impl UhyveFileMap {
@@ -33,6 +39,8 @@ impl UhyveFileMap {
 				.map(Result::unwrap)
 				.collect(),
 			tempdir: create_temp_dir(),
+			fdmap: BiHashMap::new(),
+			unlinkedfd: HashSet::new(),
 		}
 	}
 
@@ -66,6 +74,7 @@ impl UhyveFileMap {
 			.files
 			.get(&requested_guest_pathbuf.display().to_string())
 			.map(OsString::from);
+		debug!("get_host_path (host_path): {:#?}", host_path);
 		if host_path.is_some() {
 			host_path
 		} else {
@@ -113,10 +122,102 @@ impl UhyveFileMap {
 			.path()
 			.join(Uuid::new_v4().to_string())
 			.into_os_string();
+		debug!("create_temporary_file (host_path): {:#?}", host_path);
 		let ret = CString::new(host_path.as_bytes()).unwrap();
 		self.files.insert(String::from(guest_path), host_path);
 		ret
 	}
+
+	// Drops all file descriptors present in fdmap and unlinkedfd.
+	pub fn drop_all_fds(&self) {
+		for (fd, _) in &self.fdmap {
+			unsafe { File::from_raw_fd(*fd) };
+		}
+		for fd in self.unlinkedfd.iter() {
+			unsafe { File::from_raw_fd(*fd) };
+		}
+	}
+
+	/// 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;
+		}
+		false
+	}
+
+	/// Inserts a bidirectional file descriptor-path association.
+	///
+	/// * `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) {
+		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],
+	/// 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 remove_fd(&mut self, fd: RawFd) {
+		debug!("remove_fd: {:#?}", &self.fdmap);
+		if fd > 2 {
+			self.fdmap.remove_by_left(&fd);
+		}
+	}
+
+	/// Removes an entry from UhyveFileMap. This is only used by [crate::hypercall::unlink],
+	/// under the expectation that a new temporary file will be created if the guest
+	/// attempts to open a file of the same path again.
+	///
+	/// * `guest_path` - The path of the file being removed.
+	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);
+	}
+}
+
+impl Drop for UhyveFileMap {
+	fn drop(&mut self) {
+		self.drop_all_fds();
+	}
 }
 
 #[cfg(test)]

src/linux/x86_64/kvm_cpu.rs

@@ -448,20 +448,30 @@ impl VirtualCPU for KvmCpu {
 								Hypercall::Exit(sysexit) => {
 									return Ok(VcpuStopReason::Exit(sysexit.arg));
 								}
-								Hypercall::FileClose(sysclose) => hypercall::close(sysclose),
-								Hypercall::FileLseek(syslseek) => hypercall::lseek(syslseek),
+								Hypercall::FileClose(sysclose) => hypercall::close(
+									sysclose,
+									&mut self.parent_vm.file_mapping.lock().unwrap(),
+								),
+								Hypercall::FileLseek(syslseek) => hypercall::lseek(
+									syslseek,
+									&mut self.parent_vm.file_mapping.lock().unwrap(),
+								),
 								Hypercall::FileOpen(sysopen) => hypercall::open(
 									&self.parent_vm.mem,
 									sysopen,
 									&mut self.parent_vm.file_mapping.lock().unwrap(),
 								),
-								Hypercall::FileRead(sysread) => {
-									hypercall::read(&self.parent_vm.mem, sysread)
-								}
-								Hypercall::FileWrite(syswrite) => {
-									hypercall::write(&self.parent_vm, syswrite)
-										.map_err(|_e| HypervisorError::new(libc::EFAULT))?
-								}
+								Hypercall::FileRead(sysread) => hypercall::read(
+									&self.parent_vm.mem,
+									sysread,
+									&mut self.parent_vm.file_mapping.lock().unwrap(),
+								),
+								Hypercall::FileWrite(syswrite) => hypercall::write(
+									&self.parent_vm,
+									syswrite,
+									&mut self.parent_vm.file_mapping.lock().unwrap(),
+								)
+								.map_err(|_e| HypervisorError::new(libc::EFAULT))?,
 								Hypercall::FileUnlink(sysunlink) => hypercall::unlink(
 									&self.parent_vm.mem,
 									sysunlink,