External/cloud-hypervisor
1
0
Fork 0
mirror of https://github.com/cloud-hypervisor/cloud-hypervisor.git synced 2024-08-23 00:11:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

vmm: reset to the original termios

Browse Source 
Previously, we used two different functions for configuring ttys.
vmm_sys_util::terminal::Terminal::set_raw_mode() was used to configure
stdio ttys, and cfmakeraw() was used to configure ptys created by
cloud-hypervisor.  When I centralized the stdio tty cleanup, I also
switched to using cfmakeraw() everywhere, to avoid duplication.

cfmakeraw sets the OPOST flag, but when we later reset the ttys, we
used vmm_sys_util::terminal::Terminal::set_canon_mode(), which does
not unset this flag.  This meant that the terminal was getting mostly,
but not fully, reset.

To fix this without depending on the implementation of cfmakeraw(),
let's just store the original termios for stdio terminals, and restore
them to exactly the state we found them in when cloud-hypervisor exits.

Fixes: b6feae0a ("vmm: only touch the tty flags if it's being used")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
This commit is contained in:
Alyssa Ross 2023-04-27 12:57:28 +00:00 committed by Rob Bradford
parent dc7931f592
commit c90a0ffff6
3 changed files with 52 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
vmm/src/device_manager.rs
View File

