tock · jrvanwhy · Nov 17, 2024 · Aug 17, 2024 · Oct 14, 2024 · Oct 14, 2024
diff --git a/arch/cortex-m/src/syscall.rs b/arch/cortex-m/src/syscall.rs
@@ -195,7 +195,15 @@ impl<A: CortexMVariant> kernel::syscall::UserspaceKernelBoundary for SysCall<A>
         //
         // Refer to
         // https://doc.rust-lang.org/std/primitive.pointer.html#safety-13
-        return_value.encode_syscall_return(&mut *r0, &mut *r1, &mut *r2, &mut *r3);
+        kernel::utilities::arch_helpers::encode_syscall_return_trd104(
+            &kernel::utilities::arch_helpers::TRD104SyscallReturn::from_syscall_return(
+                return_value,
+            ),
+            &mut *r0,
+            &mut *r1,
+            &mut *r2,
+            &mut *r3,
+        );
 
         Ok(())
     }
@@ -232,9 +240,9 @@ impl<A: CortexMVariant> kernel::syscall::UserspaceKernelBoundary for SysCall<A>
         //  - Stack offset 4 is R12, which the syscall interface ignores
         let stack_bottom = state.psp as *mut usize;
         ptr::write(stack_bottom.offset(7), state.psr); //......... -> APSR
-        ptr::write(stack_bottom.offset(6), callback.pc | 1); //... -> PC
+        ptr::write(stack_bottom.offset(6), usize::from(callback.pc) | 1); //... -> PC
         ptr::write(stack_bottom.offset(5), state.yield_pc | 1); // -> LR
-        ptr::write(stack_bottom.offset(3), callback.argument3); // -> R3
+        ptr::write(stack_bottom.offset(3), callback.argument3.into()); // -> R3
         ptr::write(stack_bottom.offset(2), callback.argument2); // -> R2
         ptr::write(stack_bottom.offset(1), callback.argument1); // -> R1
         ptr::write(stack_bottom.offset(0), callback.argument0); // -> R0
@@ -300,8 +308,13 @@ impl<A: CortexMVariant> kernel::syscall::UserspaceKernelBoundary for SysCall<A>
 
             // Use the helper function to convert these raw values into a Tock
             // `Syscall` type.
