SDKRuntime: overhaul rpc mechanism

Simplify the rpc mechanism and make it more robust. Instead of serializing
the request token at the front of the slice assigned to request arguments,
write the token to the label field of the MessageInfo. Likewise instead
of incorporating the status in the response data return that in the
label field.  This noticeably simplifies the code and properly handles
the case where the receiver fails to map the page frame associated with
rpc (previously it kinda punted, now the caller get a proper status
result). While here extend the request/reswponse enum's to give each
error a distinct value.

Note that requsst/response tokens are passed as raw numbers under the
assumption sender + receiver are on the same machine so are using the
same byte order.

NB: this adds the num_enum crate to handle enum<>primitive conversions.

Change-Id: I536a23c7bddc43c686cc4335f22524debeeedf4f
GitOrigin-RevId: 8a9fa009dc65605b8d160330edcde02dcfa2172b

apps/system/components/SDKRuntime/kata-sdk-component/src/run.rs

@@ -48,7 +48,6 @@ use log::error;
 use sdk_interface::KeyValueData;
 use sdk_interface::SDKAppId;
 use sdk_interface::SDKError;
-use sdk_interface::SDKReplyHeader;
 use sdk_interface::SDKRuntimeError;
 use sdk_interface::SDKRuntimeInterface;
 use sdk_interface::SDKRuntimeRequest;
@@ -140,20 +139,10 @@ fn delete_path(path: &seL4_CPath) -> seL4_Result {
     unsafe { seL4_CNode_Delete(path.0, path.1, path.2 as u8) }
 }
 
-fn reply_error(error: SDKError, reply_slice: &mut [u8]) {
-    // XXX check return
-    let _ = postcard::to_slice(
-        &SDKReplyHeader {
-            status: error.into(),
-        },
-        reply_slice,
-    );
-}
-
 /// Server-side of SDKRuntime request processing.  Note CAmkES does not
 /// participate in the RPC processing we use the control thread instead
