From 3c923f072724781c513387e997e5f4fe8bfe2168 Mon Sep 17 00:00:00 2001 From: Bo Chen Date: Tue, 8 Sep 2020 19:35:03 -0700 Subject: [PATCH] virtio-devices: seccomp: Add seccomp filters for virtio_vsock thread This patch enables the seccomp filters for the virtio_vsock worker thread. Partially fixes: #925 Signed-off-by: Bo Chen --- virtio-devices/src/seccomp_filters.rs | 57 ++++++++++++++++++++++++++- virtio-devices/src/vsock/device.rs | 13 +++++- virtio-devices/src/vsock/mod.rs | 1 + vmm/src/device_manager.rs | 1 + 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/virtio-devices/src/seccomp_filters.rs b/virtio-devices/src/seccomp_filters.rs index cbc2a58fd..7b4ee776e 100644 --- a/virtio-devices/src/seccomp_filters.rs +++ b/virtio-devices/src/seccomp_filters.rs @@ -5,7 +5,9 @@ // SPDX-License-Identifier: Apache-2.0 use seccomp::{ - allow_syscall, BpfProgram, Error, SeccompAction, SeccompError, SeccompFilter, SyscallRuleSet, + allow_syscall, allow_syscall_if, BpfProgram, Error, SeccompAction, SeccompCmpArgLen as ArgLen, + SeccompCmpOp::Eq, SeccompCondition as Cond, SeccompError, SeccompFilter, SeccompRule, + SyscallRuleSet, }; use std::convert::TryInto; @@ -24,11 +26,35 @@ pub enum Thread { VirtioVhostFs, VirtioVhostNet, VirtioVhostNetCtl, + VirtioVsock, +} + +/// Shorthand for chaining `SeccompCondition`s with the `and` operator in a `SeccompRule`. +/// The rule will take the `Allow` action if _all_ the conditions are true. +/// +/// [`Allow`]: enum.SeccompAction.html +/// [`SeccompCondition`]: struct.SeccompCondition.html +/// [`SeccompRule`]: struct.SeccompRule.html +macro_rules! and { + ($($x:expr,)*) => (SeccompRule::new(vec![$($x),*], SeccompAction::Allow)); + ($($x:expr),*) => (SeccompRule::new(vec![$($x),*], SeccompAction::Allow)) +} + +/// Shorthand for chaining `SeccompRule`s with the `or` operator in a `SeccompFilter`. +/// +/// [`SeccompFilter`]: struct.SeccompFilter.html +/// [`SeccompRule`]: struct.SeccompRule.html +macro_rules! or { + ($($x:expr,)*) => (vec![$($x),*]); + ($($x:expr),*) => (vec![$($x),*]) } // Define io_uring syscalls as they are not yet part of libc. const SYS_IO_URING_ENTER: i64 = 426; +// See include/uapi/asm-generic/ioctls.h in the kernel code. +const FIONBIO: u64 = 0x5421; + fn virtio_balloon_thread_rules() -> Result, Error> { Ok(vec![ allow_syscall(libc::SYS_brk), @@ -349,6 +375,33 @@ fn virtio_vhost_net_ctl_thread_rules() -> Result, Error> { ]) } +fn create_vsock_ioctl_seccomp_rule() -> Result, Error> { + Ok(or![and![Cond::new(1, ArgLen::DWORD, Eq, FIONBIO,)?],]) +} + +fn virtio_vsock_thread_rules() -> Result, Error> { + Ok(vec![ + allow_syscall(libc::SYS_accept4), + allow_syscall(libc::SYS_close), + allow_syscall(libc::SYS_dup), + allow_syscall(libc::SYS_epoll_create1), + allow_syscall(libc::SYS_epoll_ctl), + allow_syscall(libc::SYS_epoll_pwait), + #[cfg(target_arch = "x86_64")] + allow_syscall(libc::SYS_epoll_wait), + allow_syscall(libc::SYS_exit), + allow_syscall_if(libc::SYS_ioctl, create_vsock_ioctl_seccomp_rule()?), + allow_syscall(libc::SYS_futex), + allow_syscall(libc::SYS_madvise), + allow_syscall(libc::SYS_munmap), + allow_syscall(libc::SYS_read), + allow_syscall(libc::SYS_recvfrom), + allow_syscall(libc::SYS_rt_sigprocmask), + allow_syscall(libc::SYS_sigaltstack), + allow_syscall(libc::SYS_write), + ]) +} + fn get_seccomp_filter_trap(thread_type: Thread) -> Result { let rules = match thread_type { Thread::VirtioBalloon => virtio_balloon_thread_rules()?, @@ -365,6 +418,7 @@ fn get_seccomp_filter_trap(thread_type: Thread) -> Result Thread::VirtioVhostFs => virtio_vhost_fs_thread_rules()?, Thread::VirtioVhostNet => virtio_vhost_net_thread_rules()?, Thread::VirtioVhostNetCtl => virtio_vhost_net_ctl_thread_rules()?, + Thread::VirtioVsock => virtio_vsock_thread_rules()?, }; Ok(SeccompFilter::new( @@ -389,6 +443,7 @@ fn get_seccomp_filter_log(thread_type: Thread) -> Result { Thread::VirtioVhostFs => virtio_vhost_fs_thread_rules()?, Thread::VirtioVhostNet => virtio_vhost_net_thread_rules()?, Thread::VirtioVhostNetCtl => virtio_vhost_net_ctl_thread_rules()?, + Thread::VirtioVsock => virtio_vsock_thread_rules()?, }; Ok(SeccompFilter::new( diff --git a/virtio-devices/src/vsock/device.rs b/virtio-devices/src/vsock/device.rs index 8ac23de57..170d0f5d1 100644 --- a/virtio-devices/src/vsock/device.rs +++ b/virtio-devices/src/vsock/device.rs @@ -9,6 +9,7 @@ // found in the THIRD-PARTY file. use super::{VsockBackend, VsockPacket}; +use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::Error as DeviceError; use crate::VirtioInterrupt; use crate::{ @@ -37,6 +38,7 @@ use anyhow::anyhow; /// - a backend FD. /// use byteorder::{ByteOrder, LittleEndian}; +use seccomp::{SeccompAction, SeccompFilter}; use std::io; use std::os::unix::io::AsRawFd; use std::path::PathBuf; @@ -300,6 +302,7 @@ pub struct Vsock { cid: u64, backend: Arc>, path: PathBuf, + seccomp_action: SeccompAction, } #[derive(Serialize, Deserialize)] @@ -320,6 +323,7 @@ where path: PathBuf, backend: B, iommu: bool, + seccomp_action: SeccompAction, ) -> io::Result> { let mut avail_features = 1u64 << VIRTIO_F_VERSION_1 | 1u64 << VIRTIO_F_IN_ORDER; @@ -339,6 +343,7 @@ where cid, backend: Arc::new(RwLock::new(backend)), path, + seccomp_action, }) } @@ -446,10 +451,16 @@ where let paused = self.common.paused.clone(); let paused_sync = self.common.paused_sync.clone(); let mut epoll_threads = Vec::new(); + // Retrieve seccomp filter for virtio_vsock thread + let virtio_vsock_seccomp_filter = + get_seccomp_filter(&self.seccomp_action, Thread::VirtioVsock) + .map_err(ActivateError::CreateSeccompFilter)?; thread::Builder::new() .name("virtio_vsock".to_string()) .spawn(move || { - if let Err(e) = handler.run(paused, paused_sync.unwrap()) { + if let Err(e) = SeccompFilter::apply(virtio_vsock_seccomp_filter) { + error!("Error applying seccomp filter: {:?}", e); + } else if let Err(e) = handler.run(paused, paused_sync.unwrap()) { error!("Error running worker: {:?}", e); } }) diff --git a/virtio-devices/src/vsock/mod.rs b/virtio-devices/src/vsock/mod.rs index 4ac1a764f..b21e685c7 100644 --- a/virtio-devices/src/vsock/mod.rs +++ b/virtio-devices/src/vsock/mod.rs @@ -273,6 +273,7 @@ mod tests { PathBuf::from("/test/sock"), TestBackend::new(), false, + seccomp::SeccompAction::Trap, ) .unwrap(), } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index a93ad5598..5610f94e5 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -2378,6 +2378,7 @@ impl DeviceManager { vsock_cfg.socket.clone(), backend, vsock_cfg.iommu, + self.seccomp_action.clone(), ) .map_err(DeviceManagerError::CreateVirtioVsock)?, ));