ProcessManager: cleanup interfaces

- kata-proc-common =>'s kata-proc-interface
- use explicit import's
- add kata_proc_* & kata_pkg_* stub wrappers to hide C calls
- hide RAW_BUNDLE_ID_DATA_SIZE, not needed after stub additions
- add From traits to cleanup return handling
- narrow unsafe blocks

Note: removed some less-useful trace calls to enable ?-op usage

Change-Id: Iefe704654f5d286648c94c13a88573baaa434ecb
GitOrigin-RevId: 1639da7ea59653c1d1d22a9c1dab9c05aaf409df

apps/system/components/DebugConsole/kata-shell/Cargo.toml

@@ -6,13 +6,11 @@ edition = "2018"
 
 [dependencies]
 crc = { version = "1.4.0", default_features = false }
-cstr_core = "0.2"
 hex = { version = "0.4.3", default-features = false, features = ["alloc"] }
 kata-io = { path = "../kata-io" }
 kata-line-reader = { path = "../kata-line-reader" }
-kata-proc-common = { path = "../../ProcessManager/kata-proc-common" }
+kata-proc-interface = { path = "../../ProcessManager/kata-proc-interface" }
 kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 kata-storage-interface = { path = "../../StorageManager/kata-storage-interface" }
 log = "0.4"
-postcard = { version = "0.7", features = ["alloc"], default-features = false }
 zmodem = { path = "../zmodem" }

apps/system/components/DebugConsole/kata-shell/src/lib.rs

@@ -5,13 +5,16 @@ use alloc::string::String;
 use alloc::vec::Vec;
 use core::fmt;
 use core::fmt::Write;
-use cstr_core::CString;
 use hex;
-use postcard;
+use log;
 
 use kata_io as io;
 use kata_line_reader::LineReader;
-use kata_proc_common::{BundleIdArray, ProcessManagerError, RAW_BUNDLE_ID_DATA_SIZE};
+use kata_proc_interface::kata_pkg_mgmt_install;
+use kata_proc_interface::kata_pkg_mgmt_uninstall;
+use kata_proc_interface::kata_proc_ctrl_get_running_bundles;
+use kata_proc_interface::kata_proc_ctrl_start;
+use kata_proc_interface::kata_proc_ctrl_stop;
 use kata_storage_interface::kata_storage_delete;
 use kata_storage_interface::kata_storage_read;
 use kata_storage_interface::kata_storage_write;
