diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index d79d3f7b7..1b0f8c2b0 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1928,6 +1928,12 @@ impl DeviceManager { // SAFETY: stdout is valid and owned solely by us. let stdout = unsafe { File::from_raw_fd(stdout) }; + // SAFETY: FFI call. Trivially safe. + if unsafe { libc::isatty(libc::STDOUT_FILENO) } == 1 { + self.listen_for_sigwinch_on_tty(stdout.try_clone().unwrap()) + .unwrap(); + } + // If an interactive TTY then we can accept input // SAFETY: FFI call. Trivially safe. if unsafe { libc::isatty(libc::STDIN_FILENO) == 1 } { diff --git a/vmm/src/seccomp_filters.rs b/vmm/src/seccomp_filters.rs index 8177d9faf..c48a68fd4 100644 --- a/vmm/src/seccomp_filters.rs +++ b/vmm/src/seccomp_filters.rs @@ -41,6 +41,7 @@ macro_rules! or { const TCGETS: u64 = 0x5401; const TCSETS: u64 = 0x5402; const TIOCSCTTY: u64 = 0x540E; +const TIOCGPGRP: u64 = 0x540F; const TIOCSPGRP: u64 = 0x5410; const TIOCGWINSZ: u64 = 0x5413; const TIOCSPTLCK: u64 = 0x4004_5431; @@ -264,6 +265,7 @@ fn create_vmm_ioctl_seccomp_rule_common( and![Cond::new(1, ArgLen::Dword, Eq, SIOCSIFNETMASK)?], and![Cond::new(1, ArgLen::Dword, Eq, TCSETS)?], and![Cond::new(1, ArgLen::Dword, Eq, TCGETS)?], + and![Cond::new(1, ArgLen::Dword, Eq, TIOCGPGRP)?], and![Cond::new(1, ArgLen::Dword, Eq, TIOCGTPEER)?], and![Cond::new(1, ArgLen::Dword, Eq, TIOCGWINSZ)?], and![Cond::new(1, ArgLen::Dword, Eq, TIOCSCTTY)?], @@ -455,6 +457,7 @@ fn signal_handler_thread_rules() -> Result)>, Backend fn create_pty_foreground_ioctl_seccomp_rule() -> Result, BackendError> { Ok(or![ + and![Cond::new(1, ArgLen::Dword, Eq, TIOCGPGRP)?], and![Cond::new(1, ArgLen::Dword, Eq, TIOCSCTTY)?], and![Cond::new(1, ArgLen::Dword, Eq, TIOCSPGRP)?], ]) diff --git a/vmm/src/sigwinch_listener.rs b/vmm/src/sigwinch_listener.rs index cad87f883..049dc077d 100644 --- a/vmm/src/sigwinch_listener.rs +++ b/vmm/src/sigwinch_listener.rs @@ -5,8 +5,8 @@ use crate::clone3::{clone3, clone_args, CLONE_CLEAR_SIGHAND}; use arch::_NSIG; use libc::{ c_int, c_void, close, fork, getpgrp, ioctl, pipe2, poll, pollfd, setsid, sigemptyset, - siginfo_t, signal, sigprocmask, syscall, tcsetpgrp, SYS_close_range, EINVAL, ENOSYS, O_CLOEXEC, - POLLERR, SIGWINCH, SIG_DFL, SIG_SETMASK, STDERR_FILENO, TIOCSCTTY, + siginfo_t, signal, sigprocmask, syscall, tcgetpgrp, tcsetpgrp, SYS_close_range, EINVAL, ENOSYS, + ENOTTY, O_CLOEXEC, POLLERR, SIGWINCH, SIG_DFL, SIG_SETMASK, STDERR_FILENO, TIOCSCTTY, }; use seccompiler::{apply_filter, BpfProgram}; use std::cell::RefCell; @@ -118,13 +118,48 @@ unsafe fn close_unused_fds(keep_fds: &mut [RawFd]) { } } -fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, pty: File) -> ! { - let pty_fd = pty.into_raw_fd(); +fn set_foreground_process_group(tty: &mut File) -> io::Result<()> { + // SAFETY: trivially safe. + let my_pgrp = unsafe { getpgrp() }; + // SAFETY: we have borrowed tty. + let tty_pgrp = unsafe { tcgetpgrp(tty.as_raw_fd()) }; + if tty_pgrp == -1 { + let e = io::Error::last_os_error(); + if e.raw_os_error() != Some(ENOTTY) { + return Err(e); + } + } + if tty_pgrp == my_pgrp { + return Ok(()); + } + + // SAFETY: trivially safe. + let my_pgrp = unsafe { setsid() }; + if my_pgrp == -1 { + return Err(io::Error::last_os_error()); + } + + // Set the tty to be this process's controlling terminal. + // SAFETY: we have borrowed tty. + if unsafe { ioctl(tty.as_raw_fd(), TIOCSCTTY, 0) } == -1 { + return Err(io::Error::last_os_error()); + } + + // Become the foreground process group of the tty. + // SAFETY: we have borrowed tty. + if unsafe { tcsetpgrp(tty.as_raw_fd(), my_pgrp) } == -1 { + return Err(io::Error::last_os_error()); + } + + Ok(()) +} + +fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, mut tty: File) -> ! { // SAFETY: any references to these file descriptors are // unreachable, because this function never returns. unsafe { - close_unused_fds(&mut [STDERR_FILENO, tx.as_raw_fd(), pty_fd]); + close_unused_fds(&mut [STDERR_FILENO, tx.as_raw_fd(), tty.as_raw_fd()]); } TX.with(|opt| opt.replace(Some(tx))); @@ -137,20 +172,8 @@ fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, pty: File) -> ! register_signal_handler(SIGWINCH, sigwinch_handler).unwrap(); - // SAFETY: FFI calls - unsafe { - // Create a new session (and therefore a new process group). - assert_ne!(setsid(), -1); - - // Set the tty to be this process's controlling terminal. - assert_ne!(ioctl(pty_fd, TIOCSCTTY, 0), -1); - - // Become the foreground process group of the tty. - assert_ne!(tcsetpgrp(pty_fd, getpgrp()), -1); - - // Close the PTY fd - assert_ne!(close(pty_fd), -1); - } + set_foreground_process_group(&mut tty).unwrap(); + drop(tty); notify(); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index f3c64d50c..0633c317d 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -21,7 +21,7 @@ use crate::coredump::{ CpuElf64Writable, DumpState, Elf64Writable, GuestDebuggable, GuestDebuggableError, NoteDescType, }; use crate::cpu; -use crate::device_manager::{Console, DeviceManager, DeviceManagerError, PtyPair}; +use crate::device_manager::{DeviceManager, DeviceManagerError, PtyPair}; use crate::device_tree::DeviceTree; #[cfg(feature = "guest_debug")] use crate::gdb::{Debuggable, DebuggableError, GdbRequestPayload, GdbResponsePayload}; @@ -33,7 +33,6 @@ use crate::migration::get_vm_snapshot; #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))] use crate::migration::url_to_file; use crate::migration::{url_to_path, SNAPSHOT_CONFIG_FILE, SNAPSHOT_STATE_FILE}; -use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::GuestMemoryMmap; use crate::{ PciDeviceInfo, CPU_MANAGER_SNAPSHOT_ID, DEVICE_MANAGER_SNAPSHOT_ID, MEMORY_MANAGER_SNAPSHOT_ID, @@ -56,6 +55,7 @@ use gdbstub_arch::aarch64::reg::AArch64CoreRegs as CoreRegs; #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))] use gdbstub_arch::x86::reg::X86_64CoreRegs as CoreRegs; use hypervisor::{HypervisorVmError, VmOps}; +use libc::SIGWINCH; use linux_loader::cmdline::Cmdline; #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))] use linux_loader::elf; @@ -64,9 +64,8 @@ use linux_loader::loader::elf::PvhBootCapability::PvhEntryPresent; #[cfg(target_arch = "aarch64")] use linux_loader::loader::pe::Error::InvalidImageMagicNumber; use linux_loader::loader::KernelLoader; -use seccompiler::{apply_filter, SeccompAction}; +use seccompiler::SeccompAction; use serde::{Deserialize, Serialize}; -use signal_hook::{consts::SIGWINCH, iterator::backend::Handle, iterator::Signals}; use std::cmp; use std::collections::BTreeMap; use std::collections::HashMap; @@ -80,7 +79,6 @@ use std::mem::size_of; use std::num::Wrapping; use std::ops::Deref; use std::os::unix::net::UnixStream; -use std::panic::AssertUnwindSafe; use std::sync::{Arc, Mutex, RwLock}; use std::time::Instant; use std::{result, str, thread}; @@ -96,7 +94,6 @@ use vm_migration::{ SnapshotData, Snapshottable, Transportable, }; use vmm_sys_util::eventfd::EventFd; -use vmm_sys_util::signal::unblock_signal; use vmm_sys_util::sock_ctrl_msg::ScmSocket; use vmm_sys_util::terminal::Terminal; @@ -441,7 +438,6 @@ pub struct Vm { device_manager: Arc>, config: Arc>, on_tty: bool, - signals: Option, state: RwLock, cpu_manager: Arc>, memory_manager: Arc>, @@ -451,8 +447,7 @@ pub struct Vm { #[cfg(all(feature = "kvm", target_arch = "x86_64"))] saved_clock: Option, numa_nodes: NumaNodes, - seccomp_action: SeccompAction, - exit_evt: EventFd, + #[cfg_attr(any(not(feature = "kvm"), target_arch = "aarch64"), allow(dead_code))] hypervisor: Arc, stop_on_boot: bool, load_payload_handle: Option>>, @@ -646,7 +641,6 @@ impl Vm { config, on_tty, threads: Vec::with_capacity(1), - signals: None, state: RwLock::new(vm_state), cpu_manager, memory_manager, @@ -654,8 +648,6 @@ impl Vm { #[cfg(all(feature = "kvm", target_arch = "x86_64"))] saved_clock, numa_nodes, - seccomp_action: seccomp_action.clone(), - exit_evt, hypervisor, stop_on_boot, load_payload_handle, @@ -1222,11 +1214,6 @@ impl Vm { .map_err(Error::SetTerminalCanon)?; } - // Trigger the termination of the signal_handler thread - if let Some(signals) = self.signals.take() { - signals.close(); - } - // Wake up the DeviceManager threads so they will get terminated cleanly self.device_manager .lock() @@ -1632,18 +1619,6 @@ impl Vm { Ok(self.device_manager.lock().unwrap().counters()) } - fn signal_handler(mut signals: Signals, console_input_clone: Arc) { - for sig in &Vm::HANDLED_SIGNALS { - unblock_signal(*sig).unwrap(); - } - - for signal in signals.forever() { - if signal == SIGWINCH { - console_input_clone.update_console_size(); - } - } - } - #[cfg(feature = "tdx")] fn extract_tdvf_sections(&mut self) -> Result<(Vec, bool)> { use arch::x86_64::tdx::*; @@ -1940,54 +1915,6 @@ impl Vm { Ok(()) } - fn setup_signal_handler(&mut self) -> Result<()> { - let console = self.device_manager.lock().unwrap().console().clone(); - let signals = Signals::new(Vm::HANDLED_SIGNALS); - - if !console.need_resize() { - return Ok(()); - } - - match signals { - Ok(signals) => { - self.signals = Some(signals.handle()); - let exit_evt = self.exit_evt.try_clone().map_err(Error::EventFdClone)?; - let signal_handler_seccomp_filter = get_seccomp_filter( - &self.seccomp_action, - Thread::SignalHandler, - self.hypervisor.hypervisor_type(), - ) - .map_err(Error::CreateSeccompFilter)?; - self.threads.push( - thread::Builder::new() - .name("vm_signal_handler".to_string()) - .spawn(move || { - if !signal_handler_seccomp_filter.is_empty() { - if let Err(e) = apply_filter(&signal_handler_seccomp_filter) - .map_err(Error::ApplySeccompFilter) - { - error!("Error applying seccomp filter: {:?}", e); - exit_evt.write(1).ok(); - return; - } - } - std::panic::catch_unwind(AssertUnwindSafe(|| { - Vm::signal_handler(signals, console); - })) - .map_err(|_| { - error!("vm signal_handler thread panicked"); - exit_evt.write(1).ok() - }) - .ok(); - }) - .map_err(Error::SignalHandlerSpawn)?, - ); - } - Err(e) => error!("Signal not found {}", e), - } - Ok(()) - } - fn setup_tty(&self) -> Result<()> { if self.on_tty { io::stdin() @@ -2052,7 +1979,6 @@ impl Vm { #[cfg(target_arch = "x86_64")] let rsdp_addr = self.create_acpi_tables(); - self.setup_signal_handler()?; self.setup_tty()?; // Load kernel synchronously or if asynchronous then wait for load to @@ -2167,7 +2093,6 @@ impl Vm { .start_restored_vcpus() .map_err(Error::CpuManager)?; - self.setup_signal_handler()?; self.setup_tty()?; event!("vm", "restored");