Skip to content

Commit 8ed3bd3

Browse files
committed
[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 <[email protected]>
1 parent f93f258 commit 8ed3bd3

File tree

4 files changed

+49
-41
lines changed

4 files changed

+49
-41
lines changed

src/devices/src/virtio/fs/device.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl Fs {
9595
device_state: DeviceState::Inactive,
9696
config,
9797
shm_region: None,
98-
server: Server::new(PassthroughFs::new(fs_cfg).unwrap()),
98+
server: Server::new(PassthroughFs::new(fs_cfg).map_err(FsError::FailedToMount)?),
9999
intc: None,
100100
irq_line: None,
101101
})

src/devices/src/virtio/fs/linux/passthrough.rs

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,47 @@ impl Default for Config {
266266
}
267267
}
268268

269+
fn insert_root_dir(
270+
root_dir: &str,
271+
inodes: &mut MultikeyBTreeMap<Inode, InodeAltKey, Arc<InodeData>>,
272+
) -> io::Result<()> {
273+
let root = CString::new(root_dir).expect("CString::new failed");
274+
275+
// Safe because this doesn't modify any memory and we check the return value.
276+
// We use `O_PATH` because we just want this for traversing the directory tree
277+
// and not for actually reading the contents.
278+
let fd = unsafe {
279+
libc::openat(
280+
libc::AT_FDCWD,
281+
root.as_ptr(),
282+
libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC,
283+
)
284+
};
285+
if fd < 0 {
286+
return Err(io::Error::last_os_error());
287+
}
288+
289+
// Safe because we just opened this fd above.
290+
let f = unsafe { File::from_raw_fd(fd) };
291+
292+
let st = stat(&f)?;
293+
294+
// Not sure why the root inode gets a refcount of 2 but that's what libfuse does.
295+
inodes.insert(
296+
fuse::ROOT_ID,
297+
InodeAltKey {
298+
ino: st.st_ino,
299+
dev: st.st_dev,
300+
},
301+
Arc::new(InodeData {
302+
inode: fuse::ROOT_ID,
303+
file: f,
304+
refcount: AtomicU64::new(2),
305+
}),
306+
);
307+
Ok(())
308+
}
309+
269310
/// A file system that simply "passes through" all requests it receives to the underlying file
270311
/// system. To keep the implementation simple it servers the contents of its root directory. Users
271312
/// that wish to serve only a specific directory should set up the environment so that that
@@ -324,9 +365,11 @@ impl PassthroughFs {
324365

325366
// Safe because we just opened this fd or it was provided by our caller.
326367
let proc_self_fd = unsafe { File::from_raw_fd(fd) };
368+
let mut inodes = MultikeyBTreeMap::new();
369+
insert_root_dir(&cfg.root_dir, &mut inodes)?;
327370

328371
Ok(PassthroughFs {
329-
inodes: RwLock::new(MultikeyBTreeMap::new()),
372+
inodes: RwLock::new(inodes),
330373
next_inode: AtomicU64::new(fuse::ROOT_ID + 2),
331374
init_inode: fuse::ROOT_ID + 1,
332375

@@ -683,48 +726,11 @@ impl FileSystem for PassthroughFs {
683726
type Handle = Handle;
684727

685728
fn init(&self, capable: FsOptions) -> io::Result<FsOptions> {
686-
let root = CString::new(self.cfg.root_dir.as_str()).expect("CString::new failed");
687-
688-
// Safe because this doesn't modify any memory and we check the return value.
689-
// We use `O_PATH` because we just want this for traversing the directory tree
690-
// and not for actually reading the contents.
691-
let fd = unsafe {
692-
libc::openat(
693-
libc::AT_FDCWD,
694-
root.as_ptr(),
695-
libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC,
696-
)
697-
};
698-
if fd < 0 {
699-
return Err(io::Error::last_os_error());
700-
}
701-
702-
// Safe because we just opened this fd above.
703-
let f = unsafe { File::from_raw_fd(fd) };
704-
705-
let st = stat(&f)?;
706-
707729
// Safe because this doesn't modify any memory and there is no need to check the return
708730
// value because this system call always succeeds. We need to clear the umask here because
709731
// we want the client to be able to set all the bits in the mode.
710732
unsafe { libc::umask(0o000) };
711733

712-
let mut inodes = self.inodes.write().unwrap();
713-
714-
// Not sure why the root inode gets a refcount of 2 but that's what libfuse does.
715-
inodes.insert(
716-
fuse::ROOT_ID,
717-
InodeAltKey {
718-
ino: st.st_ino,
719-
dev: st.st_dev,
720-
},
721-
Arc::new(InodeData {
722-
inode: fuse::ROOT_ID,
723-
file: f,
724-
refcount: AtomicU64::new(2),
725-
}),
726-
);
727-
728734
let mut opts = FsOptions::DO_READDIRPLUS | FsOptions::READDIRPLUS_AUTO;
729735
if self.cfg.writeback && capable.contains(FsOptions::WRITEBACK_CACHE) {
730736
opts |= FsOptions::WRITEBACK_CACHE;

src/devices/src/virtio/fs/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ pub enum FsError {
5959
InvalidXattrSize((u32, usize)),
6060
QueueReader(DescriptorError),
6161
QueueWriter(DescriptorError),
62+
/// Could not open the root directory or mapped volumes (they do not exist, permission error...)
63+
FailedToMount(io::Error),
6264
}
6365

6466
type Result<T> = std::result::Result<T, FsError>;

src/libkrun/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,8 +770,8 @@ pub extern "C" fn krun_start_enter(ctx_id: u32) -> i32 {
770770

771771
#[cfg(not(feature = "tee"))]
772772
if let Some(fs_cfg) = ctx_cfg.get_fs_cfg() {
773-
if ctx_cfg.vmr.set_fs_device(fs_cfg).is_err() {
774-
error!("Error configuring virtio-fs");
773+
if let Err(e) = ctx_cfg.vmr.set_fs_device(fs_cfg) {
774+
error!("Error configuring virtio-fs {e:?}");
775775
return -libc::EINVAL;
776776
}
777777
}

0 commit comments

Comments
 (0)