-/// of having CAmkES create an interface thread and pass parameters through
+/// of having CAmkES create an interface thread, and pass parameters
-/// a page frame attached to the IPC buffer.
+/// through a page frame attached to the IPC buffer.
 #[no_mangle]
 pub unsafe extern "C" fn run() -> ! {
     let recv_path = &Camkes::top_level_path(KATA_SDK_RECV_SLOT);
@@ -164,7 +153,12 @@ pub unsafe extern "C" fn run() -> ! {
 
     // Do initial Recv; after this we use ReplyRecv to minimize syscalls.
     let mut sdk_runtime_badge: seL4_Word = 0;
-    seL4_Recv(KATA_SDK_ENDPOINT, &mut sdk_runtime_badge as _, KATA_SDK_REPLY);
+    let mut response: Result<(), SDKError>;
+    let mut info = seL4_Recv(
+        /*src=*/ KATA_SDK_ENDPOINT,
+        /*sender=*/ &mut sdk_runtime_badge as _,
+        /*reply=*/ KATA_SDK_REPLY,
+    );
     loop {
         Camkes::debug_assert_slot_frame("run", recv_path);
         // seL4_Recv & seL4_ReplyRecv return any badge but do not reset
@@ -173,46 +167,58 @@ pub unsafe extern "C" fn run() -> ! {
         // outbound capability. To guard against this clear the field here
         // (so it happens for both calls) with clear_request_cap().
         Camkes::clear_request_cap();
-        // Map the frame with RPC parameters and decode the request header.
+        // Map the frame with RPC parameters and process the request.
         if copy_region.map(recv_path.1).is_ok() {
-            // The client serializes an SDKRequestHeader first with the
+            // The request token is passed in the MessageInfo label field.
-            // request id. This is followed by request-specific arguments
+            // Any request-specific parameters are serialized in the first
-            // that must be processed by each handler.
+            // half of the page, with the second half reserved for reply data.
+            // We might consider sending a request length out-of-band (like
+            // the request token) to enable variable page splitting.
             let (request_slice, reply_slice) = copy_region
                 .as_mut()
                 .split_at_mut(SDKRUNTIME_REQUEST_DATA_SIZE);
             let request_slice = &*request_slice; // NB: immutable alias
-            match postcard::take_from_bytes::<sdk_interface::SDKRequestHeader>(request_slice) {
+
-                Ok((header, args_slice)) => {
             let app_id = sdk_runtime_badge as SDKAppId; // XXX safe?
-                    if let Err(status) = match header.request {
+            response = match SDKRuntimeRequest::try_from(info.get_label()) {
-                        SDKRuntimeRequest::Ping => ping_request(app_id, args_slice, reply_slice),
+                Ok(SDKRuntimeRequest::Ping) => ping_request(app_id, request_slice, reply_slice),
-                        SDKRuntimeRequest::Log => log_request(app_id, args_slice, reply_slice),
+                Ok(SDKRuntimeRequest::Log) => log_request(app_id, request_slice, reply_slice),
-                        SDKRuntimeRequest::ReadKey => {
+                Ok(SDKRuntimeRequest::ReadKey) => {
-                            read_key_request(app_id, args_slice, reply_slice)
+                    read_key_request(app_id, request_slice, reply_slice)
                 }
-                        SDKRuntimeRequest::WriteKey => {
+                Ok(SDKRuntimeRequest::WriteKey) => {
-                            write_key_request(app_id, args_slice, reply_slice)
+                    write_key_request(app_id, request_slice, reply_slice)
                 }
-                        SDKRuntimeRequest::DeleteKey => {
+                Ok(SDKRuntimeRequest::DeleteKey) => {
-                            delete_key_request(app_id, args_slice, reply_slice)
+                    delete_key_request(app_id, request_slice, reply_slice)
                 }
-                    } {
+                Err(_) => {
-                        reply_error(status, reply_slice);
+                    // TODO(b/254286176): possible ddos
-                    }
+                    error!("Unknown RPC request {}", info.get_label());
-                }
+                    Err(SDKError::UnknownRequest)
-                Err(err) => reply_error(deserialize_failure(err), reply_slice),
                 }
+            };
             copy_region.unmap().expect("unmap");
         } else {
+            // TODO(b/254286176): possible ddos
             error!("Unable to map RPC parameters; badge {}", sdk_runtime_badge);
-            // TODO(jtgans): no way to return an error; signal ProcessManager to stop app?
+            response = Err(SDKError::MapPageFailed);
         }
         delete_path(recv_path).expect("delete");
         Camkes::debug_assert_slot_empty("run", recv_path);
 
-        let info = seL4_MessageInfo::new(0, 0, 0, /*length=*/ 0);
+        info = seL4_ReplyRecv(
-        seL4_ReplyRecv(KATA_SDK_ENDPOINT, info, &mut sdk_runtime_badge as _, KATA_SDK_REPLY);
+            /*src=*/ KATA_SDK_ENDPOINT,
+            /*msgInfo=*/
+            seL4_MessageInfo::new(
+                /*label=*/ SDKRuntimeError::from(response) as seL4_Word,
+                /*capsUnwrapped=*/ 0,
+                /*extraCaps=*/ 0,
+                /*length=*/ 0,
+            ),
+            /*sender=*/ &mut sdk_runtime_badge as _,
+            /*reply=*/ KATA_SDK_REPLY,
+        );
     }
 }
 
@@ -257,13 +263,7 @@ fn read_key_request(
     #[allow(clippy::uninit_assumed_init)]
     let mut keyval: KeyValueData = unsafe { ::core::mem::MaybeUninit::uninit().assume_init() };
     let value = unsafe { KATA_SDK.read_key(app_id, request.key, &mut keyval)? };
-    let _ = postcard::to_slice(
+    let _ = postcard::to_slice(&sdk_interface::ReadKeyResponse { value }, reply_slice)
-        &sdk_interface::ReadKeyResponse {
-            header: SDKReplyHeader::new(SDKRuntimeError::SDKSuccess),
-            value,
-        },
-        reply_slice,
-    )
         .map_err(serialize_failure)?;
     Ok(())
 }

apps/system/components/SDKRuntime/sdk-interface/Cargo.toml

@@ -18,6 +18,7 @@ version = "0.1.0"
 edition = "2021"
 
 [dependencies]
+num_enum = { version = "0.5", default-features = false }
 postcard = { version = "0.7", features = ["alloc"], default-features = false }
 sel4-sys = { path = "../../kata-os-common/src/sel4-sys", default-features = false }
 serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] }

apps/system/components/SDKRuntime/sdk-interface/src/error.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use serde::{Deserialize, Serialize};
+use num_enum::TryFromPrimitive;
 
 /// Rust Error enum used for representing an SDK error with postcard. This is
 /// what most rust components will actually use as their error handling enum.
@@ -25,15 +25,19 @@ pub enum SDKError {
     ReadKeyFailed,
     WriteKeyFailed,
     DeleteKeyFailed,
+    MapPageFailed,
+    UnknownRequest,
+    UnknownResponse,
 }
 
 impl From<postcard::Error> for SDKError {
     fn from(_err: postcard::Error) -> SDKError { SDKError::SerializeFailed }
 }
 
-/// C-version of SDKError presented over the CAmkES rpc interface.
+/// SDKError presented over the seL4 IPC interface. We need repr(seL4_Word)
-#[repr(C)]
+/// but cannot use that so use the implied usize type instead.
-#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
+#[repr(usize)]
+#[derive(Debug, Eq, PartialEq, TryFromPrimitive)]
 pub enum SDKRuntimeError {
     SDKSuccess = 0,
     SDKDeserializeFailed,
@@ -43,6 +47,9 @@ pub enum SDKRuntimeError {
     SDKReadKeyFailed,
     SDKWriteKeyFailed,
     SDKDeleteKeyFailed,
+    SDKMapPageFailed,
+    SDKUnknownRequest,
+    SDKUnknownResponse,
 }
 
 /// Mapping function from Rust -> C.
@@ -56,6 +63,9 @@ impl From<SDKError> for SDKRuntimeError {
             SDKError::ReadKeyFailed => SDKRuntimeError::SDKReadKeyFailed,
             SDKError::WriteKeyFailed => SDKRuntimeError::SDKWriteKeyFailed,
             SDKError::DeleteKeyFailed => SDKRuntimeError::SDKDeleteKeyFailed,
+            SDKError::MapPageFailed => SDKRuntimeError::SDKMapPageFailed,
+            SDKError::UnknownRequest => SDKRuntimeError::SDKUnknownRequest,
+            SDKError::UnknownResponse => SDKRuntimeError::SDKUnknownResponse,
         }
     }
 }
@@ -79,6 +89,9 @@ impl From<SDKRuntimeError> for Result<(), SDKError> {
             SDKRuntimeError::SDKReadKeyFailed => Err(SDKError::ReadKeyFailed),
             SDKRuntimeError::SDKWriteKeyFailed => Err(SDKError::WriteKeyFailed),
             SDKRuntimeError::SDKDeleteKeyFailed => Err(SDKError::DeleteKeyFailed),
+            SDKRuntimeError::SDKMapPageFailed => Err(SDKError::DeleteKeyFailed),
+            SDKRuntimeError::SDKUnknownRequest => Err(SDKError::UnknownRequest),
+            SDKRuntimeError::SDKUnknownResponse => Err(SDKError::UnknownResponse),
         }
     }
 }

apps/system/components/SDKRuntime/sdk-interface/src/lib.rs

@@ -21,6 +21,7 @@ pub mod error;
 pub use error::SDKError;
 pub use error::SDKRuntimeError;
 
+use num_enum::{IntoPrimitive, TryFromPrimitive};
 use serde::{Deserialize, Serialize};
 
 use sel4_sys::seL4_CPtr;
@@ -61,33 +62,6 @@ pub type SDKAppId = usize;
 pub const KEY_VALUE_DATA_SIZE: usize = 100;
 pub type KeyValueData = [u8; KEY_VALUE_DATA_SIZE];
 
-/// All RPC request must have an SDKRequestHeader at the front.
-#[derive(Serialize, Deserialize)]
-pub struct SDKRequestHeader {
-    pub request: SDKRuntimeRequest,
-}
-impl SDKRequestHeader {
-    pub fn new(request: SDKRuntimeRequest) -> Self { Self { request } }
-}
-
-/// All RPC responses must have an SDKReplyHeader at the front.
-#[derive(Serialize, Deserialize)]
-pub struct SDKReplyHeader {
-    pub status: SDKRuntimeError,
-}
-impl SDKReplyHeader {
-    pub fn new(status: SDKRuntimeError) -> Self { Self { status } }
-}
-impl From<SDKReplyHeader> for Result<(), SDKRuntimeError> {
-    fn from(header: SDKReplyHeader) -> Result<(), SDKRuntimeError> {
-        if header.status == SDKRuntimeError::SDKSuccess {
-            Ok(())
-        } else {
-            Err(header.status)
-        }
-    }
-}
-
 /// SDKRuntimeRequest::Ping
 #[derive(Serialize, Deserialize)]
 pub struct PingRequest {}
@@ -105,7 +79,6 @@ pub struct ReadKeyRequest<'a> {
 }
 #[derive(Serialize, Deserialize)]
 pub struct ReadKeyResponse<'a> {
