From d12f920b3fe287fe3d0805851a8fa70d57c11458 Mon Sep 17 00:00:00 2001
From: "James O. D. Hunt" <james.o.hunt@intel.com>
Date: Fri, 21 Aug 2020 11:40:19 +0100
Subject: [PATCH] console: Fix crash if debug console disabled

The logic for the debug console meant that if the debug console was
_disabled_, the agent was guaranteed to crash on function exit due to
the unsafe code block. Fixed by simplifying the code to use the standard
`Option` idiom for optional values.

Fixes: #554.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
---
 src/agent/src/main.rs | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs
index c124dfa5b9..91c9409d1d 100644
--- a/src/agent/src/main.rs
+++ b/src/agent/src/main.rs
@@ -47,7 +47,7 @@ use std::os::unix::io::AsRawFd;
 use std::path::Path;
 use std::sync::mpsc::{self, Sender};
 use std::sync::{Arc, Mutex, RwLock};
-use std::{io, thread};
+use std::{io, thread, thread::JoinHandle};
 use unistd::Pid;
 
 mod config;
@@ -84,8 +84,6 @@ lazy_static! {
         Arc::new(RwLock::new(config::agentConfig::new()));
 }
 
-use std::mem::MaybeUninit;
-
 fn announce(logger: &Logger, config: &agentConfig) {
     let commit = match env::var("VERSION_COMMIT") {
         Ok(s) => s,
@@ -195,32 +193,37 @@ fn main() -> Result<()> {
     // which is required to satisfy the the lifetime constraints of the auto-generated gRPC code.
     let _guard = slog_scope::set_global_logger(logger.new(o!("subsystem" => "rpc")));
 
-    start_sandbox(&logger, &config)?;
+    start_sandbox(&logger, &config, init_mode)?;
 
     let _ = log_handle.join();
 
     Ok(())
 }
 
-fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> {
+fn start_sandbox(logger: &Logger, config: &agentConfig, init_mode: bool) -> Result<()> {
     let shells = SHELLS.clone();
     let debug_console_vport = config.debug_console_vport as u32;
 
-    let shell_handle = if config.debug_console {
+    let mut shell_handle: Option<JoinHandle<()>> = None;
+    if config.debug_console {
         let thread_logger = logger.clone();
 
-        thread::spawn(move || {
-            let shells = shells.lock().unwrap();
-            let result = setup_debug_console(shells.to_vec(), debug_console_vport);
-            if result.is_err() {
-                // Report error, but don't fail
-                warn!(thread_logger, "failed to setup debug console";
+        let builder = thread::Builder::new();
+
+        let handle = builder
+            .spawn(move || {
+                let shells = shells.lock().unwrap();
+                let result = setup_debug_console(shells.to_vec(), debug_console_vport);
+                if result.is_err() {
+                    // Report error, but don't fail
+                    warn!(thread_logger, "failed to setup debug console";
                     "error" => format!("{}", result.unwrap_err()));
-            }
-        })
-    } else {
-        unsafe { MaybeUninit::zeroed().assume_init() }
-    };
+                }
+            })
+            .map_err(|e| format!("{:?}", e))?;
+
+        shell_handle = Some(handle);
+    }
 
     // Initialize unique sandbox structure.
     let mut s = Sandbox::new(&logger).map_err(|e| {
@@ -252,8 +255,8 @@ fn start_sandbox(logger: &Logger, config: &agentConfig) -> Result<()> {
 
     server.shutdown();
 
-    if config.debug_console {
-        shell_handle.join().unwrap();
+    if let Some(handle) = shell_handle {
+        handle.join().map_err(|e| format!("{:?}", e))?;
     }
 
     Ok(())