kata-security-coordinator::fakeimpl: correct LoadModel & LoadApplication

Change the fake to behave as the real impl will for LoadModel &
LoadApplication: return a deep copy of the saved package contents as
would happen if the data were pulled from flash. Match this behaviour in
the kata-shell SecurityCoordinator test commands and the MlCoordinator
by taking ownership of the received objects and free'ing them when no
longer needed.

With these changes one can install a bundle and repeatedly load_application
without leaking any memory, capabilities, or slots in the toplevel CNode
of the caller (DebugConsole in this case).

Likewise doing install of a model, test_mlexecute, and then uninstall
of the model's bundle returns all resources.

Specific changes:
- correctly release resources in kata-shell load_application & load_model
- correct release of bundle_frames in seL4BundleImpl::stop
- release resources in MlCoordinator::load_model
- connect the MemoryInterface to the MlCoordinator so it can return memory
- setup two copyregions in the SecurityCoordinator to do the deep copy
- add ObjdDescBundle::cptr_iter for iterating over the set of seL4_CPtr's
- hack kata_frame_alloc_in_cnode to split requests according to the
  kernel's config on the max Retype count
- while here switch test_mailbox to use one of the copyregions

TODO:
- deep_copy allocates all frames at once which requires a band-aid;
  either hide that in MemoryManager or maybe allocate a page at a time

Change-Id: Ia425976b31ea7a32b1d0e4affc3a0ef9ba966c87
GitOrigin-RevId: 31d5bc99b569a5eab9c33c7e1014793bfe57161e

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

@@ -8,6 +8,7 @@ use crate::CommandError;
 use crate::HashMap;
 
 use kata_io as io;
+use kata_memory_interface::kata_object_free_in_cnode;
 use kata_os_common::cspace_slot::CSpaceSlot;
 use kata_security_interface::*;
 use kata_storage_interface::KEY_VALUE_DATA_SIZE;
@@ -76,9 +77,13 @@ fn load_application_command(
     _builtin_cpio: &[u8],
 ) -> Result<(), CommandError> {
     let bundle_id = args.next().ok_or(CommandError::BadArgs)?;
-    let container_slot = CSpaceSlot::new();
+    let mut container_slot = CSpaceSlot::new();
     match kata_security_load_application(bundle_id, &container_slot) {
-        Ok(frames) => writeln!(output, "{:?}", &frames)?,
+        Ok(frames) => {
+            container_slot.release(); // NB: take ownership
+            writeln!(output, "{:?}", &frames)?;
+            let _ = kata_object_free_in_cnode(&frames);
+        },
         Err(status) => writeln!(output, "LoadApplication failed: {:?}", status)?,
     }
     Ok(())
@@ -92,9 +97,13 @@ fn load_model_command(
 ) -> Result<(), CommandError> {
     let bundle_id = args.next().ok_or(CommandError::BadArgs)?;
     let model_id = args.next().ok_or(CommandError::BadArgs)?;
-    let container_slot = CSpaceSlot::new();
+    let mut container_slot = CSpaceSlot::new();
     match kata_security_load_model(bundle_id, model_id, &container_slot) {
-        Ok(frames) => writeln!(output, "{:?}", &frames)?,
+        Ok(frames) => {
+            container_slot.release(); // NB: take ownership
+            writeln!(output, "{:?}", &frames)?;
+            let _ = kata_object_free_in_cnode(&frames);
+        }
         Err(status) => writeln!(output, "LoadApplication failed: {:?}", status)?,
     }
     Ok(())

apps/system/components/MemoryManager/kata-memory-interface/src/lib.rs

@@ -184,6 +184,11 @@ impl ObjDescBundle {
         }
     }
 
+    // Returns an iterator that enumerates each object's seL4_CPtr.
+    pub fn cptr_iter(&self) -> impl Iterator<Item = seL4_CPtr> + '_ {
+        self.objs.iter().flat_map(|od| od.cptr..(od.cptr + od.retype_count()))
+    }
+
     // Move objects to dynamically-allocated slots in the top-level
     // CNode and mutate the Object Descriptor with the new cptr's.
     // TODO(sleffler) make generic (requires supplying slot allocator)?
@@ -584,9 +589,20 @@ pub fn kata_frame_alloc_in_cnode(space_bytes: usize)
     }
     // NB: always allocate 4K pages
     let npages = howmany(space_bytes, 1 << seL4_PageBits);
