Skip to content

Commit

Permalink
passthrough: add ability to disallow operations that could change fil…
Browse files Browse the repository at this point in the history
…e size

Introduce 'seal_size' flag prohibits operations which will change file
size or allocate file block exceed file size.

This assumes the fuse server side is trustable, and client side is not
trusted and not allowed to change file size. For example, in virtiofs
case, we trust the host side and don't trust guest side.

Signed-off-by: shayan.sy <[email protected]>
Signed-off-by: Eryu Guan <[email protected]>
  • Loading branch information
shayan.sy authored and eryugey committed Aug 22, 2022
1 parent 76dbae8 commit 564dfd2
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 2 deletions.
6 changes: 6 additions & 0 deletions src/api/vfs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,9 @@ pub struct VfsOptions {
/// to remove security.capability xattr and setuid/setgid bits. See details in
/// comments for HANDLE_KILLPRIV_V2
pub killpriv_v2: bool,
/// Reject requests which will change the file size, or allocate file
/// blocks exceed file size.
pub seal_size: bool,
/// File system options passed in from client
pub in_opts: FsOptions,
/// File system options returned to client
Expand All @@ -236,6 +239,7 @@ impl Default for VfsOptions {
no_opendir: true,
no_writeback: false,
no_readdir: false,
seal_size: false,
killpriv_v2: false,
in_opts: FsOptions::empty(),
out_opts: FsOptions::ASYNC_READ
Expand Down Expand Up @@ -946,6 +950,7 @@ mod tests {
assert_eq!(opts.no_opendir, true);
assert_eq!(opts.no_writeback, false);
assert_eq!(opts.no_readdir, false);
assert_eq!(opts.seal_size, false);
assert_eq!(opts.killpriv_v2, false);
assert_eq!(opts.in_opts.is_empty(), true);

Expand All @@ -957,6 +962,7 @@ mod tests {
assert_eq!(opts.no_opendir, false);
assert_eq!(opts.no_writeback, false);
assert_eq!(opts.no_readdir, false);
assert_eq!(opts.seal_size, false);
assert_eq!(opts.killpriv_v2, false);

vfs.destroy();
Expand Down
26 changes: 25 additions & 1 deletion src/passthrough/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use async_trait::async_trait;

use super::*;
use crate::abi::fuse_abi::{
CreateIn, OpenOptions, SetattrValid, FOPEN_IN_KILL_SUIDGID, WRITE_KILL_PRIV,
CreateIn, Opcode, OpenOptions, SetattrValid, FOPEN_IN_KILL_SUIDGID, WRITE_KILL_PRIV,
};
use crate::api::filesystem::{
AsyncFileSystem, AsyncZeroCopyReader, AsyncZeroCopyWriter, Context, FileSystem,
Expand Down Expand Up @@ -478,6 +478,10 @@ impl<S: BitmapSlice + Send + Sync> AsyncFileSystem for PassthroughFs<S> {
}
};
if valid.contains(SetattrValid::SIZE) && self.seal_size.load(Ordering::Relaxed) {
return Err(io::Error::from_raw_os_error(libc::EPERM));
}
if valid.contains(SetattrValid::MODE) {
// Safe because this doesn't modify any memory and we check the return value.
let res = unsafe {
Expand Down Expand Up @@ -725,6 +729,13 @@ impl<S: BitmapSlice + Send + Sync> AsyncFileSystem for PassthroughFs<S> {
.async_get_data(ctx, handle, inode, libc::O_RDWR)
.await?;
if self.seal_size.load(Ordering::Relaxed) {
let st = self
.async_stat_fd(cxt, data.get_handle_raw_fd(), None)
.await?;
self.seal_size_check(Opcode::Write, st.st_size as u64, offset, size as u64, 0)?;
}
// Fallback to sync io if KILLPRIV_V2 is enabled to work around a limitation of io_uring.
if self.killpriv_v2.load(Ordering::Relaxed) && (fuse_flags & WRITE_KILL_PRIV != 0) {
// Manually implement File::try_clone() by borrowing fd of data.file instead of dup().
Expand Down Expand Up @@ -786,6 +797,19 @@ impl<S: BitmapSlice + Send + Sync> AsyncFileSystem for PassthroughFs<S> {
.get_drive::<D>()
.ok_or_else(|| io::Error::from_raw_os_error(libc::EINVAL))?;
if self.seal_size.load(Ordering::Relaxed) {
let st = self
.async_stat_fd(cxt, data.get_handle_raw_fd(), None)
.await?;
self.seal_size_check(
Opcode::Fallocate,
st.st_size as u64,
offset,
length,
mode as i32,
)?;
}
AsyncUtil::fallocate(drive, data.get_handle_raw_fd(), offset, length, mode).await
*/
}
Expand Down
59 changes: 59 additions & 0 deletions src/passthrough/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use std::time::Duration;
use vm_memory::ByteValued;

use crate::abi::fuse_abi as fuse;
use crate::abi::fuse_abi::Opcode;
use crate::api::filesystem::Entry;
use crate::api::{
validate_path_component, BackendFileSystem, CURRENT_DIR_CSTR, EMPTY_CSTR, PARENT_DIR_CSTR,
Expand Down Expand Up @@ -463,6 +464,11 @@ pub struct Config {
/// directory is empty even if it has children.
pub no_readdir: bool,

/// Control whether to refuse operations which modify the size of the file. For a share memory
/// file mounted from host, seal_size can prohibit guest to increase the size of
/// share memory file to attack the host.
pub seal_size: bool,

/// What size file supports dax
/// * If dax_file_size == None, DAX will disable to all files.
/// * If dax_file_size == 0, DAX will enable all files.
Expand All @@ -486,6 +492,7 @@ impl Default for Config {
killpriv_v2: false,
inode_file_handles: false,
no_readdir: false,
seal_size: false,
dax_file_size: None,
}
}
Expand Down Expand Up @@ -536,6 +543,9 @@ pub struct PassthroughFs<S: BitmapSlice + Send + Sync = ()> {
// Whether no_readdir is enabled.
no_readdir: AtomicBool,

// Whether seal_size is enabled.
seal_size: AtomicBool,

// Whether per-file DAX feature is enabled.
// Init from guest kernel Init cmd of fuse fs.
perfile_dax: AtomicBool,
Expand Down Expand Up @@ -572,6 +582,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
no_opendir: AtomicBool::new(false),
killpriv_v2: AtomicBool::new(false),
no_readdir: AtomicBool::new(cfg.no_readdir),
seal_size: AtomicBool::new(cfg.seal_size),
perfile_dax: AtomicBool::new(false),
cfg,

Expand Down Expand Up @@ -1000,6 +1011,54 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
}
validate_path_component(name)
}

// When seal_size is set, we don't allow operations that could change file size nor allocate
// space beyond EOF
fn seal_size_check(
&self,
opcode: Opcode,
file_size: u64,
offset: u64,
size: u64,
mode: i32,
) -> io::Result<()> {
if offset.checked_add(size).is_none() {
error!(
"fuse: {:?}: invalid `offset` + `size` ({}+{}) overflows u64::MAX",
opcode, offset, size
);
return Err(io::Error::from_raw_os_error(libc::EINVAL));
}

match opcode {
// write should not exceed the file size.
Opcode::Write if size + offset > file_size => {
Err(io::Error::from_raw_os_error(libc::EPERM))
}

// fallocate operation should not allocate blocks exceed the file size.
//
// FALLOC_FL_COLLAPSE_RANGE or FALLOC_FL_INSERT_RANGE mode will change file size which
// is not allowed.
//
// FALLOC_FL_PUNCH_HOLE mode won't change file size, as it must be ORed with
// FALLOC_FL_KEEP_SIZE.
Opcode::Fallocate
if ((mode == 0
|| mode == libc::FALLOC_FL_KEEP_SIZE
|| mode & libc::FALLOC_FL_ZERO_RANGE != 0)
&& size + offset > file_size)
|| (mode & libc::FALLOC_FL_COLLAPSE_RANGE != 0
|| mode & libc::FALLOC_FL_INSERT_RANGE != 0) =>
{
Err(io::Error::from_raw_os_error(libc::EPERM))
}

// setattr operation should be handled in setattr handler, other operations won't
// change file size.
_ => Ok(()),
}
}
}

#[cfg(not(feature = "async-io"))]
Expand Down
23 changes: 22 additions & 1 deletion src/passthrough/sync_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::sync::Arc;
use std::time::Duration;

use super::*;
use crate::abi::fuse_abi::{CreateIn, FOPEN_IN_KILL_SUIDGID, WRITE_KILL_PRIV};
use crate::abi::fuse_abi::{CreateIn, Opcode, FOPEN_IN_KILL_SUIDGID, WRITE_KILL_PRIV};
#[cfg(any(feature = "vhost-user-fs", feature = "virtiofs"))]
use crate::abi::virtio_fs;
use crate::api::filesystem::{
Expand Down Expand Up @@ -689,6 +689,12 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
// It's safe because the `data` variable's lifetime spans the whole function,
// so data.file won't be closed.
let f = unsafe { File::from_raw_fd(data.get_handle_raw_fd()) };

if self.seal_size.load(Ordering::Relaxed) {
let st = Self::stat_fd(f.as_raw_fd(), None)?;
self.seal_size_check(Opcode::Write, st.st_size as u64, offset, size as u64, 0)?;
}

let mut f = ManuallyDrop::new(f);

// Cap restored when _killpriv is dropped
Expand Down Expand Up @@ -744,6 +750,10 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
}
};

if valid.contains(SetattrValid::SIZE) && self.seal_size.load(Ordering::Relaxed) {
return Err(io::Error::from_raw_os_error(libc::EPERM));
}

if valid.contains(SetattrValid::MODE) {
// Safe because this doesn't modify any memory and we check the return value.
let res = unsafe {
Expand Down Expand Up @@ -1264,6 +1274,17 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
let data = self.get_data(handle, inode, libc::O_RDWR)?;
let fd = data.get_handle_raw_fd();

if self.seal_size.load(Ordering::Relaxed) {
let st = Self::stat_fd(fd, None)?;
self.seal_size_check(
Opcode::Fallocate,
st.st_size as u64,
offset,
length,
mode as i32,
)?;
}

// Safe because this doesn't modify any memory and we check the return value.
let res = unsafe {
libc::fallocate64(
Expand Down

0 comments on commit 564dfd2

Please sign in to comment.