summary refs log tree commit diff
diff options
context:
space:
mode:
authorChirantan Ekbote <chirantan@chromium.org>2019-12-05 19:20:48 +0900
committerCommit Bot <commit-bot@chromium.org>2019-12-10 03:10:57 +0000
commit4f9f5c74791fd13f6930a48232b3327066259b8e (patch)
tree0bbbad8c69a96209fee1b75e6d8f1d6352b6ee16
parentf21572c7187c8beb9c6bfea6446351ae93200d01 (diff)
downloadcrosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.tar
crosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.tar.gz
crosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.tar.bz2
crosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.tar.lz
crosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.tar.xz
crosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.tar.zst
crosvm-4f9f5c74791fd13f6930a48232b3327066259b8e.zip
devices: fs: Support fs crypto ioctls
Add support for FS_IOC_{GET,SET}_ENCRYPTION_POLICY.  Unfortunately,
since the I/O direction is encoded backwards in the ioctl definitions,
these will only work with on a kernel that's compiled with a patch to
mark them as unrestricted FUSE ioctls.

BUG=b:136127632
TEST=Compile and run the vfs_crypto.c program on a virtio-fs mount
     inside a VM

Change-Id: I124c5a943111b453dd44921a079a2baa1036dfd4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1952570
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
-rw-r--r--devices/src/virtio/fs/filesystem.rs64
-rw-r--r--devices/src/virtio/fs/fuse.rs19
-rw-r--r--devices/src/virtio/fs/mod.rs7
-rw-r--r--devices/src/virtio/fs/passthrough.rs108
-rw-r--r--devices/src/virtio/fs/server.rs99
-rw-r--r--seccomp/arm/fs_device.policy1
-rw-r--r--seccomp/x86_64/fs_device.policy1
-rw-r--r--tests/vfs_crypto.c58
8 files changed, 330 insertions, 27 deletions
diff --git a/devices/src/virtio/fs/filesystem.rs b/devices/src/virtio/fs/filesystem.rs
index 53a3f1e..232ff99 100644
--- a/devices/src/virtio/fs/filesystem.rs
+++ b/devices/src/virtio/fs/filesystem.rs
@@ -13,10 +13,7 @@ use libc;
 
 use crate::virtio::fs::fuse;
 