@ -70,7 +70,6 @@ use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::path::PathBuf; use std::path::PathBuf;
use std::result; use std::result;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use std::time::Instant; use std::time::Instant;
use tracer::trace_scoped; use tracer::trace_scoped;
@ -101,7 +100,6 @@ use vm_migration::{
use vm_virtio::AccessPlatform; use vm_virtio::AccessPlatform;
use vm_virtio::VirtioDeviceType; use vm_virtio::VirtioDeviceType;
use vmm_sys_util::eventfd::EventFd; use vmm_sys_util::eventfd::EventFd;
use vmm_sys_util::terminal::Terminal;
#[cfg(target_arch = "aarch64")] #[cfg(target_arch = "aarch64")]
const MMIO_LEN: u64 = 0x1000; const MMIO_LEN: u64 = 0x1000;
@ -833,8 +831,8 @@ pub struct DeviceManager {
// pty foreground status, // pty foreground status,
console_resize_pipe: Option<Arc<File>>, console_resize_pipe: Option<Arc<File>>,
// Are any devices using the tty? // To restore on exit.
on_tty: Option<Arc<AtomicBool>>, original_termios_opt: Arc<Mutex<Option<termios>>>,
// Interrupt controller // Interrupt controller
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
@ -1120,7 +1118,7 @@ impl DeviceManager {
serial_manager: None, serial_manager: None,
console_pty: None, console_pty: None,
console_resize_pipe: None, console_resize_pipe: None,
on_tty: None, original_termios_opt: Arc::new(Mutex::new(None)),
virtio_mem_devices: Vec::new(), virtio_mem_devices: Vec::new(),
#[cfg(target_arch = "aarch64")] #[cfg(target_arch = "aarch64")]
gpio_device: None, gpio_device: None,
@ -1168,7 +1166,7 @@ impl DeviceManager {
serial_pty: Option<PtyPair>, serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>, console_pty: Option<PtyPair>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<File>,
on_tty: Arc<AtomicBool>, original_termios_opt: Arc<Mutex<Option<termios>>>,
) -> DeviceManagerResult<()> { ) -> DeviceManagerResult<()> {
trace_scoped!("create_devices"); trace_scoped!("create_devices");
@ -1224,15 +1222,15 @@ impl DeviceManager {
)?; )?;
} }
self.original_termios_opt = original_termios_opt;
self.console = self.add_console_device( self.console = self.add_console_device(
&legacy_interrupt_manager, &legacy_interrupt_manager,
&mut virtio_devices, &mut virtio_devices,
serial_pty, serial_pty,
console_pty, console_pty,
console_resize_pipe, console_resize_pipe,
&on_tty,
)?; )?;
self.on_tty = Some(on_tty);
if let Some(tpm) = self.config.clone().lock().unwrap().tpm.as_ref() { if let Some(tpm) = self.config.clone().lock().unwrap().tpm.as_ref() {
let tpm_dev = self.add_tpm_device(tpm.socket.clone())?; let tpm_dev = self.add_tpm_device(tpm.socket.clone())?;
@ -1845,7 +1843,7 @@ impl DeviceManager {
} }
fn modify_mode<F: FnOnce(&mut termios)>( fn modify_mode<F: FnOnce(&mut termios)>(
&self, &mut self,
fd: RawFd, fd: RawFd,
f: F, f: F,
) -> vmm_sys_util::errno::Result<()> { ) -> vmm_sys_util::errno::Result<()> {
@ -1862,6 +1860,10 @@ impl DeviceManager {
if ret < 0 { if ret < 0 {
return vmm_sys_util::errno::errno_result(); return vmm_sys_util::errno::errno_result();
} }
let mut original_termios_opt = self.original_termios_opt.lock().unwrap();
if original_termios_opt.is_none() {
*original_termios_opt = Some(termios);
}
f(&mut termios); f(&mut termios);
// SAFETY: Safe because the syscall will only read the extent of termios and we check // SAFETY: Safe because the syscall will only read the extent of termios and we check
// the return result. // the return result.
@ -1873,7 +1875,7 @@ impl DeviceManager {
Ok(()) Ok(())
} }
fn set_raw_mode(&self, f: &mut dyn AsRawFd) -> vmm_sys_util::errno::Result<()> { fn set_raw_mode(&mut self, f: &mut dyn AsRawFd) -> vmm_sys_util::errno::Result<()> {
// SAFETY: FFI call. Variable t is guaranteed to be a valid termios from modify_mode. // SAFETY: FFI call. Variable t is guaranteed to be a valid termios from modify_mode.
self.modify_mode(f.as_raw_fd(), |t| unsafe { cfmakeraw(t) }) self.modify_mode(f.as_raw_fd(), |t| unsafe { cfmakeraw(t) })
} }
@ -1897,7 +1899,6 @@ impl DeviceManager {
virtio_devices: &mut Vec<MetaVirtioDevice>, virtio_devices: &mut Vec<MetaVirtioDevice>,
console_pty: Option<PtyPair>, console_pty: Option<PtyPair>,
resize_pipe: Option<File>, resize_pipe: Option<File>,
on_tty: &Arc<AtomicBool>,
) -> DeviceManagerResult<Option<Arc<virtio_devices::ConsoleResizer>>> { ) -> DeviceManagerResult<Option<Arc<virtio_devices::ConsoleResizer>>> {
let console_config = self.config.lock().unwrap().console.clone(); let console_config = self.config.lock().unwrap().console.clone();
let endpoint = match console_config.mode { let endpoint = match console_config.mode {
@ -1938,8 +1939,6 @@ impl DeviceManager {
// SAFETY: stdout is valid and owned solely by us. // SAFETY: stdout is valid and owned solely by us.
let mut stdout = unsafe { File::from_raw_fd(stdout) }; let mut stdout = unsafe { File::from_raw_fd(stdout) };
on_tty.store(true, Ordering::SeqCst);
// Make sure stdout is in raw mode, if it's a terminal. // Make sure stdout is in raw mode, if it's a terminal.
let _ = self.set_raw_mode(&mut stdout); let _ = self.set_raw_mode(&mut stdout);
@ -2019,7 +2018,6 @@ impl DeviceManager {
serial_pty: Option<PtyPair>, serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>, console_pty: Option<PtyPair>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<File>,
on_tty: &Arc<AtomicBool>,
) -> DeviceManagerResult<Arc<Console>> { ) -> DeviceManagerResult<Arc<Console>> {
let serial_config = self.config.lock().unwrap().serial.clone(); let serial_config = self.config.lock().unwrap().serial.clone();
let serial_writer: Option<Box<dyn io::Write + Send>> = match serial_config.mode { let serial_writer: Option<Box<dyn io::Write + Send>> = match serial_config.mode {
@ -2043,7 +2041,6 @@ impl DeviceManager {
} }
ConsoleOutputMode::Tty => { ConsoleOutputMode::Tty => {
let mut out = stdout(); let mut out = stdout();
on_tty.store(true, Ordering::SeqCst);
let _ = self.set_raw_mode(&mut out); let _ = self.set_raw_mode(&mut out);
Some(Box::new(out)) Some(Box::new(out))
} }
@ -2073,12 +2070,8 @@ impl DeviceManager {
}; };
} }
let console_resizer = self.add_virtio_console_device( let console_resizer =
virtio_devices, self.add_virtio_console_device(virtio_devices, console_pty, console_resize_pipe)?;
console_pty,
console_resize_pipe,
on_tty,
)?;
Ok(Arc::new(Console { console_resizer })) Ok(Arc::new(Console { console_resizer }))
} }
@ -4662,10 +4655,9 @@ impl Drop for DeviceManager {
handle.virtio_device.lock().unwrap().shutdown(); handle.virtio_device.lock().unwrap().shutdown();
} }
if let Some(ref on_tty) = self.on_tty { if let Some(termios) = *self.original_termios_opt.lock().unwrap() {
if on_tty.load(Ordering::SeqCst) { // SAFETY: FFI call
let _ = std::io::stdin().lock().set_canon_mode(); let _ = unsafe { tcsetattr(stdout().lock().as_raw_fd(), TCSANOW, &termios) };
}
} }
} }
} }

