kernel/arch-riscv: fix unsound usize->u32/u64 syscall register reinterpretation

[ppannuto]

Pull Request Overview

Our current pointer casts are unsound. The conjured &mut u32 has no lifetime tied back to a0, so the compiler still allows using a0 again while a0_u32 is alive:

let a0_u32 = &mut *core::ptr::from_mut::<usize>(a0).cast::<u32>();
*a0 = 1;   // compiles today; a0_u32 still holds a live reference into the same slot

This PR extracts the cast into kernel::utilities::helpers::usize_as_native_mut, a plain function. A function's lifetime elision (fn usize_as_native_mut<'a>(val: &'a mut usize) -> &'a mut u32) ties the output to the input at every call site — neither an inline block nor a macro provides this, since neither introduces a real function-call boundary.

With the fix, the *a0 = 1; above (correctly) fails to compile with a lifetime violation.

Testing Strategy

• make -C boards/qemu_rv32_virt — full board build, riscv32 path.
• cargo check --target riscv64imac-unknown-none-elf against kernel and arch/riscv directly — riscv64 path (no riscv64 board in-tree yet).
• Verified the repro: *a0 = 1; inserted after the conversion compiles on unpatched code, fails with E0506 after this fix.

TODO or Help Wanted

N/A

Checklist

• Ran make prepush.

PR Contents

Documentation

• No documentation updates are required.

AI Use

• No AI was used in this PR.
• AI was used in this PR. I have read Tock's AI policy and have properly disclosed my AI use below.

Prompts used across this PR (review, fix, and this update)

❯ Perform a code review on PR 4964. Pay particular attention to (1) the
  soundness of the new macros, (2) whether there is overlap / redundancy
  with existing Rust primitives that should be used instead, (3) whether
  there are other places in the codebase that should be converted to use
  these macros not covered by the current PR

❯ Fix #1, but pause before committing for me to review

❯ create a new branch, based off of PR 4964 tip; we will open a new PR
  that targets current PR 4964 as the merge base
  [...]

❯ Brad closed PR 4964 as the macros did not fix the soundness issue. But
  there is a real soundness concern it flagged. Review the conversation
  on PR 5070 [sic, 5057]. Then make a plan to update PR 5070 based on my
  most recent comment in the thread.

❯ Plan changes:
  1. target origin/master not local master
  2. good as-is
  3. good as-is
  NEW STEP HERE: Search for other places in the existing codebase where
  these helpers should be used. Pause to discuss findings.
  4. The proposed work is good. Plan to pause and iterate with me on the
     exact text.
  5. The proposed work is good. Plan to pause and iterate with me on the
     exact text.
  6. The should move before step 4, verify everything before we author
     PR and comment text

❯ Go ahead and draft the PR body

I have manually reviewed the diff.

[bradjc]

This is the same rust-unsafe-is-broken-in-macros issue isn't it. Needs #allow(unsafe) so this goes in an unsafe block. Ugh.

[bradjc]

My protest is the macro seems to have nothing to do with this, ie, if I add

fn get_dangling2() -> &'static mut u64 {
    let mut x: usize = 5;
    let r: &mut usize = &mut x;
    let ptr = core::ptr::from_mut::<usize>(r);
    let out: &mut u64 = unsafe { &mut *ptr.cast::<u64>() };
    out
} 

fn main() {
    let r = get_dangling();
    *r = 0xdeadbeef;
    println!("{}", r);       // prints the freed stack slot's leftover value
    let s = get_dangling2();
    *s = 0xdeadbefa;
    println!("{}", s);
}

to the playground link the output is still

3735928559
3735928570

Maybe the function is a good fix, maybe zerocopy is a good fix, maybe leaving it alone is a good fix, maybe making the macro unsafe is a good fix.

[ppannuto]

The problem with your example there (and any macro approach in general) is that this line:

    let out: &mut u64 = unsafe { &mut *ptr.cast::<u64>() };

is located in the get_dangling() function. The reason that converting to

fn usize_as_native_mut_fn(val: &mut usize) -> &mut u64 { ...

works is because now you have to pass the thing you wanted to convert into a function that consumes it mutably. Don't get caught up in where the unsafe keyword lands. The reason that the unsafe function can uphold its soundness (while an unsafe block cannot) is because the dedicated function never references the thing it (unsafely) converted again [even though it could without raising a compiler error].

Rust lifetime rules around blocks (i.e., braced chunks of code in a function) are not the same as the rules around function boundaries. You need the function boundary for the lifetime tracking to do what you want w.r.t. the unsafe reference that the conversion creates.

[bradjc]

What do you mean by the problem with my example? That is the code we already have. Adding a macro isn't making this any worse.

And the exact same unsafe requirements that exist for the underlying Rust code can be added to the macro, because it is the same code.

Function vs. block boundaries is orthogonal. The function might make for a better conversion tool, I'm not arguing that. But we can always leave the code exactly the way it is, which could exhibit this behavior.

[ppannuto]

True, making is a macro doesn't make it any worse, it just doesn't fix the problem either. Your original PR did identify a real soundness issue in our current codebase, i.e.

[-bash] Thu 13 Aug 09:59 [[master $] ~/code/helena-project/tock]
$ git diff
diff --git a/arch/riscv/src/syscall.rs b/arch/riscv/src/syscall.rs
index fe10dff71..097cb9a77 100644
--- a/arch/riscv/src/syscall.rs
+++ b/arch/riscv/src/syscall.rs
@@ -137,6 +137,8 @@ fn encode_syscall_return_helper(
         (a0_u32, a1_u32, a2_u32, a3_u32)
     };

+    *a0 = 1;
+
     kernel::utilities::arch_helpers::encode_syscall_return_trd104(
         &kernel::utilities::arch_helpers::TRD104SyscallReturn::from_syscall_return(return_value),
         a0_u32,
$ # This shouldn't compile, but it does. The safety comment on line 129 is insufficient for the unsafe operation in the block it's documenting
$ make -C boards/qemu_rv32_virt/
    Finished `release` profile [optimized + debuginfo] target(s) in 0.17s
   text	   data	    bss	    dec	    hex	filename
 103980	     16	 109832	 213828	  34344	/Users/ppannuto/code/helena-project/tock/target/riscv32imac-unknown-none-elf/release/qemu_rv32_virt
c4177eeafba335662960c5f7898b7992bf614ac8d2ca9a8b1383ea19aec8ef0a  /Users/ppannuto/code/helena-project/tock/target/riscv32imac-unknown-none-elf/release/qemu_rv32_virt.bin

I can update this PR to capture the soundness fixes with these functions and target master.

[ppannuto]

Superseded by #5075 -- opening fresh since retargeting this PR's base confused GitHub's merge-base tracking.