From 9dfb790d371eda788d5bfbfc2631833bbf40ad1a Mon Sep 17 00:00:00 2001 From: Matej Hrica Date: Tue, 24 Oct 2023 14:08:10 +0200 Subject: [PATCH] [Linux] Make krun_start_enter error when root directory does not exist Previously krun_start_enter would succeed and the guest kernel would just panic. The root filesystem directory was opened lazily when the guest kernel used fuse init opcode. This commit changes it so the root directory is opened when creating the fs device. This only fixes it on Linux, but not in the macOS implementation. Signed-off-by: Matej Hrica --- src/devices/src/virtio/fs/device.rs | 2 +- .../src/virtio/fs/linux/passthrough.rs | 80 ++++++++++--------- src/devices/src/virtio/fs/mod.rs | 2 + src/libkrun/src/lib.rs | 5 +- 4 files changed, 48 insertions(+), 41 deletions(-) diff --git a/src/devices/src/virtio/fs/device.rs b/src/devices/src/virtio/fs/device.rs index b1e62ff2..4f548a9a 100644 --- a/src/devices/src/virtio/fs/device.rs +++ b/src/devices/src/virtio/fs/device.rs @@ -95,7 +95,7 @@ impl Fs { device_state: DeviceState::Inactive, config, shm_region: None, - server: Server::new(PassthroughFs::new(fs_cfg).unwrap()), + server: Server::new(PassthroughFs::new(fs_cfg).map_err(FsError::RootDirNotAccessible)?), intc: None, irq_line: None, }) diff --git a/src/devices/src/virtio/fs/linux/passthrough.rs b/src/devices/src/virtio/fs/linux/passthrough.rs index f9711f90..24bf7a14 100644 --- a/src/devices/src/virtio/fs/linux/passthrough.rs +++ b/src/devices/src/virtio/fs/linux/passthrough.rs @@ -301,6 +301,47 @@ pub struct PassthroughFs { impl PassthroughFs { pub fn new(cfg: Config) -> io::Result { + // Initialze inodes with root directory already open + let inodes = { + let mut inodes = MultikeyBTreeMap::new(); + + let root = CString::new(cfg.root_dir.as_str()).expect("CString::new failed"); + + // Safe because this doesn't modify any memory and we check the return value. + // We use `O_PATH` because we just want this for traversing the directory tree + // and not for actually reading the contents. + let fd = unsafe { + libc::openat( + libc::AT_FDCWD, + root.as_ptr(), + libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC, + ) + }; + if fd < 0 { + return Err(io::Error::last_os_error()); + } + + // Safe because we just opened this fd above. + let f = unsafe { File::from_raw_fd(fd) }; + + let st = stat(&f)?; + + // Not sure why the root inode gets a refcount of 2 but that's what libfuse does. + inodes.insert( + fuse::ROOT_ID, + InodeAltKey { + ino: st.st_ino, + dev: st.st_dev, + }, + Arc::new(InodeData { + inode: fuse::ROOT_ID, + file: f, + refcount: AtomicU64::new(2), + }), + ); + inodes + }; + let fd = if let Some(fd) = cfg.proc_sfd_rawfd { fd } else { @@ -326,7 +367,7 @@ impl PassthroughFs { let proc_self_fd = unsafe { File::from_raw_fd(fd) }; Ok(PassthroughFs { - inodes: RwLock::new(MultikeyBTreeMap::new()), + inodes: RwLock::new(inodes), next_inode: AtomicU64::new(fuse::ROOT_ID + 2), init_inode: fuse::ROOT_ID + 1, @@ -683,48 +724,11 @@ impl FileSystem for PassthroughFs { type Handle = Handle; fn init(&self, capable: FsOptions) -> io::Result { - let root = CString::new(self.cfg.root_dir.as_str()).expect("CString::new failed"); - - // Safe because this doesn't modify any memory and we check the return value. - // We use `O_PATH` because we just want this for traversing the directory tree - // and not for actually reading the contents. - let fd = unsafe { - libc::openat( - libc::AT_FDCWD, - root.as_ptr(), - libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC, - ) - }; - if fd < 0 { - return Err(io::Error::last_os_error()); - } - - // Safe because we just opened this fd above. - let f = unsafe { File::from_raw_fd(fd) }; - - let st = stat(&f)?; - // Safe because this doesn't modify any memory and there is no need to check the return // value because this system call always succeeds. We need to clear the umask here because // we want the client to be able to set all the bits in the mode. unsafe { libc::umask(0o000) }; - let mut inodes = self.inodes.write().unwrap(); - - // Not sure why the root inode gets a refcount of 2 but that's what libfuse does. - inodes.insert( - fuse::ROOT_ID, - InodeAltKey { - ino: st.st_ino, - dev: st.st_dev, - }, - Arc::new(InodeData { - inode: fuse::ROOT_ID, - file: f, - refcount: AtomicU64::new(2), - }), - ); - let mut opts = FsOptions::DO_READDIRPLUS | FsOptions::READDIRPLUS_AUTO; if self.cfg.writeback && capable.contains(FsOptions::WRITEBACK_CACHE) { opts |= FsOptions::WRITEBACK_CACHE; diff --git a/src/devices/src/virtio/fs/mod.rs b/src/devices/src/virtio/fs/mod.rs index 7e307303..e6f1d58e 100644 --- a/src/devices/src/virtio/fs/mod.rs +++ b/src/devices/src/virtio/fs/mod.rs @@ -59,6 +59,8 @@ pub enum FsError { InvalidXattrSize((u32, usize)), QueueReader(DescriptorError), QueueWriter(DescriptorError), + /// The root filesystem directory is not accessible (it does not exist, permission error...) + RootDirNotAccessible(io::Error) } type Result = std::result::Result; diff --git a/src/libkrun/src/lib.rs b/src/libkrun/src/lib.rs index cd814588..ebf1a0c0 100644 --- a/src/libkrun/src/lib.rs +++ b/src/libkrun/src/lib.rs @@ -763,6 +763,7 @@ pub extern "C" fn krun_start_enter(ctx_id: u32) -> i32 { } }; + let mut ctx_cfg = match CTX_MAP.lock().unwrap().remove(&ctx_id) { Some(ctx_cfg) => ctx_cfg, None => return -libc::ENOENT, @@ -770,8 +771,8 @@ pub extern "C" fn krun_start_enter(ctx_id: u32) -> i32 { #[cfg(not(feature = "tee"))] if let Some(fs_cfg) = ctx_cfg.get_fs_cfg() { - if ctx_cfg.vmr.set_fs_device(fs_cfg).is_err() { - error!("Error configuring virtio-fs"); + if let Err(e) = ctx_cfg.vmr.set_fs_device(fs_cfg) { + error!("Error configuring virtio-fs {e:?}"); return -libc::EINVAL; } }