@@ -240,28 +243,11 @@ fn clear_command(output: &mut dyn io::Write) -> Result<(), CommandError> {
 }
 
 fn bundles_command(output: &mut dyn io::Write) -> Result<(), CommandError> {
-    extern "C" {
-        fn proc_ctrl_get_running_bundles(c_raw_data: *mut u8) -> ProcessManagerError;
-    }
-    let mut raw_data = [0u8; RAW_BUNDLE_ID_DATA_SIZE];
-    match unsafe { proc_ctrl_get_running_bundles(raw_data.as_mut_ptr()) } {
-        ProcessManagerError::Success => {
-            match postcard::from_bytes::<BundleIdArray>(raw_data.as_ref()) {
+    match kata_proc_ctrl_get_running_bundles() {
         Ok(bundle_ids) => {
-                    for bundle_id in bundle_ids {
-                        writeln!(output, "{}", bundle_id)?;
+            writeln!(output, "{}", bundle_ids.join("\n"))?;
         }
-                }
-                Err(e) => {
-                    writeln!(
-                        output,
-                        "get_running_bundles failed: deserialize returned {:?}",
-                        e
-                    )?;
-                }
-            }
-        }
-        status => {
+        Err(status) => {
             writeln!(output, "get_running_bundles failed: {:?}", status)?;
         }
     }
@@ -272,27 +258,13 @@ fn install_command(
     _args: &mut dyn Iterator<Item = &str>,
     output: &mut dyn io::Write,
 ) -> Result<(), CommandError> {
-    extern "C" {
-        fn pkg_mgmt_install(
-            c_pkg_buffer_size: usize,
-            c_pkg_buffer: *const u8,
-            c_raw_data: *mut u8,
-        ) -> ProcessManagerError;
-    }
     // TODO(sleffler): supply a real bundle (e.g. from serial)
-    let pkg_buffer = [0u8; 64]; // NB: limited by 120 byte ipc buffer
-    let mut raw_data = [0u8; RAW_BUNDLE_ID_DATA_SIZE];
-    match unsafe { pkg_mgmt_install(pkg_buffer.len(), pkg_buffer.as_ptr(), raw_data.as_mut_ptr()) }
-    {
-        ProcessManagerError::Success => match postcard::from_bytes::<String>(raw_data.as_ref()) {
+    let pkg_buffer = &[0u8; 64];
+    match kata_pkg_mgmt_install(pkg_buffer) {
         Ok(bundle_id) => {
             writeln!(output, "Bundle \"{}\" installed", bundle_id)?;
         }
-            Err(e) => {
-                writeln!(output, "install failed: deserialize returned {:?}", e)?;
-            }
-        },
-        status => {
+        Err(status) => {
             writeln!(output, "install failed: {:?}", status)?;
         }
     }
@@ -303,16 +275,12 @@ fn uninstall_command(
     args: &mut dyn Iterator<Item = &str>,
     output: &mut dyn io::Write,
 ) -> Result<(), CommandError> {
-    extern "C" {
-        fn pkg_mgmt_uninstall(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError;
-    }
     if let Some(bundle_id) = args.nth(0) {
-        let cstr = CString::new(bundle_id).unwrap();
-        match unsafe { pkg_mgmt_uninstall(cstr.as_ptr()) } {
-            ProcessManagerError::Success => {
+        match kata_pkg_mgmt_uninstall(bundle_id) {
+            Ok(_) => {
                 writeln!(output, "Bundle \"{}\" uninstalled.", bundle_id)?;
             }
-            status => {
+            Err(status) => {
                 writeln!(output, "uninstall failed: {:?}", status)?;
             }
         }
@@ -326,16 +294,12 @@ fn start_command(
     args: &mut dyn Iterator<Item = &str>,
     output: &mut dyn io::Write,
 ) -> Result<(), CommandError> {
-    extern "C" {
-        fn proc_ctrl_start(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError;
-    }
     if let Some(bundle_id) = args.nth(0) {
-        let cstr = CString::new(bundle_id).unwrap();
-        match unsafe { proc_ctrl_start(cstr.as_ptr()) } {
-            ProcessManagerError::Success => {
+        match kata_proc_ctrl_start(bundle_id) {
+            Ok(_) => {
                 writeln!(output, "Bundle \"{}\" started.", bundle_id)?;
             }
-            status => {
+            Err(status) => {
                 writeln!(output, "start failed: {:?}", status)?;
             }
         }
@@ -349,16 +313,12 @@ fn stop_command(
     args: &mut dyn Iterator<Item = &str>,
     output: &mut dyn io::Write,
 ) -> Result<(), CommandError> {
-    extern "C" {
-        fn proc_ctrl_stop(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError;
-    }
     if let Some(bundle_id) = args.nth(0) {
-        let cstr = CString::new(bundle_id).unwrap();
-        match unsafe { proc_ctrl_stop(cstr.as_ptr()) } {
-            ProcessManagerError::Success => {
+        match kata_proc_ctrl_stop(bundle_id) {
+            Ok(_) => {
                 writeln!(output, "Bundle \"{}\" stopped.", bundle_id)?;
             }
-            status => {
+            Err(status) => {
                 writeln!(output, "stop failed: {:?}", status)?;
             }
         }

apps/system/components/ProcessManager/Cargo.toml

@@ -1,8 +1,8 @@
 [workspace]
 
 members = [
-    "kata-proc-common",
     "kata-proc-component",
+    "kata-proc-interface",
     "kata-proc-manager",
 ]
 

apps/system/components/ProcessManager/kata-proc-component/Cargo.toml

@@ -8,7 +8,7 @@ cstr_core = { version = "0.2.3", default-features = false }
 kata-allocator = { path = "../../DebugConsole/kata-allocator" }
 kata-logger = { path = "../../DebugConsole/kata-logger" }
 kata-panic = { path = "../../DebugConsole/kata-panic" }
-kata-proc-common = { path = "../kata-proc-common" }
+kata-proc-interface = { path = "../kata-proc-interface" }
 kata-proc-manager = { path = "../kata-proc-manager" }
 log = "0.4"
 postcard = { version = "0.7", features = ["alloc"], default-features = false }

apps/system/components/ProcessManager/kata-proc-component/src/run.rs

@@ -7,7 +7,7 @@ use cstr_core::CStr;
 use kata_allocator;
 use kata_logger::KataLogger;
 extern crate kata_panic;
-use kata_proc_common::*;
+use kata_proc_interface::*;
 use kata_proc_manager::KATA_PROC;
 use log::trace;
 use postcard;
@@ -55,85 +55,63 @@ pub extern "C" fn pkg_mgmt_install(
     c_pkg_buffer: *const u8,
     c_raw_data: *mut RawBundleIdData,
 ) -> ProcessManagerError {
-    unsafe {
-        match KATA_PROC.install(c_pkg_buffer, c_pkg_buffer_sz) {
-            Ok(bundle_id) => match postcard::to_slice(&bundle_id, &mut (*c_raw_data)[..]) {
+    match unsafe { KATA_PROC.install(c_pkg_buffer, c_pkg_buffer_sz) } {
+        Ok(bundle_id) => match unsafe { postcard::to_slice(&bundle_id, &mut (*c_raw_data)[..]) } {
             Ok(_) => ProcessManagerError::Success,
-                Err(e) => {
-                    trace!("install failed: serialize {:?}", e);
-                    ProcessManagerError::BundleDataInvalid
-                }
+            Err(_) => ProcessManagerError::DeserializeError,
         },
-            Err(status) => {
-                trace!("install failed: {:?}", status);
-                status
-            }
-        }
+        Err(e) => e,
     }
 }
 
 #[no_mangle]
 pub extern "C" fn pkg_mgmt_uninstall(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError {
-    unsafe {
-        match CStr::from_ptr(c_bundle_id).to_str() {
-            Ok(bundle_id) => match KATA_PROC.uninstall(bundle_id) {
+    match unsafe { CStr::from_ptr(c_bundle_id).to_str() } {
+        Ok(bundle_id) => match unsafe { KATA_PROC.uninstall(bundle_id) } {
             Ok(_) => ProcessManagerError::Success,
             Err(e) => e,
         },
         Err(_) => ProcessManagerError::BundleIdInvalid,
     }
-    }
 }
 
 // ProcessControlInterface glue stubs.
 #[no_mangle]
 pub extern "C" fn proc_ctrl_start(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError {
-    unsafe {
-        match CStr::from_ptr(c_bundle_id).to_str() {
-            Ok(bundle_id) => match KATA_PROC.start(bundle_id) {
+    match unsafe { CStr::from_ptr(c_bundle_id).to_str() } {
+        Ok(bundle_id) => match unsafe { KATA_PROC.start(bundle_id) } {
             Ok(_) => ProcessManagerError::Success,
             Err(e) => e,
         },
         Err(_) => ProcessManagerError::BundleIdInvalid,
     }
-    }
 }
 
 #[no_mangle]
-pub extern "C" fn proc_ctrl_stop(bundle_id: *const cstr_core::c_char) -> ProcessManagerError {
-    unsafe {
-        match CStr::from_ptr(bundle_id).to_str() {
-            Ok(str) => match KATA_PROC.stop(str) {
+pub extern "C" fn proc_ctrl_stop(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError {
+    match unsafe { CStr::from_ptr(c_bundle_id).to_str() } {
+        Ok(str) => match unsafe { KATA_PROC.stop(str) } {
             Ok(_) => ProcessManagerError::Success,
             Err(e) => e,
         },
         Err(_) => ProcessManagerError::BundleIdInvalid,
     }
-    }
 }
 
 #[no_mangle]
 pub extern "C" fn proc_ctrl_get_running_bundles(
     c_raw_data: *mut RawBundleIdData,
 ) -> ProcessManagerError {
-    unsafe {
-        match KATA_PROC.get_running_bundles() {
+    match unsafe { KATA_PROC.get_running_bundles() } {
         Ok(bundles) => {
             // Serialize the bundle_id's in the result buffer. If we
-                // overflow the buffer, BundleDataInvalid is returned and
-                // the contents are undefined (postcard does not specify).
-                match postcard::to_slice(&bundles, &mut (*c_raw_data)[..]) {
+            // overflow the buffer, an error is returned and the
+            // contents are undefined (postcard does not specify).
+            match unsafe { postcard::to_slice(&bundles, &mut (*c_raw_data)[..]) } {
                 Ok(_) => ProcessManagerError::Success,
-                    Err(e) => {
-                        trace!("get_running_bundles failed: serialize {:?}", e);
-                        ProcessManagerError::BundleDataInvalid
-                    }
-                }
-            }
-            Err(status) => {
-                trace!("get_running_bundles failed: {:?}", status);
-                status
+                Err(_) => ProcessManagerError::DeserializeError,
             }
         }
+        Err(e) => e,
     }
 }

apps/system/components/ProcessManager/kata-proc-interface/Cargo.toml

@@ -1,9 +1,10 @@
 [package]
-name = "kata-proc-common"
+name = "kata-proc-interface"
 version = "0.1.0"
 edition = "2018"
 
 [dependencies]
+cstr_core = { version = "0.2.3", default-features = false }
 kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 postcard = { version = "0.7", features = ["alloc"], default-features = false }
 serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] }

apps/system/components/ProcessManager/kata-proc-interface/src/lib.rs

@@ -6,6 +6,8 @@ extern crate alloc;
 use alloc::string::String;
 use alloc::vec::Vec;
 use core::str;
+use kata_security_interface::SecurityRequestError;
+use postcard;
 use serde::{Deserialize, Serialize};
 
 pub type BundleIdArray = Vec<String>;
@@ -73,12 +75,13 @@ impl Bundle {
 pub enum ProcessManagerError {
     Success = 0,
     BundleIdInvalid,
-    BundleDataInvalid,
     PackageBufferLenInvalid,
     BundleNotFound,
     BundleFound,
     BundleRunning,
-    NoSpace,
+    UnknownError,
+    DeserializeError,
+    SerializeError,
     // Generic errors, mostly for unit tests.
     InstallFailed,
     UninstallFailed,
@@ -120,6 +123,109 @@ pub trait ProcessControlInterface {
     fn get_running_bundles(&self) -> Result<BundleIdArray, ProcessManagerError>;
 }
 
+impl From<postcard::Error> for ProcessManagerError {
+    fn from(err: postcard::Error) -> ProcessManagerError {
+        match err {
+            postcard::Error::SerializeBufferFull
+            | postcard::Error::SerializeSeqLengthUnknown
+            | postcard::Error::SerdeSerCustom => ProcessManagerError::SerializeError,
+            // NB: bit of a cheat; this lumps in *Implement*
+            _ => ProcessManagerError::DeserializeError,
+        }
+    }
+}
+
+impl From<SecurityRequestError> for ProcessManagerError {
+    fn from(err: SecurityRequestError) -> ProcessManagerError {
+        match err {
+            SecurityRequestError::SreSuccess => ProcessManagerError::Success,
+            SecurityRequestError::SreBundleIdInvalid => ProcessManagerError::BundleIdInvalid,
+            SecurityRequestError::SreBundleNotFound => ProcessManagerError::BundleNotFound,
+            SecurityRequestError::SrePackageBufferLenInvalid => {
+                ProcessManagerError::PackageBufferLenInvalid
+            }
+            SecurityRequestError::SreInstallFailed => ProcessManagerError::InstallFailed,
+            SecurityRequestError::SreUninstallFailed => ProcessManagerError::UninstallFailed,
+            // NB: other errors "cannot happen" so just return something unique
+            _ => ProcessManagerError::UnknownError,
+        }
+    }
+}
+
+impl From<ProcessManagerError> for Result<(), ProcessManagerError> {
+    fn from(err: ProcessManagerError) -> Result<(), ProcessManagerError> {
+        if err == ProcessManagerError::Success {
+            Ok(())
+        } else {
+            Err(err)
+        }
+    }
+}
+
+#[inline]
+#[allow(dead_code)]
+pub fn kata_proc_ctrl_get_running_bundles() -> Result<BundleIdArray, ProcessManagerError> {
+    extern "C" {
+        fn proc_ctrl_get_running_bundles(c_raw_data: *mut u8) -> ProcessManagerError;
+    }
+    let raw_data = &mut [0u8; RAW_BUNDLE_ID_DATA_SIZE];
+    match unsafe { proc_ctrl_get_running_bundles(raw_data as *mut _) } {
+        ProcessManagerError::Success => {
+            let bids = postcard::from_bytes::<BundleIdArray>(raw_data)?;
+            Ok(bids)
+        }
+        status => Err(status),
+    }
+}
+
+#[inline]
+#[allow(dead_code)]
+pub fn kata_pkg_mgmt_install(pkg_buffer: &[u8]) -> Result<String, ProcessManagerError> {
+    extern "C" {
+        fn pkg_mgmt_install(
+            c_pkg_buffer_size: usize,
+            c_pkg_buffer: *const u8,
+            c_raw_data: *mut u8,
+        ) -> ProcessManagerError;
+    }
+    let raw_data = &mut [0u8; RAW_BUNDLE_ID_DATA_SIZE];
+    match unsafe { pkg_mgmt_install(pkg_buffer.len(), pkg_buffer.as_ptr(), raw_data as *mut _) } {
+        ProcessManagerError::Success => {
+            let bundle_id = postcard::from_bytes::<String>(raw_data.as_ref())?;
+            Ok(bundle_id)
+        }
+        status => Err(status),
+    }
+}
+
+#[inline]
+#[allow(dead_code)]
+pub fn kata_pkg_mgmt_uninstall(bundle_id: &str) -> Result<(), ProcessManagerError> {
+    extern "C" {
+        fn pkg_mgmt_uninstall(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError;
+    }
+    unsafe { pkg_mgmt_uninstall(bundle_id.as_ptr()) }.into()
+}
+
+#[inline]
+#[allow(dead_code)]
+pub fn kata_proc_ctrl_start(bundle_id: &str) -> Result<(), ProcessManagerError> {
+    extern "C" {
+        fn proc_ctrl_start(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError;
+    }
+    unsafe { proc_ctrl_start(bundle_id.as_ptr()) }.into()
+}
+
+#[inline]
+#[allow(dead_code)]
+pub fn kata_proc_ctrl_stop(bundle_id: &str) -> Result<(), ProcessManagerError> {
+    extern "C" {
+        fn proc_ctrl_stop(c_bundle_id: *const cstr_core::c_char) -> ProcessManagerError;
+    }
+    unsafe { proc_ctrl_stop(bundle_id.as_ptr()) }.into()
+}
+
+// TODO(sleffler): move out of interface?
 #[cfg(test)]
 mod tests {
     use super::*;

apps/system/components/ProcessManager/kata-proc-manager/Cargo.toml

@@ -6,7 +6,7 @@ edition = "2018"
 
 [dependencies]
 hashbrown = { version = "0.11", features = ["ahash-compile-time-rng"] }
-kata-proc-common = { path = "../kata-proc-common" }
+kata-proc-interface = { path = "../kata-proc-interface" }
 kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 log = "0.4"
 postcard = { version = "0.7", features = ["alloc"], default-features = false }

apps/system/components/ProcessManager/kata-proc-manager/src/lib.rs

@@ -4,13 +4,17 @@
 
 extern crate alloc;
 use alloc::string::String;
-use kata_proc_common::*;
+use kata_proc_interface::Bundle;
+use kata_proc_interface::BundleIdArray;
+use kata_proc_interface::PackageManagementInterface;
+use kata_proc_interface::ProcessControlInterface;
+use kata_proc_interface::ProcessManagerError;
+use kata_proc_interface::ProcessManagerInterface;
 use kata_security_interface::kata_security_request;
 use kata_security_interface::InstallRequest;
 use kata_security_interface::SecurityRequest;
 use kata_security_interface::UninstallRequest;
 use kata_security_interface::SECURITY_REPLY_DATA_SIZE;
-use log::trace;
 use postcard;
 use spin::Mutex;
 
@@ -93,26 +97,16 @@ impl ProcessManagerInterface for KataManagerInterface {
 
         // This is handled by the SecurityCoordinator.
         let reply = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        match kata_security_request(
+        kata_security_request(
             SecurityRequest::SrInstall,
             &InstallRequest {
                 pkg_buffer_size: pkg_buffer_size,
                 pkg_buffer: pkg_buffer,
             },
             reply,
-        ) {
-            Ok(_) => {
-                fn deserialize_failure(e: postcard::Error) -> ProcessManagerError {
-                    trace!("install failed: deserialize {:?}", e);
-                    ProcessManagerError::BundleDataInvalid
-                }
-                postcard::from_bytes::<String>(reply).map_err(deserialize_failure)
-            }
-            Err(status) => {
-                trace!("install failed: {:?}", status);
-                Err(ProcessManagerError::InstallFailed)
-            }
-        }
+        )?;
+        let bundle_id = postcard::from_bytes::<String>(reply)?;
+        Ok(bundle_id)
     }
     fn uninstall(&mut self, bundle_id: &str) -> Result<(), ProcessManagerError> {
         // NB: the caller has already checked no running application exists
@@ -120,19 +114,14 @@ impl ProcessManagerInterface for KataManagerInterface {
 
         // This is handled by the SecurityCoordinator.
         let reply = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        match kata_security_request(
+        kata_security_request(
             SecurityRequest::SrUninstall,
             &UninstallRequest {
                 bundle_id: &bundle_id,
             },
             reply,
-        ) {
-            Ok(_) => Ok(()),
-            Err(status) => {
-                trace!("uninstall failed: {:?}", status);
-                Err(ProcessManagerError::UninstallFailed)
-            }
-        }
+        )?;
+        Ok(())
     }
     fn start(&mut self, _bundle: &Bundle) -> Result<(), ProcessManagerError> {
         // 1. Ask security core for application footprint with SizeBuffer

apps/system/components/ProcessManager/kata-proc-manager/src/proc_manager/mod.rs

@@ -6,13 +6,16 @@ use alloc::string::String;
 use core::convert::TryFrom;
 use core::marker::Sync;
 use hashbrown::HashMap;
-use kata_proc_common::{Bundle, DEFAULT_BUNDLE_ID_CAPACITY};
+use kata_proc_interface::Bundle;
+use kata_proc_interface::BundleIdArray;
+use kata_proc_interface::PackageManagementInterface;
+use kata_proc_interface::ProcessControlInterface;
+use kata_proc_interface::ProcessManagerError;
+use kata_proc_interface::ProcessManagerInterface;
+use kata_proc_interface::DEFAULT_BUNDLE_ID_CAPACITY;
 use log::trace;
 use smallstr::SmallString;
 
-use kata_proc_common as proc;
-use proc::*;
-
 pub type BundleId = SmallString<[u8; DEFAULT_BUNDLE_ID_CAPACITY]>;
 
 // Bundle capacity before spillover to the heap.
@@ -187,7 +190,7 @@ impl ProcessControlInterface for ProcessManager {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use proc::ProcessManagerError as pme;
+    use kata_proc_interface::ProcessManagerError as pme;
 
     // NB: just enough state to track install'd bundles
     struct FakeManager {