From 532a94c1c83c1f829a03e5c95ae584a57c0e86d5 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 21 Feb 2018 10:21:56 -0800 Subject: [PATCH] Fix race between un-pausing vcpu and requesting vcpu pause To ensure that we do not miss pause request sent while we were in paused state, or were exiting paused state, let's start using KVM_SET_SIGNAL_MASK. SIGRTMIN() + 0 signal will be blocked and thus is not delivered, and it will only be checked when KVM_RUN is being executed, reliably interrupting KVM_RUN. TEST=cargo test --features plugin; cargo test -p kvm; ./build_test BUG=chromium:800626 Change-Id: Iae67a411c23c2b14fbfcbc7d53d0bc86ec4b67d9 Signed-off-by: Dmitry Torokhov Reviewed-on: https://chromium-review.googlesource.com/944850 Reviewed-by: Zach Reizner --- src/plugin/mod.rs | 18 +++++++++++++++++- tests/plugin_vcpu_pause.c | 15 +++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 853b7e58aa..4528df89cc 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -26,7 +26,7 @@ use io_jail::{self, Minijail}; use kvm::{Kvm, Vm, Vcpu, VcpuExit, IoeventAddress, NoDatamatch}; use sys_util::{EventFd, MmapError, Killable, SignalFd, SignalFdError, Poller, Pollable, GuestMemory, Result as SysResult, Error as SysError, - register_signal_handler, SIGRTMIN, + register_signal_handler, block_signal, clear_signal, SIGRTMIN, geteuid, getegid}; use Config; @@ -307,10 +307,21 @@ pub fn run_vcpus(kvm: &Kvm, unsafe { extern "C" fn handle_signal() {} // Our signal handler does nothing and is trivially async signal safe. + // We need to install this signal handler even though we do block + // the signal below, to ensure that this signal will interrupt + // execution of KVM_RUN (this is implementation issue). register_signal_handler(SIGRTMIN() + 0, handle_signal) .expect("failed to register vcpu signal handler"); } + // We do not really want the signal handler to run... + block_signal(SIGRTMIN() + 0) + .expect("failed to block signal"); + // Tell KVM to not block anything when entering kvm run + // because we will be using first RT signal to kick the VCPU. + vcpu.set_signal_mask(&[]) + .expect("failed to set up KVM VCPU signal mask"); + let res = vcpu_plugin.init(&vcpu); vcpu_thread_barrier.wait(); if let Err(e) = res { @@ -356,6 +367,11 @@ pub fn run_vcpus(kvm: &Kvm, break; } + // Try to clear the signal that we use to kick VCPU if it is + // pending before attempting to handle pause requests. + clear_signal(SIGRTMIN() + 0) + .expect("failed to clear pending signal"); + if let Err(e) = vcpu_plugin.pre_run(&vcpu) { error!("failed to process pause on vcpu {}: {:?}", cpu_id, e); break; diff --git a/tests/plugin_vcpu_pause.c b/tests/plugin_vcpu_pause.c index a9a27f8fa1..ff69b04802 100644 --- a/tests/plugin_vcpu_pause.c +++ b/tests/plugin_vcpu_pause.c @@ -238,6 +238,21 @@ int main(int argc, char** argv) { return 1; } + signal_unpause(crosvm, false); + + /* Wait until VCPU thread tells us that it is no longer paused */ + read(g_kill_evt, &dummy, sizeof(dummy)); + + /* + * Try pausing VCPUs 3rd time to see if we will miss pause + * request as we are exiting previous pause. + */ + ret = signal_pause(crosvm); + if (ret) { + fprintf(stderr, "failed to pause vcpus (2nd time): %d\n", ret); + return 1; + } + signal_unpause(crosvm, true); /* Wait until VCPU thread tells us that it is no longer paused */