43
vmm/src/lib.rs
View File

@ -25,7 +25,7 @@ use crate::migration::{recv_vm_config, recv_vm_state};
use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::seccomp_filters::{get_seccomp_filter, Thread};
use crate::vm::{Error as VmError, Vm, VmState}; use crate::vm::{Error as VmError, Vm, VmState};
use anyhow::anyhow; use anyhow::anyhow;
use libc::{EFD_NONBLOCK, SIGINT, SIGTERM}; use libc::{tcsetattr, termios, EFD_NONBLOCK, SIGINT, SIGTERM, TCSANOW};
use memory_manager::MemoryManagerSnapshotData; use memory_manager::MemoryManagerSnapshotData;
use pci::PciBdf; use pci::PciBdf;
use seccompiler::{apply_filter, SeccompAction}; use seccompiler::{apply_filter, SeccompAction};
@ -35,13 +35,12 @@ use signal_hook::iterator::{Handle, Signals};
use std::collections::HashMap; use std::collections::HashMap;
use std::fs::File; use std::fs::File;
use std::io; use std::io;
use std::io::{Read, Write}; use std::io::{stdout, Read, Write};
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::os::unix::net::UnixListener; use std::os::unix::net::UnixListener;
use std::os::unix::net::UnixStream; use std::os::unix::net::UnixStream;
use std::panic::AssertUnwindSafe; use std::panic::AssertUnwindSafe;
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{Receiver, RecvError, SendError, Sender}; use std::sync::mpsc::{Receiver, RecvError, SendError, Sender};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use std::time::Instant; use std::time::Instant;
@ -54,7 +53,6 @@ use vm_migration::{MigratableError, Pausable, Snapshot, Snapshottable, Transport
use vmm_sys_util::eventfd::EventFd; use vmm_sys_util::eventfd::EventFd;
use vmm_sys_util::signal::unblock_signal; use vmm_sys_util::signal::unblock_signal;
use vmm_sys_util::sock_ctrl_msg::ScmSocket; use vmm_sys_util::sock_ctrl_msg::ScmSocket;
use vmm_sys_util::terminal::Terminal;
mod acpi; mod acpi;
pub mod api; pub mod api;
@ -422,13 +420,17 @@ pub struct Vmm {
activate_evt: EventFd, activate_evt: EventFd,
signals: Option<Handle>, signals: Option<Handle>,
threads: Vec<thread::JoinHandle<()>>, threads: Vec<thread::JoinHandle<()>>,
on_tty: Arc<AtomicBool>, original_termios_opt: Arc<Mutex<Option<termios>>>,
} }
impl Vmm { impl Vmm {
pub const HANDLED_SIGNALS: [i32; 2] = [SIGTERM, SIGINT]; pub const HANDLED_SIGNALS: [i32; 2] = [SIGTERM, SIGINT];
fn signal_handler(mut signals: Signals, on_tty: Arc<AtomicBool>, exit_evt: &EventFd) { fn signal_handler(
mut signals: Signals,
original_termios_opt: Arc<Mutex<Option<termios>>>,
exit_evt: &EventFd,
) {
for sig in &Self::HANDLED_SIGNALS { for sig in &Self::HANDLED_SIGNALS {
unblock_signal(*sig).unwrap(); unblock_signal(*sig).unwrap();
} }
@ -438,12 +440,17 @@ impl Vmm {
SIGTERM | SIGINT => { SIGTERM | SIGINT => {
if exit_evt.write(1).is_err() { if exit_evt.write(1).is_err() {
// Resetting the terminal is usually done as the VMM exits // Resetting the terminal is usually done as the VMM exits
if on_tty.load(Ordering::SeqCst) { if let Ok(lock) = original_termios_opt.lock() {
io::stdin() if let Some(termios) = *lock {
.lock() // SAFETY: FFI call
.set_canon_mode() let _ = unsafe {
.expect("failed to restore terminal mode"); tcsetattr(stdout().lock().as_raw_fd(), TCSANOW, &termios)
};
} }
} else {
warn!("Failed to lock original termios");
}
std::process::exit(1); std::process::exit(1);
} }
} }
@ -458,7 +465,7 @@ impl Vmm {
Ok(signals) => { Ok(signals) => {
self.signals = Some(signals.handle()); self.signals = Some(signals.handle());
let exit_evt = self.exit_evt.try_clone().map_err(Error::EventFdClone)?; let exit_evt = self.exit_evt.try_clone().map_err(Error::EventFdClone)?;
let on_tty = Arc::clone(&self.on_tty); let original_termios_opt = Arc::clone(&self.original_termios_opt);
let signal_handler_seccomp_filter = get_seccomp_filter( let signal_handler_seccomp_filter = get_seccomp_filter(
&self.seccomp_action, &self.seccomp_action,
@ -480,7 +487,7 @@ impl Vmm {
} }
} }
std::panic::catch_unwind(AssertUnwindSafe(|| { std::panic::catch_unwind(AssertUnwindSafe(|| {
Vmm::signal_handler(signals, on_tty, &exit_evt); Vmm::signal_handler(signals, original_termios_opt, &exit_evt);
})) }))
.map_err(|_| { .map_err(|_| {
error!("vmm signal_handler thread panicked"); error!("vmm signal_handler thread panicked");
@ -547,7 +554,7 @@ impl Vmm {
activate_evt, activate_evt,
signals: None, signals: None,
threads: vec![], threads: vec![],
on_tty: Arc::new(AtomicBool::new(false)), original_termios_opt: Arc::new(Mutex::new(None)),
}) })
} }
@ -598,7 +605,7 @@ impl Vmm {
None, None,
None, None,
None, None,
Arc::clone(&self.on_tty), Arc::clone(&self.original_termios_opt),
None, None,
None, None,
None, None,
@ -697,7 +704,7 @@ impl Vmm {
None, None,
None, None,
None, None,
Arc::clone(&self.on_tty), Arc::clone(&self.original_termios_opt),
Some(snapshot), Some(snapshot),
Some(source_url), Some(source_url),
Some(restore_cfg.prefault), Some(restore_cfg.prefault),
@ -778,7 +785,7 @@ impl Vmm {
serial_pty, serial_pty,
console_pty, console_pty,
console_resize_pipe, console_resize_pipe,
Arc::clone(&self.on_tty), Arc::clone(&self.original_termios_opt),
None, None,
None, None,
None, None,
@ -1288,7 +1295,7 @@ impl Vmm {
None, None,
None, None,
None, None,
Arc::clone(&self.on_tty), Arc::clone(&self.original_termios_opt),
Some(snapshot), Some(snapshot),
) )
.map_err(|e| { .map_err(|e| {

16
vmm/src/vm.rs
View File

@ -55,7 +55,7 @@ use gdbstub_arch::aarch64::reg::AArch64CoreRegs as CoreRegs;
#[cfg(all(target_arch = "x86_64", feature = "guest_debug"))] #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))]
use gdbstub_arch::x86::reg::X86_64CoreRegs as CoreRegs; use gdbstub_arch::x86::reg::X86_64CoreRegs as CoreRegs;
use hypervisor::{HypervisorVmError, VmOps}; use hypervisor::{HypervisorVmError, VmOps};
use libc::SIGWINCH; use libc::{termios, SIGWINCH};
use linux_loader::cmdline::Cmdline; use linux_loader::cmdline::Cmdline;
#[cfg(all(target_arch = "x86_64", feature = "guest_debug"))] #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))]
use linux_loader::elf; use linux_loader::elf;
@ -79,7 +79,6 @@ use std::mem::size_of;
use std::num::Wrapping; use std::num::Wrapping;
use std::ops::Deref; use std::ops::Deref;
use std::os::unix::net::UnixStream; use std::os::unix::net::UnixStream;
use std::sync::atomic::AtomicBool;
use std::sync::{Arc, Mutex, RwLock}; use std::sync::{Arc, Mutex, RwLock};
use std::time::Instant; use std::time::Instant;
use std::{result, str, thread}; use std::{result, str, thread};
@ -464,7 +463,7 @@ impl Vm {
serial_pty: Option<PtyPair>, serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>, console_pty: Option<PtyPair>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<File>,
on_tty: Arc<AtomicBool>, original_termios: Arc<Mutex<Option<termios>>>,
snapshot: Option<Snapshot>, snapshot: Option<Snapshot>,
) -> Result<Self> { ) -> Result<Self> {
trace_scoped!("Vm::new_from_memory_manager"); trace_scoped!("Vm::new_from_memory_manager");
@ -586,7 +585,12 @@ impl Vm {
device_manager device_manager
.lock() .lock()
.unwrap() .unwrap()
.create_devices(serial_pty, console_pty, console_resize_pipe, on_tty) .create_devices(
serial_pty,
console_pty,
console_resize_pipe,
original_termios,
)
.map_err(Error::DeviceManager)?; .map_err(Error::DeviceManager)?;
#[cfg(feature = "tdx")] #[cfg(feature = "tdx")]
@ -736,7 +740,7 @@ impl Vm {
serial_pty: Option<PtyPair>, serial_pty: Option<PtyPair>,
console_pty: Option<PtyPair>, console_pty: Option<PtyPair>,
console_resize_pipe: Option<File>, console_resize_pipe: Option<File>,
on_tty: Arc<AtomicBool>, original_termios: Arc<Mutex<Option<termios>>>,
snapshot: Option<Snapshot>, snapshot: Option<Snapshot>,
source_url: Option<&str>, source_url: Option<&str>,
prefault: Option<bool>, prefault: Option<bool>,
@ -806,7 +810,7 @@ impl Vm {
serial_pty, serial_pty,
console_pty, console_pty,
console_resize_pipe, console_resize_pipe,
on_tty, original_termios,
snapshot, snapshot,
) )
} }