Skip to content

Commit

Permalink
[Linux] Make krun_start_enter error when root directory does not exist
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
mtjhrc committed Oct 25, 2023
1 parent f93f258 commit 9dfb790
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/devices/src/virtio/fs/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down
80 changes: 42 additions & 38 deletions src/devices/src/virtio/fs/linux/passthrough.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,47 @@ pub struct PassthroughFs {

impl PassthroughFs {
pub fn new(cfg: Config) -> io::Result<PassthroughFs> {
// 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 {
Expand All @@ -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,

Expand Down Expand Up @@ -683,48 +724,11 @@ impl FileSystem for PassthroughFs {
type Handle = Handle;

fn init(&self, capable: FsOptions) -> io::Result<FsOptions> {
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;
Expand Down
2 changes: 2 additions & 0 deletions src/devices/src/virtio/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = std::result::Result<T, FsError>;
5 changes: 3 additions & 2 deletions src/libkrun/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -763,15 +763,16 @@ 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,
};

#[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;
}
}
Expand Down

0 comments on commit 9dfb790

Please sign in to comment.