feat: handle hermit images + config nested in image

Author: fogti

Cargo.lock

@@ -71,6 +71,10 @@ uuid = { version = "1.18.1", features = ["fast-rng", "v4"]}
 toml = "0.9.8"
 serde = { version = "1.0", features = ["derive"] }
 merge = { version = "0.2" }
+memmap2 = "0.9.8"
+
+[dependencies.hermit-image-reader]
+git = "https://codeberg.org/hermit-os-contrib/hermit-image-reader.git"
 
 [target.'cfg(target_os = "linux")'.dependencies]
 kvm-bindings = "0.14"

Cargo.toml

@@ -5,7 +5,7 @@ use std::{fs, num::ParseIntError, path::PathBuf, process, str::FromStr};
 use clap::{Command, CommandFactory, Parser, error::ErrorKind};
 use core_affinity::CoreId;
 use env_logger::Builder;
-use log::{LevelFilter, info};
+use log::{LevelFilter, info, warn};
 use merge::Merge;
 use serde::Deserialize;
 use thiserror::Error;
@@ -146,6 +146,17 @@ struct UhyveArgs {
 	#[serde(skip)]
 	#[merge(strategy = merge::option::overwrite_none)]
 	pub config: Option<PathBuf>,
+
+	/// Kernel image file
+	///
+	/// Reads configuration and associated data from a locally given path.
+	///
+	/// If `--config CONFIG_FILE` is given, the configurations are merged.
+	/// Also keep in mind that the configuration formats differ.
+	#[clap(long)]
+	#[serde(skip)]
+	#[merge(strategy = merge::option::overwrite_none)]
+	pub image: Option<PathBuf>,
 }
 
 /// Arguments for memory resources allocated to the guest (both guest and host).
