commit 9cbe543e6a77d08e8b49e87ecf3331d2b4f12ff2
author Dmitry Torokhov <dtor@chromium.org> Tue Jan 25 19:17:07 2022 -0800
committer Commit Bot <commit-bot@chromium.org> Thu Jan 27 00:06:10 2022 +0000
tree 923f6fa1085d9af44555528879f9a3329e4ef23e
parent cbfef3b3e37517f2f5a49b75721745224d2acc55

devices: gpu: control lifetime of render server jail

Rearrange the code that creates devices and starts the VM so that the
jail object is not destroyed while VM is running and also make sure we
kill the process when vm stops.

BUG=b:177267762
TEST=./tools/presubmit

Change-Id: Id35791e7264a52e34d2ddfac86425ad4b3c2d059
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3416101
Reviewed-by: Chia-I Wu <olv@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Dmitry Torokhov <dtor@chromium.org>

src/linux.rs

diff --git a/src/linux.rs b/src/linux.rs
index a8fca2f..68643c5 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -993,10 +993,20 @@
 }
 
 #[cfg(feature = "gpu")]
+struct ScopedMinijail(Minijail);
+
+#[cfg(feature = "gpu")]
+impl Drop for ScopedMinijail {
+    fn drop(&mut self) {
+        let _ = self.0.kill();
+    }
+}
+
+#[cfg(feature = "gpu")]
 fn start_gpu_render_server(
     cfg: &Config,
     render_server_parameters: &GpuRenderServerParameters,
-) -> Result<SafeDescriptor> {
+) -> Result<(Minijail, SafeDescriptor)> {
     let (server_socket, client_socket) =
         UnixSeqpacket::pair().context("failed to create render server socket")?;
 
@@ -1058,7 +1068,7 @@
     )?)
     .context("failed to start gpu render server")?;
 
-    Ok(SafeDescriptor::from(client_socket))
+    Ok((jail, SafeDescriptor::from(client_socket)))
 }
 
 fn create_wayland_device(
@@ -1503,6 +1513,7 @@
     pmem_device_tubes: &mut Vec<Tube>,
     map_request: Arc<Mutex<Option<ExternalMapping>>>,
     fs_device_tubes: &mut Vec<Tube>,
+    #[cfg(feature = "gpu")] render_server_fd: Option<SafeDescriptor>,
 ) -> DeviceResult<Vec<VirtioDeviceStub>> {
     let mut devs = Vec::new();
 
@@ -1735,11 +1746,6 @@
                 event_devices.push(EventDevice::keyboard(event_device_socket));
             }
 
-            let mut render_server_fd = None;
-            if let Some(ref render_server_parameters) = gpu_parameters.render_server {
-                render_server_fd = Some(start_gpu_render_server(cfg, render_server_parameters)?);
-            }
-
             devs.push(create_gpu_device(
                 cfg,
                 _exit_evt,
@@ -1958,6 +1964,7 @@
     fs_device_tubes: &mut Vec<Tube>,
     #[cfg(feature = "usb")] usb_provider: HostBackendDeviceProvider,
     map_request: Arc<Mutex<Option<ExternalMapping>>>,
+    #[cfg(feature = "gpu")] render_server_fd: Option<SafeDescriptor>,
 ) -> DeviceResult<Vec<(Box<dyn BusDeviceObj>, Option<Minijail>)>> {
     let mut devices: Vec<(Box<dyn BusDeviceObj>, Option<Minijail>)> = Vec::new();
     let mut balloon_inflate_tube: Option<Tube> = None;
@@ -2085,6 +2092,8 @@
         pmem_device_tubes,
         map_request,
         fs_device_tubes,
+        #[cfg(feature = "gpu")]
+        render_server_fd,
     )?;
 
     for stub in stubs {
@@ -3027,6 +3036,21 @@
     create_file_backed_mappings(&cfg, &mut vm, &mut sys_allocator)?;
 
     let phys_max_addr = (1u64 << vm.get_guest_phys_addr_bits()) - 1;
+
+    #[cfg(feature = "gpu")]
+    // Hold on to the render server jail so it keeps running until we exit run_vm()
+    let mut _render_server_jail = None;
+    #[cfg(feature = "gpu")]
+    let mut render_server_fd = None;
+    #[cfg(feature = "gpu")]
+    if let Some(gpu_parameters) = &cfg.gpu_parameters {
+        if let Some(ref render_server_parameters) = gpu_parameters.render_server {
+            let (jail, fd) = start_gpu_render_server(&cfg, render_server_parameters)?;
+            _render_server_jail = Some(ScopedMinijail(jail));
+            render_server_fd = Some(fd);
+        }
+    }
+
     let mut devices = create_devices(
         &cfg,
         &mut vm,
@@ -3044,6 +3068,8 @@
         #[cfg(feature = "usb")]
         usb_provider,
         Arc::clone(&map_request),
+        #[cfg(feature = "gpu")]
+        render_server_fd,
     )?;
 
     #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]