-            let syscall =
-                kernel::syscall::Syscall::from_register_arguments(svc_num, r0, r1, r2, r3);
+            let syscall = kernel::syscall::Syscall::from_register_arguments(
+                svc_num,
+                r0,
+                r1.into(),
+                r2.into(),
+                r3.into(),
+            );
 
             match syscall {
                 Some(s) => kernel::syscall::ContextSwitchReason::SyscallFired { syscall: s },

diff --git a/arch/rv32i/src/syscall.rs b/arch/rv32i/src/syscall.rs
@@ -172,7 +172,10 @@ impl kernel::syscall::UserspaceKernelBoundary for SysCall {
         let (a1slice, r) = r.split_at_mut(R_A2 - R_A1);
         let (a2slice, a3slice) = r.split_at_mut(R_A3 - R_A2);
 
-        return_value.encode_syscall_return(
+        kernel::utilities::arch_helpers::encode_syscall_return_trd104(
+            &kernel::utilities::arch_helpers::TRD104SyscallReturn::from_syscall_return(
+                return_value,
+            ),
             &mut a0slice[0],
             &mut a1slice[0],
             &mut a2slice[0],
@@ -195,7 +198,7 @@ impl kernel::syscall::UserspaceKernelBoundary for SysCall {
         state.regs[R_A0] = callback.argument0 as u32;
         state.regs[R_A1] = callback.argument1 as u32;
         state.regs[R_A2] = callback.argument2 as u32;
-        state.regs[R_A3] = callback.argument3 as u32;
+        state.regs[R_A3] = callback.argument3.as_ptr::<()>() as usize as u32;
 
         // We also need to set the return address (ra) register so that the new
         // function that the process is running returns to the correct location.
@@ -206,7 +209,7 @@ impl kernel::syscall::UserspaceKernelBoundary for SysCall {
         state.regs[R_RA] = state.pc;
 
         // Save the PC we expect to execute.
-        state.pc = callback.pc as u32;
+        state.pc = usize::from(callback.pc) as u32;
 
         Ok(())
     }
@@ -631,9 +634,9 @@ impl kernel::syscall::UserspaceKernelBoundary for SysCall {
                         let syscall = kernel::syscall::Syscall::from_register_arguments(
                             state.regs[R_A4] as u8,
                             state.regs[R_A0] as usize,
-                            state.regs[R_A1] as usize,
-                            state.regs[R_A2] as usize,
-                            state.regs[R_A3] as usize,
+                            (state.regs[R_A1] as usize).into(),
+                            (state.regs[R_A2] as usize).into(),
+                            (state.regs[R_A3] as usize).into(),
                         );
 
                         match syscall {

diff --git a/kernel/src/grant.rs b/kernel/src/grant.rs
@@ -138,6 +138,7 @@ use crate::process::{Error, Process, ProcessCustomGrantIdentifier, ProcessId};
 use crate::processbuffer::{ReadOnlyProcessBuffer, ReadWriteProcessBuffer};
 use crate::processbuffer::{ReadOnlyProcessBufferRef, ReadWriteProcessBufferRef};
 use crate::upcall::{Upcall, UpcallError, UpcallId};
+use crate::utilities::capability_ptr::CapabilityPtr;
 use crate::ErrorCode;
 
 /// Tracks how many upcalls a grant instance supports automatically.
@@ -707,8 +708,8 @@ impl<'a> GrantKernelData<'a> {
 #[repr(C)]
 #[derive(Default)]
 struct SavedUpcall {
-    appdata: usize,
-    fn_ptr: Option<NonNull<()>>,
+    appdata: CapabilityPtr,
+    fn_ptr: CapabilityPtr,
 }
 
 /// A minimal representation of a read-only allow from app, used for storing a

diff --git a/kernel/src/kernel.rs b/kernel/src/kernel.rs
@@ -9,7 +9,6 @@
 //! etc.) is defined in the `scheduler` subcrate and selected by a board.
 
 use core::cell::Cell;
-use core::ptr::NonNull;
 
 use crate::capabilities;
 use crate::config;
@@ -877,15 +876,31 @@ impl Kernel {
                             subscribe_num: subdriver_number,
                         };
 
-                        // First check if `upcall_ptr` is null. A null
-                        // `upcall_ptr` will result in `None` here and
-                        // represents the special "unsubscribe" operation.
-                        let ptr = NonNull::new(upcall_ptr);
+                        // TODO: when the compiler supports capability types
+                        // bring this back as a NonNull
+                        // type. https://github.com/tock/tock/issues/4134.
+                        //
+                        // Previously, we had a NonNull type (that had a niche)
+                        // here, and could wrap that in Option to fill the niche
+                        // and handle the Null case. CapabilityPtr is filling
+                        // the gap left by * const(), which does not have the
+                        // niche and allows NULL internally. Having a CHERI
+                        // capability type with a niche is (maybe?) predicated
+                        // on having better compiler support.
+                        // Option<NonNull<()>> is preferable here, and it should
+                        // go back to it just as soon as we can express "non
+                        // null capability". For now, checking for the null case
+                        // is handled internally in each `map_or` call.
+                        //
+                        //First check if `upcall_ptr` is null. A null
+                        //`upcall_ptr` will result in `None` here and
+                        //represents the special "unsubscribe" operation.
+                        //let ptr = NonNull::new(upcall_ptr);
 
                         // For convenience create an `Upcall` type now. This is
                         // just a data structure and doesn't do any checking or
                         // conversion.
-                        let upcall = Upcall::new(process.processid(), upcall_id, appdata, ptr);
+                        let upcall = Upcall::new(process.processid(), upcall_id, appdata, upcall_ptr);
 
                         // If `ptr` is not null, we must first verify that the
                         // upcall function pointer is within process accessible
@@ -895,8 +910,8 @@ impl Kernel {
                         // > process executable memory...), the kernel...MUST
                         // > immediately return a failure with a error code of
                         // > `INVALID`.
-                        let rval1 = ptr.and_then(|upcall_ptr_nonnull| {
-                            if !process.is_valid_upcall_function_pointer(upcall_ptr_nonnull) {
+                        let rval1 = upcall_ptr.map_or(None, |upcall_ptr_nonnull| {
+                            if !process.is_valid_upcall_function_pointer(upcall_ptr_nonnull.as_ptr()) {
                                 Some(ErrorCode::INVAL)
                             } else {
                                 None
@@ -1010,7 +1025,7 @@ impl Kernel {
                                 process.processid(),
                                 driver_number,
                                 subdriver_number,
-                                upcall_ptr as usize,
+                                upcall_ptr,
                                 appdata,
                                 rval
                             );

diff --git a/kernel/src/memop.rs b/kernel/src/memop.rs
@@ -6,6 +6,7 @@
 
 use crate::process::Process;
 use crate::syscall::SyscallReturn;
+use crate::utilities::capability_ptr::{CapabilityPtr, CapabilityPtrPermissions};
 use crate::ErrorCode;
 
 /// Handle the `memop` syscall.
@@ -52,48 +53,94 @@ pub(crate) fn memop(process: &dyn Process, op_type: usize, r1: usize) -> Syscall
         // Op Type 1: SBRK
         1 => process
             .sbrk(r1 as isize)
-            .map(|addr| SyscallReturn::SuccessU32(addr as u32))
+            .map(|addr| SyscallReturn::SuccessPtr(addr))
             .unwrap_or(SyscallReturn::Failure(ErrorCode::NOMEM)),
 
         // Op Type 2: Process memory start
-        2 => SyscallReturn::SuccessU32(process.get_addresses().sram_start as u32),
+        2 => SyscallReturn::SuccessPtr(unsafe {
+            let addresses = process.get_addresses();
+            CapabilityPtr::new_with_authority(
+                addresses.sram_start as *const _,
+                addresses.sram_start,
+                addresses.sram_app_brk - addresses.sram_start,
+                CapabilityPtrPermissions::ReadWrite,
+            )
+        }),
 
         // Op Type 3: Process memory end
-        3 => SyscallReturn::SuccessU32(process.get_addresses().sram_end as u32),
+        3 => SyscallReturn::SuccessPtr(unsafe {
+            let addresses = process.get_addresses();
+            CapabilityPtr::new_with_authority(
+                addresses.sram_end as *const _,
+                addresses.sram_start,
+                addresses.sram_end - addresses.sram_start,
+                CapabilityPtrPermissions::ReadWrite,
+            )
+        }),
 
         // Op Type 4: Process flash start
-        4 => SyscallReturn::SuccessU32(process.get_addresses().flash_start as u32),
+        4 => SyscallReturn::SuccessPtr(unsafe {
+            let addresses = process.get_addresses();
+            CapabilityPtr::new_with_authority(
+                addresses.flash_start as *const _,
+                addresses.flash_start,
+                addresses.flash_end - addresses.flash_start,
+                CapabilityPtrPermissions::Execute,
+            )
+        }),
 
         // Op Type 5: Process flash end
-        5 => SyscallReturn::SuccessU32(process.get_addresses().flash_end as u32),
+        5 => SyscallReturn::SuccessPtr(unsafe {
+            let addresses = process.get_addresses();
+            CapabilityPtr::new_with_authority(
+                addresses.flash_end as *const _,
+                addresses.flash_start,
+                addresses.flash_end - addresses.flash_start,
+                CapabilityPtrPermissions::Execute,
+            )
+        }),
 
         // Op Type 6: Grant region begin
-        6 => SyscallReturn::SuccessU32(process.get_addresses().sram_grant_start as u32),
+        6 => SyscallReturn::SuccessAddr(process.get_addresses().sram_grant_start),
 
         // Op Type 7: Number of defined writeable regions in the TBF header.
         7 => SyscallReturn::SuccessU32(process.number_writeable_flash_regions() as u32),
 
         // Op Type 8: The start address of the writeable region indexed by r1.
         8 => {
-            let flash_start = process.get_addresses().flash_start as u32;
+            let flash_start = process.get_addresses().flash_start;
             let (offset, size) = process.get_writeable_flash_region(r1);
             if size == 0 {
                 SyscallReturn::Failure(ErrorCode::FAIL)
             } else {
-                SyscallReturn::SuccessU32(flash_start + offset)
+                SyscallReturn::SuccessPtr(unsafe {
+                    CapabilityPtr::new_with_authority(
+                        (flash_start + offset) as *const _,
+                        flash_start + offset,
+                        size,
+                        CapabilityPtrPermissions::ReadWrite,
+                    )
+                })
             }
         }
 
         // Op Type 9: The end address of the writeable region indexed by r1.
         // Returns (void*) -1 on failure, meaning the selected writeable region
         // does not exist.
         9 => {
-            let flash_start = process.get_addresses().flash_start as u32;
+            let flash_start = process.get_addresses().flash_start;
             let (offset, size) = process.get_writeable_flash_region(r1);
             if size == 0 {
                 SyscallReturn::Failure(ErrorCode::FAIL)
             } else {
-                SyscallReturn::SuccessU32(flash_start + offset + size)
+                SyscallReturn::SuccessPtr(unsafe {
+                    CapabilityPtr::new_with_authority(
+                        (flash_start + offset + size) as *const _,
+                        flash_start + offset,
+                        size,
+                        CapabilityPtrPermissions::ReadWrite,
+                    )
+                })
             }
         }
 

diff --git a/kernel/src/process.rs b/kernel/src/process.rs
@@ -19,6 +19,7 @@ use crate::processbuffer::{ReadOnlyProcessBuffer, ReadWriteProcessBuffer};
 use crate::storage_permissions;
 use crate::syscall::{self, Syscall, SyscallReturn};
 use crate::upcall::UpcallId;
+use crate::utilities::capability_ptr::CapabilityPtr;
 use tock_tbf::types::CommandPermissions;
 
 // Export all process related types via `kernel::process::`.
@@ -528,7 +529,8 @@ pub trait Process {
     ///
     /// ## Returns
     ///
-    /// On success, return the previous break address.
+    /// On success, return the previous break address with authority that
+    /// has RW permissions from the start of process RAM to the new break.
     ///
     /// On error, return:
     /// - [`Error::InactiveApp`] if the process is not running and adjusting the
@@ -540,15 +542,16 @@ pub trait Process {
     ///   process's memory region.
     /// - [`Error::KernelError`] if there was an internal kernel error. This is
     ///   a bug.
-    fn brk(&self, new_break: *const u8) -> Result<*const u8, Error>;
+    fn brk(&self, new_break: *const u8) -> Result<CapabilityPtr, Error>;
 
     /// Change the location of the program break by `increment` bytes,
     /// reallocate the MPU region covering program memory, and return the
     /// previous break address.
     ///
     /// ## Returns
     ///
-    /// On success, return the previous break address.
+    /// On success, return the previous break address with authority that
+    /// has RW permissions from the start of process RAM to the new break.
     ///
     /// On error, return:
     /// - [`Error::InactiveApp`] if the process is not running and adjusting the
@@ -560,7 +563,7 @@ pub trait Process {
     ///   process's memory region.
     /// - [`Error::KernelError`] if there was an internal kernel error. This is
     ///   a bug.
-    fn sbrk(&self, increment: isize) -> Result<*const u8, Error>;
+    fn sbrk(&self, increment: isize) -> Result<CapabilityPtr, Error>;
 
     /// How many writeable flash regions defined in the TBF header for this
     /// process.
@@ -575,10 +578,10 @@ pub trait Process {
     ///
     /// ## Returns
     ///
-    /// A tuple containing the a `u32` of the offset from the beginning of the
-    /// process's flash region where the writeable region starts and a `u32` of
+    /// A tuple containing the a `usize` of the offset from the beginning of the
+    /// process's flash region where the writeable region starts and a `usize` of
     /// the size of the region in bytes.
-    fn get_writeable_flash_region(&self, region_index: usize) -> (u32, u32);
+    fn get_writeable_flash_region(&self, region_index: usize) -> (usize, usize);
 
     /// Debug function to update the kernel on where the stack starts for this
     /// process. Processes are not required to call this through the memop
@@ -791,7 +794,9 @@ pub trait Process {
     ///
     /// Returns `true` if the upcall function pointer is valid for this process,
     /// and `false` otherwise.
-    fn is_valid_upcall_function_pointer(&self, upcall_fn: NonNull<()>) -> bool;
+    // `upcall_fn` can eventually be a better type:
+    // <https://github.com/tock/tock/issues/4134>
+    fn is_valid_upcall_function_pointer(&self, upcall_fn: *const ()) -> bool;
 
     // functions for processes that are architecture specific
 
@@ -1078,10 +1083,10 @@ pub struct FunctionCall {
     pub argument1: usize,
     /// The third argument to the function.
     pub argument2: usize,
-    /// The fourth argument to the function.
-    pub argument3: usize,
+    /// The userdata provided by the process via `subscribe`
+    pub argument3: CapabilityPtr,
     /// The PC of the function to execute.
-    pub pc: usize,
+    pub pc: CapabilityPtr,
 }
 
 /// This is similar to `FunctionCall` but for the special case of the Null