islet-project / islet

An on-device confidential computing platform
Apache License 2.0
91 stars 16 forks source link

safe-abstraction: Correct the misconception #309

Closed bitboom closed 4 months ago

bitboom commented 4 months ago

This PR

Previous Assumptions (misconception)

The use of closures was initially based on the belief that they helped maintain the context for unsafe code, which was presumed difficult to analyze effectively.

Findings from MIR Analysis

Recent evaluations using MIR-level analysis tools have shown that this tool, MIRI, is capable of effectively analyzing hidden unsafe code, regardless of the use of hidden unsafe code.

Evaluation

fn hidden_unsafe() {
    let handle1 = thread::spawn(move || {
        for _ in 0..1_000_000 {
            let x1 = get_mut_counter();
            *x1 += 1;
        }
    });

    let handle2 = thread::spawn(move || {
        for _ in 0..1_000_000 {
            let x1 = get_mut_counter();
            *x1 += 1;
        }
    });

    handle1.join().unwrap();
    handle2.join().unwrap();

    println!("Final value: {}", get_mut_counter());
}

fn get_mut_counter<'a>() -> &'a mut i32 {
    unsafe {
        let x1 = &COUNTER as *const i32 as usize;
        &mut *(x1 as *mut i32)
    }
}

fn main() {
    unsafe { explicit_unsafe(); }
    hidden_unsafe();
}

MIRI results


error: Undefined Behavior: Data race detected between (1) Write on thread `<unnamed>` and (2) Read on thread `<unnamed>` at alloc1. (2) just happened here
  --> src/main.rs:18:13
   |
18 |             *x1 += 1;
   |             ^^^^^^^^ Data race detected between (1) Write on thread `<unnamed>` and (2) Read on thread `<unnamed>` at alloc1. (2) just happened here
   |
help: and (1) occurred earlier here
  --> src/main.rs:10:13
   |
10 |             *x1 += 1;
   |             ^^^^^^^^
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE (of the first span):
   = note: inside closure at src/main.rs:18:13: 18:21

Decision Rationale

jinbpark commented 4 months ago

does it mean that all the codes, in our repo, that use SafeAbstraction can be investigated through MIRI? right?

bitboom commented 4 months ago

that use

Yes, right!