-pub use fuse::FsOptions;
-pub use fuse::OpenOptions;
-pub use fuse::SetattrValid;
-pub use fuse::ROOT_ID;
+pub use fuse::{FsOptions, IoctlFlags, IoctlIovec, OpenOptions, SetattrValid, ROOT_ID};
 
 /// Information about a path in the filesystem.
 pub struct Entry {
@@ -111,6 +108,26 @@ pub enum ListxattrReply {
     Count(u32),
 }
 
+/// A reply to an `ioctl` method call.
+pub enum IoctlReply {
+    /// Indicates that the ioctl should be retried. This is only a valid reply when the `flags`
+    /// field of the ioctl request contains `IoctlFlags::UNRESTRICTED`. The kernel will read in data
+    /// and prepare output buffers as specified in the `input` and `output` fields before re-sending
+    /// the ioctl message.
+    Retry {
+        /// Data that should be read by the kernel module and sent to the server when the ioctl is
+        /// retried.
+        input: Vec<IoctlIovec>,
+
+        /// Buffer space that should be prepared so that the server can send back the response to
+        /// the ioctl.
+        output: Vec<IoctlIovec>,
+    },
+
+    /// Indicates that the ioctl was processed.
+    Done(io::Result<Vec<u8>>),
+}
+
 /// A trait for directly copying data from the fuse transport into a `File` without first storing it
 /// in an intermediate buffer.
 pub trait ZeroCopyReader {
@@ -1054,23 +1071,52 @@ pub trait FileSystem {
         Err(io::Error::from_raw_os_error(libc::ENOSYS))
     }
 
-    /// TODO: support this
-    fn getlk(&self) -> io::Result<()> {
+    /// Perform an ioctl on a file or directory.
+    ///
+    /// `handle` is the `Handle` returned by the file system from the `open` or `opendir` methods,
+    /// if any. If the file system did not return a `Handle` from then the contents of `handle` are
+    /// undefined.
+    ///
+    /// If `flags` contains `IoctlFlags::UNRESTRICTED` then the file system may retry the ioctl
+    /// after informing the kernel about the input and output areas. If `flags` does not contain
+    /// `IoctlFlags::UNRESTRICTED` then the kernel will prepare the input and output areas according
+    /// to the encoding in the ioctl command. In that case the ioctl cannot be retried.
+    ///
+    /// `cmd` is the ioctl request made by the calling process, truncated to 32 bits.
+    ///
+    /// `arg` is the argument provided by the calling process.
+    ///
+    /// `in_size` is the length of the additional data that accompanies the request. The file system
+    /// may fetch this data from `reader`.
+    ///
+    /// `out_size` is the length of the output area prepared by the kernel to hold the response to
+    /// the ioctl.
+    fn ioctl<R: io::Read>(
+        &self,
+        ctx: Context,
+        handle: Self::Handle,
+        flags: IoctlFlags,
+        cmd: u32,
+        arg: u64,
+        in_size: u32,
+        out_size: u32,
+        reader: R,
+    ) -> io::Result<IoctlReply> {
         Err(io::Error::from_raw_os_error(libc::ENOSYS))
     }
 
     /// TODO: support this
-    fn setlk(&self) -> io::Result<()> {
+    fn getlk(&self) -> io::Result<()> {
         Err(io::Error::from_raw_os_error(libc::ENOSYS))
     }
 
     /// TODO: support this
-    fn setlkw(&self) -> io::Result<()> {
+    fn setlk(&self) -> io::Result<()> {
         Err(io::Error::from_raw_os_error(libc::ENOSYS))
     }
 
     /// TODO: support this
-    fn ioctl(&self) -> io::Result<()> {
+    fn setlkw(&self) -> io::Result<()> {
         Err(io::Error::from_raw_os_error(libc::ENOSYS))
     }
 
diff --git a/devices/src/virtio/fs/fuse.rs b/devices/src/virtio/fs/fuse.rs
index f9f97d8..612202b 100644
--- a/devices/src/virtio/fs/fuse.rs
+++ b/devices/src/virtio/fs/fuse.rs
@@ -339,27 +339,24 @@ const IOCTL_32BIT: u32 = 8;
 const IOCTL_DIR: u32 = 16;
 
 /// Maximum of in_iovecs + out_iovecs
-const IOCTL_MAX_IOV: u32 = 256;
+pub const IOCTL_MAX_IOV: usize = 256;
 
 bitflags! {
     pub struct IoctlFlags: u32 {
         /// 32bit compat ioctl on 64bit machine
-        const IOCTL_COMPAT = IOCTL_COMPAT;
+        const COMPAT = IOCTL_COMPAT;
 
         /// Not restricted to well-formed ioctls, retry allowed
-        const IOCTL_UNRESTRICTED = IOCTL_UNRESTRICTED;
+        const UNRESTRICTED = IOCTL_UNRESTRICTED;
 
         /// Retry with new iovecs
-        const IOCTL_RETRY = IOCTL_RETRY;
+        const RETRY = IOCTL_RETRY;
 
         /// 32bit ioctl
         const IOCTL_32BIT = IOCTL_32BIT;
 
         /// Is a directory
-        const IOCTL_DIR = IOCTL_DIR;
-
-        /// Maximum of in_iovecs + out_iovecs
-        const IOCTL_MAX_IOV = IOCTL_MAX_IOV;
+        const DIR = IOCTL_DIR;
     }
 }
 
@@ -872,10 +869,16 @@ pub struct IoctlIn {
 }
 unsafe impl DataInit for IoctlIn {}
 
+/// Describes a region of memory in the address space of the process that made the ioctl syscall.
+/// Similar to `libc::iovec` but uses `u64`s for the address and the length.
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct IoctlIovec {
+    /// The start address of the memory region. This must be in the address space of the process
+    /// that made the ioctl syscall.
     pub base: u64,
+
+    /// The length of the memory region.
     pub len: u64,
 }
 unsafe impl DataInit for IoctlIovec {}
diff --git a/devices/src/virtio/fs/mod.rs b/devices/src/virtio/fs/mod.rs
index a1f8868..9cac98b 100644
--- a/devices/src/virtio/fs/mod.rs
+++ b/devices/src/virtio/fs/mod.rs
@@ -84,6 +84,8 @@ pub enum Error {
     /// The `size` field of the `SetxattrIn` message does not match the length
     /// of the decoded value.
     InvalidXattrSize((u32, usize)),
+    /// Requested too many `iovec`s for an `ioctl` retry.
+    TooManyIovecs((usize, usize)),
 }
 
 impl ::std::error::Error for Error {}
@@ -116,6 +118,11 @@ impl fmt::Display for Error {
                  decoded value: size = {}, value.len() = {}",
                 size, len
             ),
+            TooManyIovecs((count, max)) => write!(
+                f,
+                "requested too many `iovec`s for an `ioctl` retry reply: requested {}, max: {}",
+                count, max
+            ),
         }
     }
 }
diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs
index 7f6e3bb..bdc1c6f 100644
--- a/devices/src/virtio/fs/passthrough.rs
+++ b/devices/src/virtio/fs/passthrough.rs
@@ -17,11 +17,11 @@ use std::time::Duration;
 use data_model::DataInit;
 use libc;
 use sync::Mutex;
-use sys_util::error;
+use sys_util::{error, ioctl_ior_nr, ioctl_iow_nr, ioctl_with_mut_ptr, ioctl_with_ptr};
 
 use crate::virtio::fs::filesystem::{
-    Context, DirEntry, Entry, FileSystem, FsOptions, GetxattrReply, ListxattrReply, OpenOptions,
-    SetattrValid, ZeroCopyReader, ZeroCopyWriter,
+    Context, DirEntry, Entry, FileSystem, FsOptions, GetxattrReply, IoctlFlags, IoctlIovec,
+    IoctlReply, ListxattrReply, OpenOptions, SetattrValid, ZeroCopyReader, ZeroCopyWriter,
 };
 use crate::virtio::fs::fuse;
 use crate::virtio::fs::multikey::MultikeyBTreeMap;
@@ -32,6 +32,22 @@ const EMPTY_CSTR: &[u8] = b"\0";
 const ROOT_CSTR: &[u8] = b"/\0";
 const PROC_CSTR: &[u8] = b"/proc\0";
 
+const FSCRYPT_KEY_DESCRIPTOR_SIZE: usize = 8;
+
+#[repr(C)]
+#[derive(Debug, Clone, Copy)]
+struct fscrypt_policy_v1 {
+    _version: u8,
+    _contents_encryption_mode: u8,
+    _filenames_encryption_mode: u8,
+    _flags: u8,
+    _master_key_descriptor: [u8; FSCRYPT_KEY_DESCRIPTOR_SIZE],
+}
+unsafe impl DataInit for fscrypt_policy_v1 {}
+
+ioctl_ior_nr!(FS_IOC_SET_ENCRYPTION_POLICY, 0x66, 19, fscrypt_policy_v1);
+ioctl_iow_nr!(FS_IOC_GET_ENCRYPTION_POLICY, 0x66, 21, fscrypt_policy_v1);
+
 type Inode = u64;
 type Handle = u64;
 
@@ -629,6 +645,50 @@ impl PassthroughFs {
             Err(io::Error::last_os_error())
         }
     }
+
+    fn get_encryption_policy(&self, handle: Handle) -> io::Result<IoctlReply> {
+        let data = self
+            .handles
+            .read()
+            .unwrap()
+            .get(&handle)
+            .map(Arc::clone)
+            .ok_or_else(ebadf)?;
+
+        let mut buf = MaybeUninit::<fscrypt_policy_v1>::zeroed();
+        let file = data.file.lock();
+
+        // Safe because the kernel will only write to `buf` and we check the return value.
+        let res =
+            unsafe { ioctl_with_mut_ptr(&*file, FS_IOC_GET_ENCRYPTION_POLICY(), buf.as_mut_ptr()) };
+        if res < 0 {
+            Ok(IoctlReply::Done(Err(io::Error::last_os_error())))
+        } else {
+            // Safe because the kernel guarantees that the policy is now initialized.
+            let policy = unsafe { buf.assume_init() };
+            Ok(IoctlReply::Done(Ok(policy.as_slice().to_vec())))
+        }
+    }
+
+    fn set_encryption_policy<R: io::Read>(&self, handle: Handle, r: R) -> io::Result<IoctlReply> {
+        let data = self
+            .handles
+            .read()
+            .unwrap()
+            .get(&handle)
+            .map(Arc::clone)
+            .ok_or_else(ebadf)?;
+
+        let policy = fscrypt_policy_v1::from_reader(r)?;
+        let file = data.file.lock();
+        // Safe because the kernel will only read from `policy` and we check the return value.
+        let res = unsafe { ioctl_with_ptr(&*file, FS_IOC_SET_ENCRYPTION_POLICY(), &policy) };
+        if res < 0 {
+            Ok(IoctlReply::Done(Err(io::Error::last_os_error())))
+        } else {
+            Ok(IoctlReply::Done(Ok(Vec::new())))
+        }
+    }
 }
 
 fn forget_one(
@@ -1587,4 +1647,46 @@ impl FileSystem for PassthroughFs {
             Err(io::Error::last_os_error())
         }
     }
+
+    fn ioctl<R: io::Read>(
+        &self,
+        _ctx: Context,
+        handle: Handle,
+        _flags: IoctlFlags,
+        cmd: u32,
+        arg: u64,
+        in_size: u32,
+        out_size: u32,
+        r: R,
+    ) -> io::Result<IoctlReply> {
+        // Normally, we wouldn't need to retry the FS_IOC_GET_ENCRYPTION_POLICY and
+        // FS_IOC_SET_ENCRYPTION_POLICY ioctls. Unfortunately, the I/O directions for both of them
+        // are encoded backwards so they can only be handled as unrestricted fuse ioctls.
+        if cmd == FS_IOC_GET_ENCRYPTION_POLICY() as u32 {
+            if out_size < size_of::<fscrypt_policy_v1>() as u32 {
+                let input = Vec::new();
+                let output = vec![IoctlIovec {
+                    base: arg,
+                    len: size_of::<fscrypt_policy_v1>() as u64,
+                }];
+                Ok(IoctlReply::Retry { input, output })
+            } else {
+                self.get_encryption_policy(handle)
+            }
+        } else if cmd == FS_IOC_SET_ENCRYPTION_POLICY() as u32 {
+            if in_size < size_of::<fscrypt_policy_v1>() as u32 {
+                let input = vec![IoctlIovec {
+                    base: arg,
+                    len: size_of::<fscrypt_policy_v1>() as u64,
+                }];
+                let output = Vec::new();
+                Ok(IoctlReply::Retry { input, output })
+            } else {
+                self.set_encryption_policy(handle, r)
+            }
+        } else {
+            // Did you know that a file/directory is not a TTY?
+            Err(io::Error::from_raw_os_error(libc::ENOTTY))
+        }
+    }
 }
