From 5b6d99b75966fdd8516af839ff029f175d74c0ae Mon Sep 17 00:00:00 2001 From: Manu Bretelle Date: Wed, 1 Nov 2023 14:13:44 -0700 Subject: [PATCH] qemu: Validate that guest_init_path is a suffix of dirname(host_init_path) If this is not the case, the guest is not going to be able to find the init file we pass as a parameter of `init=`. We better error out than having a VM that fails to boot properly. Signed-off-by: Manu Bretelle --- src/qemu.rs | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/qemu.rs b/src/qemu.rs index acd027a..aa0d404 100644 --- a/src/qemu.rs +++ b/src/qemu.rs @@ -79,10 +79,29 @@ fn gen_sock(prefix: &str) -> PathBuf { // Given a guest temp dir and a host init path, generate the path to the init file // in the guest. // This path is the one that will be passed to the guest via the kernel's `init=` parameter. -fn guest_init_path(guest_temp_dir: PathBuf, host_init_path: PathBuf) -> PathBuf { +// Returns an error if the guest_temp_dir is not a suffix of host_init_path.parent(). +fn guest_init_path(guest_temp_dir: PathBuf, host_init_path: PathBuf) -> Result { + // guest_temp_dir should be the suffix of host_init_path.parent() + if !host_init_path + .parent() + .context(format!( + "host_init_path {:?} should have a parent", + host_init_path + ))? + .ends_with(guest_temp_dir.strip_prefix("/").context(format!( + "guest_temp_dir {:?} should be an absolute path", + guest_temp_dir + ))?) + { + bail!( + "guest_temp_dir {:?} should be a suffix of host_init_path.parent() {:?}", + guest_temp_dir, + host_init_path.parent() + ); + } let mut guest_init_path = guest_temp_dir; guest_init_path.push(host_init_path.file_name().unwrap()); - guest_init_path + Ok(guest_init_path) } // Given a rootfs, generate a tempfile with the init script inside. @@ -120,7 +139,7 @@ fn gen_init(rootfs: &Path) -> Result<(NamedTempFile, PathBuf)> { // Path in the guest is our guest_temp_dir to which we append the file // name of the host init script. - let guest_init = guest_init_path(guest_temp_dir, host_init.path().to_path_buf()); + let guest_init = guest_init_path(guest_temp_dir, host_init.path().to_path_buf())?; debug!( "rootfs path: {rootfs:?}, init host path: {host_init:?}, init guest path: {guest_init:?}" ); @@ -923,19 +942,24 @@ mod tests { #[case("/tmp/", "/foo/tmp/bar.sh", "/tmp/bar.sh")] // A valid case if rootfs was /foo/tmp and env::temp_dir() was /. #[case("/", "/foo/tmp/bar.sh", "/bar.sh")] - // This should never happen given that host_init_path is made by appending guest_temp_dir to rootfs. - // for now it will return a guest_init_path which won't work. - #[case("/tmp", "/foo/bar.sh", "/tmp/bar.sh")] // A valid case if env::temp_dir() was /foo/tmp and rootfs was /. #[case("/foo/tmp", "/foo/tmp/bar.sh", "/foo/tmp/bar.sh")] - // Invalid case because guest_temp_dir is not a suffix of dirname(host_init_path) - #[case("/foo/tmp", "/bar/tmp/bar.sh", "/foo/tmp/bar.sh")] fn test_guest_init_path( #[case] guest_temp_dir: &str, #[case] host_init_path: &str, #[case] expected: &str, ) { - let r = guest_init_path(guest_temp_dir.into(), host_init_path.into()); + let r = guest_init_path(guest_temp_dir.into(), host_init_path.into()).unwrap(); assert_eq!(r, PathBuf::from(expected)); } + + #[rstest] + // This should never happen given that host_init_path is made by appending guest_temp_dir to rootfs. + // for now it will return a guest_init_path which won't work. + #[case("/tmp", "/foo/bar.sh")] + // Invalid case because guest_temp_dir is not a suffix of dirname(host_init_path) + #[case("/foo/tmp", "/bar/tmp/bar.sh")] + fn test_invalid_guest_init_path(#[case] guest_temp_dir: &str, #[case] host_init_path: &str) { + guest_init_path(guest_temp_dir.into(), host_init_path.into()).unwrap_err(); + } }