From 732231efb0f9f0a2215b525d19d11ba66356cbc7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Sun, 29 Sep 2019 06:06:25 +0200
Subject: [PATCH] vm: refuse to start a VM not present in a collection

Check early (but after grabbing a startup_lock) if VM isn't just
removed. This could happen if someone grabs its reference from other
places (netvm of something else?) or just before removing it.
This commit makes the simple removal from the collection (done as the
first step in admin.vm.Remove implementation) efficient way to block
further VM startups, without introducing extra properties.

For this to be effective, removing from the collection, needs to happen
with the startup_lock held. Modify admin.vm.Remove accordingly.
---
 qubes/api/admin.py  | 23 ++++++++++++-----------
 qubes/vm/qubesvm.py |  3 +++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/qubes/api/admin.py b/qubes/api/admin.py
index 5a6f31e5..ea6e5b0e 100644
--- a/qubes/api/admin.py
+++ b/qubes/api/admin.py
@@ -1130,19 +1130,20 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
 
         self.fire_event_for_permission()
 
-        if not self.dest.is_halted():
-            raise qubes.exc.QubesVMNotHaltedError(self.dest)
+        with (yield from self.dest.startup_lock):
+            if not self.dest.is_halted():
+                raise qubes.exc.QubesVMNotHaltedError(self.dest)
 
-        if self.dest.installed_by_rpm:
-            raise qubes.exc.QubesVMInUseError(self.dest, \
-                "VM installed by package manager: " + self.dest.name)
+            if self.dest.installed_by_rpm:
+                raise qubes.exc.QubesVMInUseError(self.dest,
+                    "VM installed by package manager: " + self.dest.name)
 
-        del self.app.domains[self.dest]
-        try:
-            yield from self.dest.remove_from_disk()
-        except:  # pylint: disable=bare-except
-            self.app.log.exception('Error while removing VM \'%s\' files',
-                self.dest.name)
+            del self.app.domains[self.dest]
+            try:
+                yield from self.dest.remove_from_disk()
+            except:  # pylint: disable=bare-except
+                self.app.log.exception('Error while removing VM \'%s\' files',
+                    self.dest.name)
 
         self.app.save()
 
diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py
index e1df0dc8..a42cc032 100644
--- a/qubes/vm/qubesvm.py
+++ b/qubes/vm/qubesvm.py
@@ -1008,6 +1008,9 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         '''
 
         with (yield from self.startup_lock):
+            # check if domain wasn't removed in the meantime
+            if self not in self.app.domains:
+                raise qubes.exc.QubesVMNotFoundError(self.name)
             # Intentionally not used is_running(): eliminate also "Paused",
             # "Crashed", "Halting"
             if self.get_power_state() != 'Halted':