diff --git a/src/main.rs b/src/main.rs index 7ac9e393e..028aad9de 100644 --- a/src/main.rs +++ b/src/main.rs @@ -247,7 +247,7 @@ fn create_app<'a, 'b>( .arg( Arg::with_name("serial") .long("serial") - .help("Control serial port: off|null|tty|file=/path/to/a/file") + .help("Control serial port: off|null|pty|tty|file=/path/to/a/file") .default_value("null") .group("vm-config"), ) @@ -255,7 +255,7 @@ fn create_app<'a, 'b>( Arg::with_name("console") .long("console") .help( - "Control (virtio) console: \"off|null|tty|file=/path/to/a/file,iommu=on|off\"", + "Control (virtio) console: \"off|null|pty|tty|file=/path/to/a/file,iommu=on|off\"", ) .default_value("tty") .group("vm-config"), @@ -1412,6 +1412,57 @@ mod unit_tests { }); } + #[test] + fn test_valid_vm_config_serial_pty_console_pty() { + vec![ + ( + vec!["cloud-hypervisor", "--kernel", "/path/to/kernel"], + r#"{ + "kernel": {"path": "/path/to/kernel"}, + "serial": {"mode": "Null"}, + "console": {"mode": "Tty"} + }"#, + true, + ), + ( + vec![ + "cloud-hypervisor", + "--kernel", + "/path/to/kernel", + "--serial", + "null", + "--console", + "tty", + ], + r#"{ + "kernel": {"path": "/path/to/kernel"} + }"#, + true, + ), + ( + vec![ + "cloud-hypervisor", + "--kernel", + "/path/to/kernel", + "--serial", + "pty", + "--console", + "pty", + ], + r#"{ + "kernel": {"path": "/path/to/kernel"}, + "serial": {"mode": "Pty"}, + "console": {"mode": "Pty"} + }"#, + true, + ), + ] + .iter() + .for_each(|(cli, openapi, equal)| { + compare_vm_config_cli_vs_json(cli, openapi, *equal); + }); + } + #[test] #[cfg(target_arch = "x86_64")] fn test_valid_vm_config_devices() { diff --git a/tests/integration.rs b/tests/integration.rs index d0aabcd6b..a9620ca59 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -27,7 +27,8 @@ mod tests { use std::process::{Child, Command, Stdio}; use std::str::FromStr; use std::string::String; - use std::sync::Mutex; + use std::sync::mpsc::Receiver; + use std::sync::{mpsc, Mutex}; use std::thread; use tempdir::TempDir; use tempfile::NamedTempFile; @@ -2363,6 +2364,37 @@ mod tests { } } + fn pty_read(mut pty: std::fs::File) -> Receiver { + let (tx, rx) = mpsc::channel::(); + thread::spawn(move || loop { + thread::sleep(std::time::Duration::new(1, 0)); + let mut buf = [0; 512]; + match pty.read(&mut buf) { + Ok(_) => { + let output = std::str::from_utf8(&buf).unwrap().to_string(); + match tx.send(output) { + Ok(_) => (), + Err(_) => break, + } + } + Err(_) => break, + } + }); + rx + } + + fn get_pty_path(api_socket: &str, pty_type: &str) -> PathBuf { + let (cmd_success, cmd_output) = remote_command_w_output(&api_socket, "info", None); + assert!(cmd_success); + let info: serde_json::Value = serde_json::from_slice(&cmd_output).unwrap_or_default(); + assert_eq!("Pty", info["config"][pty_type]["mode"]); + PathBuf::from( + info["config"][pty_type]["file"] + .as_str() + .expect("Missing pty path"), + ) + } + mod parallel { use crate::tests::*; @@ -3654,6 +3686,94 @@ mod tests { handle_child_output(r, &output); } + #[test] + #[cfg(target_arch = "x86_64")] + fn test_pty_interaction() { + let mut focal = UbuntuDiskConfig::new(FOCAL_IMAGE_NAME.to_string()); + let guest = Guest::new(&mut focal); + let api_socket = temp_api_path(&guest.tmp_dir); + let cmdline = DIRECT_KERNEL_BOOT_CMDLINE.to_owned() + " console=ttyS0"; + + let mut child = GuestCommand::new(&guest) + .args(&["--cpus", "boot=1"]) + .args(&["--memory", "size=512M"]) + .args(&[ + "--kernel", + direct_kernel_boot_path().unwrap().to_str().unwrap(), + ]) + .args(&["--cmdline", &cmdline]) + .default_disks() + .default_net() + .args(&["--serial", "null"]) + .args(&["--console", "pty"]) + .args(&["--api-socket", &api_socket]) + .spawn() + .unwrap(); + + let r = std::panic::catch_unwind(|| { + guest.wait_vm_boot(None).unwrap(); + // Get pty fd for console + let console_path = get_pty_path(&api_socket, "console"); + // TODO: Get serial pty test working + let mut cf = std::fs::OpenOptions::new() + .write(true) + .read(true) + .open(console_path) + .unwrap(); + + // Some dumb sleeps but we don't want to write + // before the console is up and we don't want + // to try and write the next line before the + // login process is ready. + thread::sleep(std::time::Duration::new(5, 0)); + assert_eq!(cf.write(b"cloud\n").unwrap(), 6); + thread::sleep(std::time::Duration::new(2, 0)); + assert_eq!(cf.write(b"cloud123\n").unwrap(), 9); + thread::sleep(std::time::Duration::new(2, 0)); + assert_eq!(cf.write(b"echo test_pty_console\n").unwrap(), 22); + thread::sleep(std::time::Duration::new(2, 0)); + + // read pty and ensure they have a login shell + // some fairly hacky workarounds to avoid looping + // forever in case the channel is blocked getting output + let ptyc = pty_read(cf); + let mut empty = 0; + let mut prev = String::new(); + loop { + thread::sleep(std::time::Duration::new(2, 0)); + match ptyc.try_recv() { + Ok(line) => { + empty = 0; + prev = prev + &line; + if prev.contains("test_pty_console") { + break; + } + } + Err(mpsc::TryRecvError::Empty) => { + empty += 1; + if empty > 5 { + panic!("No login on pty"); + } + } + _ => panic!("No login on pty"), + } + } + + guest.ssh_command("sudo shutdown -h now").unwrap(); + }); + + let _ = child.wait_timeout(std::time::Duration::from_secs(20)); + let _ = child.kill(); + let output = child.wait_with_output().unwrap(); + handle_child_output(r, &output); + + let r = std::panic::catch_unwind(|| { + // Check that the cloud-hypervisor binary actually terminated + assert_eq!(output.status.success(), true); + }); + handle_child_output(r, &output); + } + #[test] fn test_virtio_console() { let mut focal = UbuntuDiskConfig::new(FOCAL_IMAGE_NAME.to_string()); diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index 6ed3476ac..d515b0c91 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -758,7 +758,7 @@ components: type: string mode: type: string - enum: [Off, Tty, File, Null] + enum: [Off, Pty, Tty, File, Null] iommu: type: boolean default: false diff --git a/vmm/src/config.rs b/vmm/src/config.rs index ecec20012..342194b1c 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -1170,6 +1170,7 @@ impl PmemConfig { #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] pub enum ConsoleOutputMode { Off, + Pty, Tty, File, Null, @@ -1177,7 +1178,7 @@ pub enum ConsoleOutputMode { impl ConsoleOutputMode { pub fn input_enabled(&self) -> bool { - matches!(self, ConsoleOutputMode::Tty) + matches!(self, ConsoleOutputMode::Tty | ConsoleOutputMode::Pty) } } @@ -1199,6 +1200,7 @@ impl ConsoleConfig { let mut parser = OptionParser::new(); parser .add_valueless("off") + .add_valueless("pty") .add_valueless("tty") .add_valueless("null") .add("file") @@ -1209,6 +1211,8 @@ impl ConsoleConfig { let mut mode: ConsoleOutputMode = ConsoleOutputMode::Off; if parser.is_set("off") { + } else if parser.is_set("pty") { + mode = ConsoleOutputMode::Pty } else if parser.is_set("tty") { mode = ConsoleOutputMode::Tty } else if parser.is_set("null") { @@ -2156,6 +2160,14 @@ mod tests { file: None, } ); + assert_eq!( + ConsoleConfig::parse("pty")?, + ConsoleConfig { + mode: ConsoleOutputMode::Pty, + iommu: false, + file: None, + } + ); assert_eq!( ConsoleConfig::parse("tty")?, ConsoleConfig { diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index ba5a6d456..01714e02a 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -57,8 +57,10 @@ use hypervisor::kvm_ioctls::*; use hypervisor::CpuState; #[cfg(feature = "mshv")] use hypervisor::IoEventAddress; -use libc::TIOCGWINSZ; -use libc::{MAP_NORESERVE, MAP_PRIVATE, MAP_SHARED, O_TMPFILE, PROT_READ, PROT_WRITE}; +use libc::{ + isatty, tcgetattr, tcsetattr, termios, ECHO, ICANON, ISIG, MAP_NORESERVE, MAP_PRIVATE, + MAP_SHARED, O_TMPFILE, PROT_READ, PROT_WRITE, TCSANOW, TIOCGWINSZ, +}; use pci::{ DeviceRelocation, PciBarRegionType, PciBus, PciConfigIo, PciConfigMmio, PciDevice, PciRoot, VfioPciDevice, @@ -66,12 +68,14 @@ use pci::{ use seccomp::SeccompAction; use std::any::Any; use std::collections::HashMap; -use std::fs::{File, OpenOptions}; +use std::convert::TryInto; +use std::fs::{read_link, File, OpenOptions}; use std::io::{self, sink, stdout, Seek, SeekFrom}; +use std::mem::zeroed; use std::num::Wrapping; use std::os::unix::fs::OpenOptionsExt; -#[cfg(feature = "kvm")] -use std::os::unix::io::FromRawFd; +use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; +use std::path::PathBuf; use std::result; use std::sync::{Arc, Barrier, Mutex}; #[cfg(feature = "kvm")] @@ -228,6 +232,18 @@ pub enum DeviceManagerError { /// Error creating console output file ConsoleOutputFileOpen(io::Error), + /// Error creating serial pty + SerialPtyOpen(io::Error), + + /// Error creating console pty + ConsolePtyOpen(io::Error), + + /// Error setting pty raw mode + SetPtyRaw(vmm_sys_util::errno::Error), + + /// Error getting pty peer + GetPtyPeer(vmm_sys_util::errno::Error), + /// Cannot create a VFIO device VfioCreate(vfio_ioctls::VfioError), @@ -406,6 +422,56 @@ pub fn get_win_size() -> (u16, u16) { (ws.cols, ws.rows) } +const TIOCSPTLCK: libc::c_int = 0x4004_5431; +const TIOCGTPEER: libc::c_int = 0x5441; + +pub fn create_pty() -> io::Result<(File, File, PathBuf)> { + // Try to use /dev/pts/ptmx first then fall back to /dev/ptmx + // This is done to try and use the devpts filesystem that + // could be available for use in the process's namespace first. + // Ideally these are all the same file though but different + // kernels could have things setup differently. + // See https://www.kernel.org/doc/Documentation/filesystems/devpts.txt + // for further details. + let main = match OpenOptions::new() + .read(true) + .write(true) + .custom_flags(libc::O_NOCTTY) + .open("/dev/pts/ptmx") + { + Ok(f) => f, + _ => OpenOptions::new() + .read(true) + .write(true) + .custom_flags(libc::O_NOCTTY) + .open("/dev/ptmx")?, + }; + let mut unlock: libc::c_ulong = 0; + unsafe { + libc::ioctl( + main.as_raw_fd(), + TIOCSPTLCK.try_into().unwrap(), + &mut unlock, + ) + }; + + let sub_fd = unsafe { + libc::ioctl( + main.as_raw_fd(), + TIOCGTPEER.try_into().unwrap(), + libc::O_NOCTTY | libc::O_RDWR, + ) + }; + if sub_fd == -1 { + return vmm_sys_util::errno::errno_result().map_err(|e| e.into()); + } + + let proc_path = PathBuf::from(format!("/proc/self/fd/{}", sub_fd)); + let path = read_link(proc_path)?; + + Ok((main, unsafe { File::from_raw_fd(sub_fd) }, path)) +} + enum ConsoleInput { Serial, VirtioConsole, @@ -422,23 +488,11 @@ impl Console { pub fn queue_input_bytes(&self, out: &[u8]) -> vmm_sys_util::errno::Result<()> { match self.input { Some(ConsoleInput::Serial) => { - if self.serial.is_some() { - self.serial - .as_ref() - .unwrap() - .lock() - .expect("Failed to process stdin event due to poisoned lock") - .queue_input_bytes(out)?; - } + self.queue_input_bytes_serial(out)?; } Some(ConsoleInput::VirtioConsole) => { - if self.virtio_console_input.is_some() { - self.virtio_console_input - .as_ref() - .unwrap() - .queue_input_bytes(out); - } + self.queue_input_bytes_console(out); } None => {} } @@ -446,6 +500,27 @@ impl Console { Ok(()) } + pub fn queue_input_bytes_serial(&self, out: &[u8]) -> vmm_sys_util::errno::Result<()> { + if self.serial.is_some() { + self.serial + .as_ref() + .unwrap() + .lock() + .unwrap() + .queue_input_bytes(out)?; + } + Ok(()) + } + + pub fn queue_input_bytes_console(&self, out: &[u8]) { + if self.virtio_console_input.is_some() { + self.virtio_console_input + .as_ref() + .unwrap() + .queue_input_bytes(out); + } + } + pub fn update_console_size(&self, cols: u16, rows: u16) { if self.virtio_console_input.is_some() { self.virtio_console_input @@ -712,6 +787,12 @@ pub struct DeviceManager { // Console abstraction console: Arc, + // console PTY + console_pty: Option>>, + + // serial PTY + serial_pty: Option>>, + // Interrupt controller #[cfg(target_arch = "x86_64")] interrupt_controller: Option>>, @@ -881,6 +962,8 @@ impl DeviceManager { .map_err(DeviceManagerError::EventFd)?, #[cfg(feature = "acpi")] acpi_address, + serial_pty: None, + console_pty: None, }; let device_manager = Arc::new(Mutex::new(device_manager)); @@ -898,6 +981,18 @@ impl DeviceManager { Ok(device_manager) } + pub fn serial_pty(&self) -> Option { + self.serial_pty + .as_ref() + .map(|pty| pty.lock().unwrap().0.try_clone().unwrap()) + } + + pub fn console_pty(&self) -> Option { + self.console_pty + .as_ref() + .map(|pty| pty.lock().unwrap().0.try_clone().unwrap()) + } + pub fn create_devices(&mut self) -> DeviceManagerResult<()> { let mut virtio_devices: Vec<(VirtioDeviceArc, bool, String)> = Vec::new(); @@ -1488,6 +1583,37 @@ impl DeviceManager { Ok(serial) } + fn modify_mode( + &self, + fd: RawFd, + f: F, + ) -> vmm_sys_util::errno::Result<()> { + // Safe because we check the return value of isatty. + if unsafe { isatty(fd) } != 1 { + return Ok(()); + } + + // The following pair are safe because termios gets totally overwritten by tcgetattr and we + // check the return result. + let mut termios: termios = unsafe { zeroed() }; + let ret = unsafe { tcgetattr(fd, &mut termios as *mut _) }; + if ret < 0 { + return vmm_sys_util::errno::errno_result(); + } + f(&mut termios); + // Safe because the syscall will only read the extent of termios and we check the return result. + let ret = unsafe { tcsetattr(fd, TCSANOW, &termios as *const _) }; + if ret < 0 { + return vmm_sys_util::errno::errno_result(); + } + + Ok(()) + } + + fn set_raw_mode(&self, f: &mut File) -> vmm_sys_util::errno::Result<()> { + self.modify_mode(f.as_raw_fd(), |t| t.c_lflag &= !(ICANON | ECHO | ISIG)) + } + fn add_console_device( &mut self, interrupt_manager: &Arc>, @@ -1499,6 +1625,18 @@ impl DeviceManager { File::create(serial_config.file.as_ref().unwrap()) .map_err(DeviceManagerError::SerialOutputFileOpen)?, )), + ConsoleOutputMode::Pty => { + let (main, mut sub, path) = + create_pty().map_err(DeviceManagerError::SerialPtyOpen)?; + self.set_raw_mode(&mut sub) + .map_err(DeviceManagerError::SetPtyRaw)?; + self.serial_pty = Some(Arc::new(Mutex::new(( + main.try_clone().unwrap(), + sub.try_clone().unwrap(), + )))); + self.config.lock().unwrap().serial.file = Some(path); + Some(Box::new(main.try_clone().unwrap())) + } ConsoleOutputMode::Tty => Some(Box::new(stdout())), ConsoleOutputMode::Off | ConsoleOutputMode::Null => None, }; @@ -1515,6 +1653,18 @@ impl DeviceManager { File::create(console_config.file.as_ref().unwrap()) .map_err(DeviceManagerError::ConsoleOutputFileOpen)?, )), + ConsoleOutputMode::Pty => { + let (main, mut sub, path) = + create_pty().map_err(DeviceManagerError::SerialPtyOpen)?; + self.set_raw_mode(&mut sub) + .map_err(DeviceManagerError::SetPtyRaw)?; + self.console_pty = Some(Arc::new(Mutex::new(( + main.try_clone().unwrap(), + sub.try_clone().unwrap(), + )))); + self.config.lock().unwrap().console.file = Some(path); + Some(Box::new(main.try_clone().unwrap())) + } ConsoleOutputMode::Tty => Some(Box::new(stdout())), ConsoleOutputMode::Null => Some(Box::new(sink())), ConsoleOutputMode::Off => None, diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 53452a74f..abb663c38 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -104,6 +104,10 @@ pub enum Error { #[error("Error handling VM stdin: {0:?}")] Stdin(VmError), + /// Cannot handle the VM pty stream + #[error("Error handling VM pty: {0:?}")] + Pty(VmError), + /// Cannot reboot the VM #[error("Error rebooting VM: {0:?}")] VmReboot(VmError), @@ -145,6 +149,7 @@ pub enum EpollDispatch { Stdin, Api, ActivateVirtioDevices, + Pty, } pub struct EpollContext { @@ -354,6 +359,16 @@ impl Vmm { self.hypervisor.clone(), activate_evt, )?; + if let Some(ref serial_pty) = vm.serial_pty() { + self.epoll + .add_event(serial_pty, EpollDispatch::Pty) + .map_err(VmError::EventfdError)?; + }; + if let Some(ref console_pty) = vm.console_pty() { + self.epoll + .add_event(console_pty, EpollDispatch::Pty) + .map_err(VmError::EventfdError)?; + }; self.vm = Some(vm); } } @@ -1116,6 +1131,11 @@ impl Vmm { .map_err(Error::ActivateVirtioDevices)?; } } + EpollDispatch::Pty => { + if let Some(ref vm) = self.vm { + vm.handle_pty().map_err(Error::Pty)?; + } + } EpollDispatch::Api => { // Consume the event. self.api_evt.read().map_err(Error::EventFdRead)?; diff --git a/vmm/src/seccomp_filters.rs b/vmm/src/seccomp_filters.rs index d14c20053..e74d16a44 100644 --- a/vmm/src/seccomp_filters.rs +++ b/vmm/src/seccomp_filters.rs @@ -47,6 +47,8 @@ const SYS_IO_URING_REGISTER: i64 = 427; const TCGETS: u64 = 0x5401; const TCSETS: u64 = 0x5402; const TIOCGWINSZ: u64 = 0x5413; +const TIOCSPTLCK: u64 = 0x4004_5431; +const TIOCGTPEER: u64 = 0x5441; const FIOCLEX: u64 = 0x5451; const FIONBIO: u64 = 0x5421; @@ -155,6 +157,8 @@ fn create_vmm_ioctl_seccomp_rule_common() -> Result, Error> { and![Cond::new(1, ArgLen::DWORD, Eq, TCSETS)?], and![Cond::new(1, ArgLen::DWORD, Eq, TCGETS)?], and![Cond::new(1, ArgLen::DWORD, Eq, TIOCGWINSZ)?], + and![Cond::new(1, ArgLen::DWORD, Eq, TIOCSPTLCK)?], + and![Cond::new(1, ArgLen::DWORD, Eq, TIOCGTPEER)?], and![Cond::new(1, ArgLen::DWORD, Eq, TUNGETFEATURES)?], and![Cond::new(1, ArgLen::DWORD, Eq, TUNGETIFF)?], and![Cond::new(1, ArgLen::DWORD, Eq, TUNSETIFF)?], diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index d203c9d98..998c606ee 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -130,6 +130,9 @@ pub enum Error { /// Write to the console failed. Console(vmm_sys_util::errno::Error), + /// Write to the pty console failed. + PtyConsole(io::Error), + /// Cannot setup terminal in raw mode. SetTerminalRaw(vmm_sys_util::errno::Error), @@ -1073,6 +1076,14 @@ impl Vm { Ok(()) } + pub fn serial_pty(&self) -> Option { + self.device_manager.lock().unwrap().serial_pty() + } + + pub fn console_pty(&self) -> Option { + self.device_manager.lock().unwrap().console_pty() + } + pub fn shutdown(&mut self) -> Result<()> { let mut state = self.state.try_write().map_err(|_| Error::PoisonedState)?; let new_state = VmState::Shutdown; @@ -1557,6 +1568,33 @@ impl Vm { Ok(()) } + pub fn handle_pty(&self) -> Result<()> { + // Could be a little dangerous, picks up a lock on device_manager + // and goes into a blocking read. If the epoll loops starts to be + // services by multiple threads likely need to revist this. + let dm = self.device_manager.lock().unwrap(); + let mut out = [0u8; 64]; + if let Some(mut pty) = dm.serial_pty() { + let count = pty.read(&mut out).map_err(Error::PtyConsole)?; + let console = dm.console(); + if console.input_enabled() { + console + .queue_input_bytes_serial(&out[..count]) + .map_err(Error::Console)?; + } + }; + let count = match dm.console_pty() { + Some(mut pty) => pty.read(&mut out).map_err(Error::PtyConsole)?, + None => return Ok(()), + }; + let console = dm.console(); + if console.input_enabled() { + console.queue_input_bytes_console(&out[..count]) + } + + Ok(()) + } + pub fn handle_stdin(&self) -> Result<()> { let mut out = [0u8; 64]; let count = io::stdin()