-    kata_object_alloc_in_cnode(vec![
+    // XXX horrible band-aid to workaround Retype "fanout" limit of 256
-        ObjDesc::new( seL4_RISCV_4K_Page, npages, /*cptr=*/ 0)
+    // objects: split our request accordingly. This shold be handled in
-    ])
+    // MemoryManager using the kernel config or bump the kernel limit.
+    assert!(npages <= 512); // XXX 2MB
+    if npages > 256 {
+        kata_object_alloc_in_cnode(vec![
+            ObjDesc::new( seL4_RISCV_4K_Page, 256, /*cptr=*/ 0),
+            ObjDesc::new( seL4_RISCV_4K_Page, npages - 256, /*cptr=*/ 256),
+        ])
+    } else {
+        kata_object_alloc_in_cnode(vec![
+            ObjDesc::new( seL4_RISCV_4K_Page, npages, /*cptr=*/ 0),
+        ])
+    }
 }
 
 #[inline]

apps/system/components/MlCoordinator/MlCoordinator.camkes

@@ -1,4 +1,5 @@
 import <LoggerInterface.camkes>;
+import <MemoryInterface.camkes>;
 import <MlCoordinatorInterface.camkes>;
 import <SecurityCoordinatorInterface.camkes>;
 
@@ -19,6 +20,7 @@ component MlCoordinator {
   dataport Buf(0x1000000) dtcm;
 
   uses LoggerInterface logger;
+  uses MemoryInterface memory;
   uses SecurityCoordinatorInterface security;
 
   // Enable KataOS CAmkES support.

apps/system/components/MlCoordinator/kata-ml-coordinator/src/lib.rs

@@ -6,6 +6,7 @@ extern crate alloc;
 
 use alloc::string::String;
 use alloc::vec::Vec;
+use kata_memory_interface::kata_object_free_in_cnode;
 use kata_ml_interface::MlCoordError;
 use kata_os_common::cspace_slot::CSpaceSlot;
 use kata_security_interface::*;
@@ -89,10 +90,11 @@ impl MLCoordinator {
         // To load the model into the vector core the pages must be
         // mapped into the MlCoordinator's VSpace before being copied
         // to their destination.
-        let container_slot = CSpaceSlot::new();
+        let mut container_slot = CSpaceSlot::new();
         match kata_security_load_model(&model.bundle_id, &model.model_id, &container_slot) {
             Ok(model_frames) => {
-                match MlCore::load_image(&model_frames) {
+                container_slot.release();  // NB: take ownership
+                let ret_status = match MlCore::load_image(&model_frames) {
                     Err(e) => {
                         error!(
                             "Load of {}:{} failed: {:?}",
@@ -109,7 +111,9 @@ impl MLCoordinator {
                         self.loaded_model = Some(model_idx);
                         Ok(())
                     }
-                }
+                };
+                let _ = kata_object_free_in_cnode(&model_frames);
+                ret_status
             }
             Err(e) => {
                 error!(

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

@@ -592,7 +592,7 @@ impl BundleImplInterface for seL4BundleImpl {
     }
     fn stop(&mut self) -> Result<(), ProcessManagerError> {
         self.suspend()?;
-        kata_object_free(&self.bundle_frames)
+        kata_object_free_in_cnode(&self.bundle_frames)
             .map_err(|_| ProcessManagerError::StopFailed)?;
         kata_object_free_in_cnode(&self.dynamic_objs)
             .map_err(|_| ProcessManagerError::StopFailed)?;

apps/system/components/SecurityCoordinator/SecurityCoordinator.camkes

@@ -18,6 +18,7 @@ component SecurityCoordinator {
   // Add free slots for processing requests.
   attribute int cnode_headroom = 32;
 
-  // For mailbox use. What should we call this?
+  // For fakeimpl deep_copy (re-used by test_mailbox).
-  has copyregion COPYREGION;
+  has copyregion DEEP_COPY_SRC;
+  has copyregion DEEP_COPY_DEST;
 }

apps/system/components/SecurityCoordinator/kata-security-coordinator/src/fakeimpl/mod.rs

@@ -4,13 +4,32 @@ extern crate alloc;
 use alloc::fmt;
 use alloc::string::{String, ToString};
 use core::mem::size_of;
+use core::ptr;
 use hashbrown::HashMap;
-use kata_memory_interface::*;
+use kata_memory_interface::kata_frame_alloc;
-use kata_os_common::sel4_sys::*;
+use kata_memory_interface::kata_frame_alloc_in_cnode;
+use kata_memory_interface::kata_object_free_in_cnode;
+use kata_memory_interface::kata_object_free_toplevel;
+use kata_memory_interface::ObjDescBundle;
+use kata_os_common::copyregion::CopyRegion;
+use kata_os_common::cspace_slot::CSpaceSlot;
+use kata_os_common::sel4_sys;
 use kata_security_interface::*;
 use kata_storage_interface::KeyValueData;
 use log::trace;
 
+use sel4_sys::seL4_Page_GetAddress;
+use sel4_sys::seL4_PageBits;
+use sel4_sys::seL4_Word;
+
+const PAGE_SIZE: usize = 1 << seL4_PageBits;
+
+extern "C" {
+    // Regions for deep_copy work.
+    static mut DEEP_COPY_SRC: [seL4_Word; PAGE_SIZE / size_of::<seL4_Word>()];
+    static mut DEEP_COPY_DEST: [seL4_Word; PAGE_SIZE / size_of::<seL4_Word>()];
+}
+
 struct BundleData {
     pkg_contents: ObjDescBundle,
     pkg_size: usize,
@@ -19,10 +38,9 @@ struct BundleData {
 }
 impl BundleData {
     fn new(pkg_contents: &ObjDescBundle) -> Self {
-        let size_bytes = pkg_contents.objs.len() * 4096; // XXX
         BundleData {
             pkg_contents: pkg_contents.clone(),
-            pkg_size: size_bytes,
+            pkg_size: pkg_contents.size_bytes(),
             manifest: String::from(
                 r##"
 # Comments like this
@@ -80,6 +98,47 @@ impl FakeSecurityCoordinator {
 }
 pub type KataSecurityCoordinatorInterface = FakeSecurityCoordinator;
 
+// Returns a deep copy (including seL4 objects) of |src|. The container
+// CNode is in the toplevel (allocated from the slot allocator).
+fn deep_copy(src: &ObjDescBundle) -> ObjDescBundle {
+    let dest = kata_frame_alloc_in_cnode(src.size_bytes())
+        .expect("deep_copy:alloc");
+    // Src top-level slot & copy region
+    let src_slot = CSpaceSlot::new();
+    let mut src_region = CopyRegion::new(
+        unsafe { ptr::addr_of_mut!(DEEP_COPY_SRC[0]) }, PAGE_SIZE
+    );
+    // Dest top-level slot & copy region
+    let dest_slot = CSpaceSlot::new();
+    let mut dest_region = CopyRegion::new(
+        unsafe { ptr::addr_of_mut!(DEEP_COPY_DEST[0]) }, PAGE_SIZE
+    );
+    for (src_cptr, dest_cptr) in src.cptr_iter().zip(dest.cptr_iter()) {
+        // Map src & dest frames and copy data.
+        src_slot.copy_to(src.cnode, src_cptr, src.depth)
+            .and_then(|_| src_region.map(src_slot.slot))
+            .expect("src_map");
+        dest_slot.copy_to(dest.cnode, dest_cptr, dest.depth)
+            .and_then(|_| dest_region.map(dest_slot.slot))
+            .expect("dest_map");
+
+        unsafe {
+            ptr::copy_nonoverlapping(
+                src_region.as_ref().as_ptr(),
+                dest_region.as_mut().as_mut_ptr(),
+                PAGE_SIZE,
+            );
+        }
+
+        // Unmap & clear top-level slot required for mapping.
+        src_region.unmap().and_then(|_| src_slot.delete())
+            .expect("src_unmap");
+        dest_region.unmap().and_then(|_| dest_slot.delete())
+            .expect("dest_unmap");
+    }
+    dest
+}
+
 impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
     fn install(&mut self, pkg_contents: &ObjDescBundle) -> Result<String, SecurityRequestError> {
         // TODO(sleffler): get bundle_id from the manifest; for now use the
@@ -108,8 +167,10 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
     }
     fn load_application(&self, bundle_id: &str) -> Result<ObjDescBundle, SecurityRequestError> {
         let bundle_data = self.get_bundle(bundle_id)?;
+        // Clone everything (struct + associated seL4 objects) so the
+        // return is as though it was newly instantiated from flash.
         // XXX just return the package for now
-        Ok(bundle_data.pkg_contents.clone())
+        Ok(deep_copy(&bundle_data.pkg_contents))
     }
     fn load_model(
         &self,
@@ -118,8 +179,10 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
     ) -> Result<ObjDescBundle, SecurityRequestError> {
         let bundle_data = self.get_bundle(bundle_id)?;
         // TODO(sleffler): check model id
+        // Clone everything (struct + associated seL4 objects) so the
+        // return is as though it was newly instantiated from flash.
         // XXX just return the package for now
-        Ok(bundle_data.pkg_contents.clone())
+        Ok(deep_copy(&bundle_data.pkg_contents))
     }
     fn read_key(&self, bundle_id: &str, key: &str) -> Result<&KeyValueData, SecurityRequestError> {
         let bundle = self.get_bundle(bundle_id)?;
@@ -148,17 +211,11 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
     fn test_mailbox(&mut self) -> Result<(), SecurityRequestError> {
         trace!("test_mailbox_command()");
 
-        const PAGE_SIZE: usize = 1 << seL4_PageBits;
         const MESSAGE_SIZE_DWORDS: usize = 17; // Just a random message size for testing.
 
         extern "C" {
             fn mailbox_api_send(paddr: u32, size: u32);
             fn mailbox_api_receive(paddr: *mut u32, size: *mut u32);
-            static SELF_VSPACE_ROOT: seL4_CPtr;
-
-            // This is not actually a block of memory, it's a reserved range in
-            // the virtual address space that we can map physical memory into.
-            static mut COPYREGION: [u32; 1024];
         }
 
         // Allocate a 4k page to serve as our message buffer.
@@ -168,31 +225,25 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
 
         unsafe {
             // Map the message buffer into our copyregion so we can access it.
-            // FIXME(aappleby): We need a drop() impl here somewhere so this
+            // NB: re-use one of the deep_copy copyregions.
-            // doesn't leak if something fails.
+            let mut msg_region = CopyRegion::new(
-            let message_ptr = core::ptr::addr_of_mut!(COPYREGION[0]);
+                ptr::addr_of_mut!(DEEP_COPY_SRC[0]),
-            seL4_Page_Map(
+                PAGE_SIZE
-                /*sel4_page=*/ frame_bundle.objs[0].cptr,
+            );
-                /*seL4_pd=*/ SELF_VSPACE_ROOT,
+            msg_region.map(frame_bundle.objs[0].cptr)
-                /*vaddr=*/ message_ptr as usize,
+                .map_err(|_| SecurityRequestError::SreTestFailed)?;
-                seL4_CapRights::new(
+
-                    // NB: RW 'cuz W-only silently gets upgraded by kernel
+            let message_ptr = msg_region.as_word_mut();
-                    /*grant_reply=*/
-                    0, /*grant=*/ 0, /*read=1*/ 1, /*write=*/ 1,
-                ),
-                seL4_Default_VMAttributes,
-            )
-            .map_err(|_| SecurityRequestError::SreTestFailed)?;
 
             // Write to the message buffer through the copyregion.
-            let offset_a = 0 as isize;
+            let offset_a = 0;
-            let offset_b = (MESSAGE_SIZE_DWORDS - 1) as isize;
+            let offset_b = MESSAGE_SIZE_DWORDS - 1;
-            message_ptr.offset(offset_a).write(0xDEADBEEF);
+            message_ptr[offset_a] = 0xDEADBEEF;
-            message_ptr.offset(offset_b).write(0xF00DCAFE);
+            message_ptr[offset_b] = 0xF00DCAFE;
             trace!(
                 "test_mailbox: old buf contents  0x{:X} 0x{:X}",
-                message_ptr.offset(offset_a).read(),
+                message_ptr[offset_a],
-                message_ptr.offset(offset_b).read()
+                message_ptr[offset_b]
             );
 
             // Send the _physical_ address of the message buffer to the security
@@ -216,14 +267,14 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
             trace!("test_mailbox: expected contents 0x12345678 0x87654321");
             trace!(
                 "test_mailbox: new buf contents  0x{:X} 0x{:X}",
-                message_ptr.offset(offset_a).read(),
+                message_ptr[offset_a],
-                message_ptr.offset(offset_b).read()
+                message_ptr[offset_b]
             );
 
-            let dword_a = message_ptr.offset(offset_a).read();
+            let dword_a = message_ptr[offset_a];
-            let dword_b = message_ptr.offset(offset_b).read();
+            let dword_b = message_ptr[offset_b];
 
-            seL4_Page_Unmap(frame_bundle.objs[0].cptr)
+            msg_region.unmap()
                 .map_err(|_| SecurityRequestError::SreTestFailed)?;
 
             // Done, free the message buffer.

apps/system/system.camkes

@@ -155,7 +155,7 @@ assembly {
             from debug_console.memory,
             from process_manager.memory,
             from security_coordinator.memory,
-            // TOOD(sleffler): from ml_coordinator.memory,
+            from ml_coordinator.memory,
             to memory_manager.memory);
 
         // Connect the SecurityCoordinatorInterface to each component that needs
@@ -164,7 +164,7 @@ assembly {
         connection seL4RPCOverMultiSharedData multi_security(
             from debug_console.security,   // NB: for debug/test
             from process_manager.security,
-            from ml_coordinator.security,  // NB: for LoadModel but not in design
+            from ml_coordinator.security,  // NB: for LoadModel
             from storage_manager.security,
             to security_coordinator.security);