@@ -382,6 +393,7 @@ impl From<Args> for Params {
 					#[cfg(target_os = "linux")]
 					gdb_port,
 					config: _,
+					image: _,
 				},
 			memory:
 				MemoryArgs {
@@ -451,6 +463,59 @@ fn read_toml_contents(toml_path: &PathBuf) -> Result<Args, Box<dyn std::error::E
 ///
 /// This overrides missing CLI configs (None) with configs obtained from config file.
 fn load_vm_config(args: &mut Args) {
+	// If there is an image given, try to find a config file in it, and load that
+	if let Some(image_file) = &args.uhyve.image {
+		// Unfortunately, we have to read the image twice (unless we figure out a way to cache it...)
+		let data = std::fs::read(image_file).expect("unable to read supplied hermit image file");
+		let decompressed = hermit_image_reader::decompress_image(&data[..])
+			.expect("unable to decompress supplied hermit image file");
+
+		let entry = hermit_image_reader::ImageParser::new(&decompressed[..])
+			.filter_map(|i| i.ok())
+			.filter(|i| {
+				if let Ok(name) = str::from_utf8(&*i.name) {
+					name == hermit_image_reader::config::DEFAULT_CONFIG_NAME
+				} else {
+					false
+				}
+			})
+			// for `tar` and similar formats, duplicate entries are allowed,
+			// and the latest entry wins
+			.last();
+		if let Some(entry) = entry {
+			let mut config = hermit_image_reader::config::parse(entry.value)
+				.expect("unable to parse config of supplied hermit image file");
+
+			// .input
+			args.guest.kernel_args.append(&mut config.input.kernel_args);
+			if !config.input.app_args.is_empty() {
+				args.guest.kernel_args.push("--".to_string());
+				args.guest.kernel_args.append(&mut config.input.app_args);
+			}
+			args.guest.env_vars.append(&mut config.input.env_vars);
+			// don't pass privileged env-var commands through
+			args.guest.env_vars.retain(|i| i.contains('='));
+
+			// .requirements
+			// TODO: currently no corresponding options exist
+
+			let image_file_str = image_file.to_str().unwrap();
+
+			// .kernel
+			args.guest.kernel = format!("{}:{}", image_file_str, config.kernel).into();
+
+			// .file_mapping
+			// TODO: improve this
+			for (key, value) in config.file_mapping {
+				args.uhyve
+					.file_mapping
+					.push(format!("{}:{}:{}", image_file_str, key, value));
+			}
+		} else {
+			warn!("Supplied hermit image file doesn't contain a configuration file");
+		}
+	}
+
 	// Tries to read arguments from a configuration file. If it doesn't exist or if
 	// parsing is not possible, panic.
 	if let Some(config_file) = args.get_config_file() {
@@ -624,6 +689,7 @@ mod tests {
 				#[cfg(target_os = "linux")]
 				gdb_port: None,
 				config: Some(PathBuf::from("config.txt")),
+				image: None,
 			},
 			memory: MemoryArgs {
 				memory_size: None,
@@ -683,6 +749,7 @@ mod tests {
 				#[cfg(target_os = "linux")]
 				gdb_port: Some(1),
 				config: Some(PathBuf::from("config.txt")),
+				image: None,
 			},
 			memory: MemoryArgs {
 				memory_size: Some(GuestMemorySize::from_str("16MiB").unwrap()),

src/bin/uhyve.rs

@@ -1,13 +1,16 @@
 use std::{
 	ffi::{CStr, CString, OsStr},
+	fs::File,
 	io::{self, Error, ErrorKind},
-	os::{fd::IntoRawFd, unix::ffi::OsStrExt},
+	os::{fd::IntoRawFd, unix::ffi::OsStrExt, unix::io::FromRawFd},
+	sync::Arc,
 };
 
+use clean_path::clean;
 use uhyve_interface::{GuestPhysAddr, Hypercall, HypercallAddress, MAX_ARGC_ENVC, parameters::*};
 
 use crate::{
-	isolation::filemap::UhyveFileMap,
+	isolation::filemap::{HermitImageThinFile, MappedFileMutRef, UhyveFileMap},
 	mem::{MemoryError, MmapMemory},
 	params::EnvVars,
 	virt_to_phys,
@@ -88,8 +91,44 @@ pub fn unlink(mem: &MmapMemory, sysunlink: &mut UnlinkParams, file_map: &mut Uhy
 	sysunlink.ret = if let Some(host_path) = file_map.get_host_path(guest_path) {
 		// We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes
 		// 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();
-		unsafe { libc::unlink(host_path_c_string.as_c_str().as_ptr()) }
+		match host_path {
+			MappedFileMutRef::OnHost(oh) => {
+				let host_path_c_string = CString::new(oh.as_os_str().as_bytes()).unwrap();
+				unsafe { libc::unlink(host_path_c_string.as_c_str().as_ptr()) }
+			}
+			MappedFileMutRef::InImage { .. } => {
+				// we need to find the parent entry
+				// TODO: reject if the parent entry points to a different entry / image
+				let mut guest_pathbuf = clean(OsStr::from_bytes(guest_path.to_bytes()));
+				(|| {
+					let Some(file_name) = guest_pathbuf.file_name() else {
+						return -ENOENT;
+					};
+					let Ok(file_name) = str::from_utf8(file_name.as_bytes()) else {
+						return -ENOENT;
+					};
+					let file_name = file_name.to_string();
+					if !guest_pathbuf.pop() {
+						return -ENOENT;
+					}
+					let guest_path_c_string =
+						CString::new(guest_pathbuf.as_os_str().as_bytes()).unwrap();
+					if let Some(MappedFileMutRef::InImage { image: _, thin }) =
+						file_map.get_host_path(&guest_path_c_string)
+					{
+						// TODO: reject unlinking of directories
+						thin.unlink(&[&*file_name]);
+						0
+					} else {
+						// TODO: unmount entire image if we unlink the root?
+						error!(
+							"The kernel requested to unlink() an unknown path ({guest_path:?}): Rejecting..."
+						);
+						-ENOENT
+					}
+				})()
+			}
+		}
 	} else {
 		error!("The kernel requested to unlink() an unknown path ({guest_path:?}): Rejecting...");
 		-ENOENT
@@ -109,16 +148,63 @@ pub fn open(mem: &MmapMemory, sysopen: &mut OpenParams, file_map: &mut UhyveFile
 		return;
 	}
 
-	if let Some(host_path) = file_map.get_host_path(guest_path) {
+	if let Some(guest_entry) = file_map.get_host_path(guest_path) {
 		debug!("{guest_path:#?} found in file map.");
-		// We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes
-		// 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();
+		match guest_entry {
+			MappedFileMutRef::OnHost(host_path) => {
+				// We can safely unwrap here, as host_path.as_bytes will never contain internal \0 bytes
+				// 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_os_str().as_bytes()).unwrap();
+
+				sysopen.ret = unsafe {
+					libc::open(host_path_c_string.as_c_str().as_ptr(), flags, sysopen.mode)
+				};
 
-		sysopen.ret =
-			unsafe { libc::open(host_path_c_string.as_c_str().as_ptr(), flags, sysopen.mode) };
+				file_map.fdmap.insert_fd(sysopen.ret);
+			}
+			MappedFileMutRef::InImage {
+				image,
+				thin: HermitImageThinFile::File(r),
+			} => {
+				// For simplicity, create a temporary files with these contents.
+				debug!("Attempting to open a temp file for unpacked {guest_path:#?}...");
+				// Existing files that already exist should be in the file map, not here.
+				// If a supposed attacker can predict where we open a file and its filename,
+				// this contigency, together with O_CREAT, will cause the write to fail.
+				flags |= O_EXCL;
+				let image = Arc::clone(image);
+				let r = r.clone();
 
-		file_map.fdmap.insert_fd(sysopen.ret);
+				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 {
+					use std::io::Write;
+					let raw_fd = sysopen.ret.into_raw_fd();
+					let mut fh = unsafe { File::from_raw_fd(raw_fd) };
+					// SAFETY (slice access): we assume that the image parsing was correct
+					// and that the temporary file wasn't modified after creation.
+					match fh.write_all(&image[r.clone()]) {
+						Ok(_) => {
+							// don't destroy the file descriptor
+							core::mem::forget(fh);
+							file_map.fdmap.insert_fd(sysopen.ret.into_raw_fd());
+						}
+
+						Err(_) => {
+							// TODO: what's the correct error code for this?
+							sysopen.ret = -ENOENT;
+
+							unsafe { libc::unlink(new_host_path) };
+						}
+					}
+				}
+			}
+			_ => {
+				debug!("Returning -ENOENT for {guest_path:#?}");
+				sysopen.ret = -ENOENT;
+			}
+		};
 	} else {
 		debug!("{guest_path:#?} not found in file map.");
 		if (flags & O_CREAT) == O_CREAT {