passthrough: add ability to disallow operations that could change file size

shayan.sy · jiangliu · commit 240dbd9ec13e · 2022-09-09T14:33:00.000+08:00
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 &lt;shayan.sy@alibaba-inc.com&gt;
Signed-off-by: Eryu Guan &lt;eguan@linux.alibaba.com&gt;
diff --git a/src/api/vfs/mod.rs b/src/api/vfs/mod.rs
@@ -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
@@ -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
@@ -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);
 
@@ -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();
diff --git a/src/passthrough/async_io.rs b/src/passthrough/async_io.rs
@@ -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,
@@ -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 {
@@ -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().
@@ -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
          */
     }
diff --git a/src/passthrough/mod.rs b/src/passthrough/mod.rs
@@ -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,
@@ -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.
@@ -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,
         }
     }
@@ -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,
@@ -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,
 
@@ -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"))]
diff --git a/src/passthrough/sync_io.rs b/src/passthrough/sync_io.rs
@@ -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::{
@@ -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
@@ -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 {
@@ -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(