diff --git a/devices/src/virtio/fs/server.rs b/devices/src/virtio/fs/server.rs
index 9c3136d..32b5008 100644
--- a/devices/src/virtio/fs/server.rs
+++ b/devices/src/virtio/fs/server.rs
@@ -12,8 +12,8 @@ use libc;
 use sys_util::error;
 
 use crate::virtio::fs::filesystem::{
-    Context, DirEntry, Entry, FileSystem, GetxattrReply, ListxattrReply, ZeroCopyReader,
-    ZeroCopyWriter,
+    Context, DirEntry, Entry, FileSystem, GetxattrReply, IoctlReply, ListxattrReply,
+    ZeroCopyReader, ZeroCopyWriter,
 };
 use crate::virtio::fs::fuse::*;
 use crate::virtio::fs::{Error, Result};
@@ -1097,11 +1097,35 @@ impl<F: FileSystem + Sync> Server<F> {
         Ok(0)
     }
 
-    fn ioctl(&self, in_header: InHeader, _r: Reader, w: Writer) -> Result<usize> {
-        if let Err(e) = self.fs.ioctl() {
-            reply_error(e, in_header.unique, w)
-        } else {
-            Ok(0)
+    fn ioctl(&self, in_header: InHeader, mut r: Reader, w: Writer) -> Result<usize> {
+        let IoctlIn {
+            fh,
+            flags,
+            cmd,
+            arg,
+            in_size,
+            out_size,
+        } = r.read_obj().map_err(Error::DecodeMessage)?;
+
+        let res = self.fs.ioctl(
+            in_header.into(),
+            fh.into(),
+            IoctlFlags::from_bits_truncate(flags),
+            cmd,
+            arg,
+            in_size,
+            out_size,
+            r,
+        );
+
+        match res {
+            Ok(reply) => match reply {
+                IoctlReply::Retry { input, output } => {
+                    retry_ioctl(in_header.unique, input, output, w)
+                }
+                IoctlReply::Done(res) => finish_ioctl(in_header.unique, res, w),
+            },
+            Err(e) => reply_error(e, in_header.unique, w),
         }
     }
 
@@ -1186,6 +1210,67 @@ impl<F: FileSystem + Sync> Server<F> {
     }
 }
 
+fn retry_ioctl(
+    unique: u64,
+    input: Vec<IoctlIovec>,
+    output: Vec<IoctlIovec>,
+    mut w: Writer,
+) -> Result<usize> {
+    // We don't need to check for overflow here because if adding these 2 values caused an overflow
+    // we would have run out of memory before reaching this point.
+    if input.len() + output.len() > IOCTL_MAX_IOV {
+        return Err(Error::TooManyIovecs((
+            input.len() + output.len(),
+            IOCTL_MAX_IOV,
+        )));
+    }
+
+    let len = size_of::<OutHeader>()
+        + size_of::<IoctlOut>()
+        + (input.len() * size_of::<IoctlIovec>())
+        + (output.len() * size_of::<IoctlIovec>());
+    let header = OutHeader {
+        len: len as u32,
+        error: 0,
+        unique,
+    };
+    let out = IoctlOut {
+        result: 0,
+        flags: IoctlFlags::RETRY.bits(),
+        in_iovs: input.len() as u32,
+        out_iovs: output.len() as u32,
+    };
+
+    w.write_obj(header).map_err(Error::EncodeMessage)?;
+    w.write_obj(out).map_err(Error::EncodeMessage)?;
+    for i in input.into_iter().chain(output.into_iter()) {
+        w.write_obj(i).map_err(Error::EncodeMessage)?;
+    }
+
+    debug_assert_eq!(len, w.bytes_written());
+    Ok(w.bytes_written())
+}
+
+fn finish_ioctl(unique: u64, res: io::Result<Vec<u8>>, w: Writer) -> Result<usize> {
+    let (out, data) = match res {
+        Ok(data) => {
+            let out = IoctlOut {
+                result: 0,
+                ..Default::default()
+            };
+            (out, Some(data))
+        }
+        Err(e) => {
+            let out = IoctlOut {
+                result: -e.raw_os_error().unwrap_or(libc::EIO),
+                ..Default::default()
+            };
+            (out, None)
+        }
+    };
+    reply_ok(Some(out), data.as_ref().map(|d| &d[..]), unique, w)
+}
+
 fn reply_ok<T: DataInit>(
     out: Option<T>,
     data: Option<&[u8]>,
diff --git a/seccomp/arm/fs_device.policy b/seccomp/arm/fs_device.policy
index 0ea7fe0..0708ec8 100644
--- a/seccomp/arm/fs_device.policy
+++ b/seccomp/arm/fs_device.policy
@@ -16,6 +16,7 @@ ftruncate64: 1
 getdents64: 1
 getegid32: 1
 geteuid32: 1
+ioctl: arg1 == FS_IOC_GET_ENCRYPTION_POLICY || arg1 == FS_IOC_SET_ENCRYPTION_POLICY
 linkat: 1
 _llseek: 1
 mkdirat: 1
diff --git a/seccomp/x86_64/fs_device.policy b/seccomp/x86_64/fs_device.policy
index cbf0288..20db0bf 100644
--- a/seccomp/x86_64/fs_device.policy
+++ b/seccomp/x86_64/fs_device.policy
@@ -15,6 +15,7 @@ ftruncate: 1
 getdents64: 1
 getegid: 1
 geteuid: 1
+ioctl: arg1 == FS_IOC_GET_ENCRYPTION_POLICY || arg1 == FS_IOC_SET_ENCRYPTION_POLICY
 linkat: 1
 lseek: 1
 mkdirat: 1
diff --git a/tests/vfs_crypto.c b/tests/vfs_crypto.c
new file mode 100644
index 0000000..3672b31
--- /dev/null
+++ b/tests/vfs_crypto.c
@@ -0,0 +1,58 @@
+// Copyright 2019 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/fs.h>
+#include <stdio.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+extern char* program_invocation_short_name;
+
+int main(int argc, char** argv) {
+  if (argc != 2) {
+    printf("Usage: %s <path_to_directory>\n", program_invocation_short_name);
+    return 1;
+  }
+
+  int dir = open(argv[1], O_DIRECTORY | O_CLOEXEC);
+  if (dir < 0) {
+    perror("Failed to open directory");
+    return 1;
+  }
+
+  struct fscrypt_policy policy;
+  int ret = ioctl(dir, FS_IOC_GET_ENCRYPTION_POLICY, &policy);
+  if (ret < 0) {
+    perror("FS_IOC_GET_ENCRYPTION_POLICY failed");
+    return 1;
+  }
+
+  printf("File system encryption policy:\n");
+  printf("\tversion = %#x\n", policy.version);
+  printf("\tcontents_encryption_mode = %#x\n", policy.contents_encryption_mode);
+  printf("\tfilenames_encryption_mode = %#x\n",
+         policy.filenames_encryption_mode);
+  printf("\tflags = %#x\n", policy.flags);
+  printf("\tmaster_key_descriptor = 0x");
+  for (int i = 0; i < FS_KEY_DESCRIPTOR_SIZE; ++i) {
+    printf("%x", policy.master_key_descriptor[i]);
+  }
+  printf("\n");
+
+  ret = ioctl(dir, FS_IOC_SET_ENCRYPTION_POLICY, &policy);
+  if (ret < 0) {
+    perror("FS_IOC_SET_ENCRYPTION_POLICY failed");
+    return 1;
+  }
+
+  return 0;
+}