-    pub header: SDKReplyHeader,
     pub value: &'a [u8],
 }
 
@@ -122,8 +95,10 @@ pub struct DeleteKeyRequest<'a> {
     pub key: &'a str,
 }
 
-#[repr(C)] // XXX needed?
+/// SDKRequest token sent over the seL4 IPC interface. We need repr(seL4_Word)
-#[derive(Clone, Copy, Eq, PartialEq, Serialize, Deserialize, Debug)]
+/// but cannot use that so use the implied usize type instead.
+#[repr(usize)]
+#[derive(Debug, Clone, Copy, Eq, PartialEq, IntoPrimitive, TryFromPrimitive)]
 pub enum SDKRuntimeRequest {
     Ping = 0, // Check runtime is alive
     Log,      // Log message: [msg: &str]
@@ -170,10 +145,10 @@ pub trait SDKRuntimeInterface {
 /// Rust client-side request processing. Note there is no CAmkES stub to
 /// call; everything is done here. A single page frame is attached to the
 /// IPC buffer with request parameters in the first half and return values
-/// in the second half. Requests must have an SDKRequestHeader serialized
+/// in the second half. Requests must have an SDKRequestHeader written to
-/// separately from any arguments. Responses must have an SDKReplyHeader
+/// the label field of the MessageInfo. Responses must have an SDKRuntimeError
-/// included in the reply data. For the moment this uses postcard to do
+/// written to the label field of the reply. For the moment this uses
-/// serde work; this may change in the future (e.g. to flatbuffers).
+/// postcard for serde work; this may change in the future (e.g. to flatbuffers).
 ///
 /// The caller is responsible for synchronizing access to KATA_SDK_* state
 /// and the IPC buffer.
@@ -185,10 +160,6 @@ pub trait SDKRuntimeInterface {
 //   to lookup the mapped page early. Downside to a fixed mapping is it
 //   limits how to handle requests w/ different-sized params (e.g. sensor
 //   frame vs key-value params).
-// TODO(sleffler): could send request header and reponse statatus inline.
-//   This would align request arguments to the page boundary which might
-//   be useful and having the reply inline would mean SDKRuntime could
-//   send a meaningful error back when unable to map the page frame.
 fn sdk_request<'a, S: Serialize, D: Deserialize<'a>>(
     request: SDKRuntimeRequest,
     request_args: &S,
@@ -199,24 +170,32 @@ fn sdk_request<'a, S: Serialize, D: Deserialize<'a>>(
     let (request_slice, reply_slice) = params_slice.split_at_mut(SDKRUNTIME_REQUEST_DATA_SIZE);
     reply_slice.fill(0); // XXX paranoid, could zero-pad request too
 
-    // Encode heeader with request.
+    // Encode request arguments.
-    // TODO(sleffler): eliminate struct? (could add a sequence #)
+    let _ = postcard::to_slice(request_args, request_slice)
-    let header_size = (postcard::to_slice(&SDKRequestHeader::new(request), request_slice)
-        .map_err(|_| SDKRuntimeError::SDKSerializeFailed)?)
-    .len();
-
-    // Encode arguments immediately after.
-    let (_, args_slice) = request_slice.split_at_mut(header_size);
-    let _ = postcard::to_slice(request_args, args_slice)
         .map_err(|_| SDKRuntimeError::SDKSerializeFailed)?;
 
     // Attach params & call the SDKRuntime; then wait (block) for a reply.
     unsafe {
         seL4_SetCap(0, KATA_SDK_FRAME);
-        seL4_Call(KATA_SDK_ENDPOINT, seL4_MessageInfo::new(0, 0, 1, 0));
+        let info = seL4_Call(
+            KATA_SDK_ENDPOINT,
+            seL4_MessageInfo::new(
+                /*label=*/ request.into(),
+                /*capsUnrapped=*/ 0,
+                /*extraCaps=*/ 1,
+                /*length=*/ 0,
+            ),
+        );
         seL4_SetCap(0, 0);
+
+        let status = SDKRuntimeError::try_from(info.get_label())
+            .map_err(|_| SDKRuntimeError::SDKUnknownResponse)?;
+        if status != SDKRuntimeError::SDKSuccess {
+            return Err(status);
+        }
     }
 
+    // Decode response data.
     postcard::from_bytes::<D>(reply_slice).map_err(|_| SDKRuntimeError::SDKDeserializeFailed)
 }
 
@@ -224,22 +203,19 @@ fn sdk_request<'a, S: Serialize, D: Deserialize<'a>>(
 #[inline]
 #[allow(dead_code)]
 pub fn sdk_ping() -> Result<(), SDKRuntimeError> {
-    let header =
+    sdk_request::<PingRequest, ()>(SDKRuntimeRequest::Ping, &PingRequest {})
-        sdk_request::<PingRequest, SDKReplyHeader>(SDKRuntimeRequest::Ping, &PingRequest {})?;
-    header.into()
 }
 
 /// Rust client-side wrapper for the log method.
 #[inline]
 #[allow(dead_code)]
 pub fn sdk_log(msg: &str) -> Result<(), SDKRuntimeError> {
-    let header = sdk_request::<LogRequest, SDKReplyHeader>(
+    sdk_request::<LogRequest, ()>(
         SDKRuntimeRequest::Log,
         &LogRequest {
             msg: msg.as_bytes(),
         },
-    )?;
+    )
-    header.into()
 }
 
 /// Rust client-side wrapper for the read key method.
@@ -251,34 +227,20 @@ pub fn sdk_read_key<'a>(key: &str, keyval: &'a mut [u8]) -> Result<&'a [u8], SDK
         SDKRuntimeRequest::ReadKey,
         &ReadKeyRequest { key },
     )?;
-    match response.header.status {
+    keyval.copy_from_slice(response.value);
-        SDKRuntimeError::SDKSuccess => {
+    Ok(keyval)
-            let (left, _) = keyval.split_at_mut(response.value.len());
-            left.copy_from_slice(response.value);
-            Ok(left)
-        }
-        e => Err(e),
-    }
 }
 
 /// Rust client-side wrapper for the write key method.
 #[inline]
 #[allow(dead_code)]
 pub fn sdk_write_key(key: &str, value: &[u8]) -> Result<(), SDKRuntimeError> {
-    let header = sdk_request::<WriteKeyRequest, SDKReplyHeader>(
+    sdk_request::<WriteKeyRequest, ()>(SDKRuntimeRequest::WriteKey, &WriteKeyRequest { key, value })
-        SDKRuntimeRequest::WriteKey,
-        &WriteKeyRequest { key, value },
-    )?;
-    header.into()
 }
 
 /// Rust client-side wrapper for the delete key method.
 #[inline]
 #[allow(dead_code)]
 pub fn sdk_delete_key(key: &str) -> Result<(), SDKRuntimeError> {
-    let header = sdk_request::<DeleteKeyRequest, SDKReplyHeader>(
+    sdk_request::<DeleteKeyRequest, ()>(SDKRuntimeRequest::DeleteKey, &DeleteKeyRequest { key })
-        SDKRuntimeRequest::DeleteKey,
-        &DeleteKeyRequest { key },
-    )?;
-    header.into()
 }