DemesneGH commented on code in PR #209: URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/209#discussion_r2203801135
########## optee-utee/src/object/object_handle.rs: ########## @@ -0,0 +1,185 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use alloc::boxed::Box; +use optee_utee_sys as raw; + +use super::GenericObject; + +/// An opaque handle on an object. +#[derive(Debug)] +pub struct ObjectHandle(Box<raw::TEE_ObjectHandle>); + +impl ObjectHandle { + /// A null handle for later usage + /// + /// Example: + /// ``` rust,no_run + /// # use optee_utee::ObjectHandle; + /// let handle = ObjectHandle::new_null(); + /// assert!(handle.is_null()); + /// ``` + pub fn new_null() -> Self { + Self(Box::new(core::ptr::null_mut())) + } + + pub fn from_raw(raw: raw::TEE_ObjectHandle) -> ObjectHandle { + Self(Box::new(raw)) + } + + pub fn handle(&self) -> raw::TEE_ObjectHandle { + *self.0 + } + + pub fn handle_mut(&mut self) -> &mut raw::TEE_ObjectHandle { + &mut self.0 + } + + pub fn is_null(&self) -> bool { + self.0.is_null() || (*self.0).is_null() + } + + /// Forget the inner handle to prevent a double-free, this function would be + /// called when the inner handle is(or will be) freed externally. + /// + /// Caution: This has the side effect of nullifying the inner handle. + /// + /// Example: + /// ``` rust,no_run + /// # use optee_utee::ObjectHandle; + /// # use optee_utee_sys as raw; + /// # let external_handle: raw::TEE_ObjectHandle = core::ptr::null_mut(); + /// // `external_handle` is a handle that is constructed and controlled + /// // externally. + /// // `handle` is valid, and will call TEE_CloseObject on + /// // `external_handle` when it is dropping, which is not allowed + /// // as the `external_handle` is externally controlled. + /// let mut handle = ObjectHandle::from_raw(external_handle); + /// // ... Some operation + /// // forget the inner handle, so it won't call TEE_CloseObject on + /// // `external_handle` + /// handle.forget_raw(); + /// assert!(handle.is_null()); + /// ``` + pub fn forget_raw(&mut self) { + self.0 = Box::new(core::ptr::null_mut()); + } +} + +impl Drop for ObjectHandle { + fn drop(&mut self) { + if !self.is_null() { + unsafe { raw::TEE_CloseObject(self.handle()) } + } + } +} + +impl GenericObject for ObjectHandle { + fn handle(&self) -> raw::TEE_ObjectHandle { + self.handle() + } +} + +#[cfg(test)] +mod tests { + extern crate std; + use std::sync::Arc; + + use optee_utee_mock::{ + object::{set_global_object_mock, MockObjectController, SERIAL_TEST_LOCK}, + raw, + }; + + use super::*; + + /// Ensures `ObjectHandle` can be safely constructed from a raw handle + /// and automatically calls `TEE_CloseObject` when dropped. + #[test] + fn test_from_raw() { + let _lock = SERIAL_TEST_LOCK.lock(); + + let mut mock = MockObjectController::new(); + let mut handle_struct: raw::__TEE_ObjectHandle = unsafe { core::mem::zeroed() }; + let handle: Arc<raw::TEE_ObjectHandle> = Arc::new(&mut handle_struct); + + mock.expect_TEE_CloseObject().return_once_st(|_| {}).once(); + set_global_object_mock(mock); + + let obj = ObjectHandle::from_raw(*handle); + assert!(!obj.is_null()); + assert_eq!(obj.handle(), *handle); + } + + /// Ensures `ObjectHandle` can call `forget_raw` to prevent automatically + /// calls `TEE_CloseObject` when dropped. + #[test] + fn test_forget_raw() { + let _lock = SERIAL_TEST_LOCK.lock(); + + let mut handle_struct: raw::__TEE_ObjectHandle = unsafe { core::mem::zeroed() }; + let handle: Arc<raw::TEE_ObjectHandle> = Arc::new(&mut handle_struct); + // making sure nothing would be called(includes TEE_CloseObject) + let mock = MockObjectController::new(); + set_global_object_mock(mock); + + let mut obj = ObjectHandle::from_raw(*handle); + assert!(!obj.is_null()); + assert_eq!(obj.handle(), *handle); + + obj.forget_raw(); + assert!(obj.is_null()); + } + + #[test] + fn test_new_null() { + let _lock = SERIAL_TEST_LOCK.lock(); + // making sure nothing would be called(includes TEE_CloseObject) + let mock = MockObjectController::new(); + set_global_object_mock(mock); + + let obj = ObjectHandle::new_null(); + assert!(obj.is_null()); + } + + /// Ensures the return value of `handle_mut` can be written. + /// + /// TODO: still need a systest to ensure that, I found that if change + /// `ObjectHandle` from `Object(Box<raw::TEE_ObjectHandle>)` to + /// `ObjectHandle(raw::TEE_ObjectHandle)`, this unittest still works, + /// but will result in `Panic by Permission Fault` in TA, so things is kind + /// of different between unittest and TA. Review Comment: > For example, the demo code you’ve provided demonstrates the memory issue I mentioned earlier, which will result in a **Permission Fault** in OP-TEE OS. Do you mean that the variable used to receive the handle cannot be on the stack (causes Permission Fault) and should be on the heap instead? I notice that the C examples for `object` also use the variable on the stack: https://github.com/linaro-swg/optee_examples/blob/master/secure_storage/ta/secure_storage_ta.c#L85. Any thing different with Rust? Additionally, I recall the similar pattern from the TA implementation: https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/main/optee-utee/src/ta_session.rs#L52 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org For additional commands, e-mail: dev-h...@teaclave.apache.org
