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 <dtor@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/944850
Reviewed-by: Zach Reizner <zachr@chromium.org>

2 files changed, 32 insertions, 1 deletions

diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs
index 853b7e5..4528df8 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 a9a27f8..ff69b04 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 */