enarx-archive / sallyport

API for the hypervisor-microkernel boundary
Apache License 2.0
7 stars 6 forks source link

v2: Add guest allocator #29

Closed rvolosatovs closed 2 years ago

rvolosatovs commented 2 years ago

Part of PR series for #17

Closes https://github.com/enarx/sallyport/issues/35

Blocked by ~https://github.com/enarx/sallyport/pull/28~ ~#27 (due to item crate changes)~ #36

haraldh commented 2 years ago

Miri is failing with:

❯ cargo +nightly miri test
    Finished test [unoptimized + debuginfo] target(s) in 0.00s
     Running unittests (target/miri/x86_64-unknown-linux-gnu/debug/deps/sallyport-bed894b0ae9968c3)
warning: unused import: `core::ptr::null_mut`
  --> src/guest/syscall/result/linux/x86_64.rs:63:9
   |
63 |     use core::ptr::null_mut;
   |         ^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

running 5 tests
test guest::syscall::result::linux::x86_64::tests::result ... ok
test guest::syscall::tests::exit ... error: Undefined Behavior: no item granting read access to tag <untagged> at alloc69380+0x48 found in borrow stack.
   --> /home/harald/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:701:9
    |
701 |         copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item granting read access to tag <untagged> at alloc69380+0x48 found in borrow stack.
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

    = note: inside `std::ptr::read::<[usize; 2]>` at /home/harald/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:701:9
    = note: inside `std::ptr::const_ptr::<impl *const [usize; 2]>::read` at /home/harald/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:810:18
note: inside `guest::alloc::output::OutRef::<[usize; 2]>::read::<guest::alloc::phase_alloc::Alloc<guest::alloc::phase_alloc::phase::Collect>>` at src/guest/alloc/output.rs:73:18
   --> src/guest/alloc/output.rs:73:18
    |
73  |         unsafe { self.as_ptr().read() }
    |                  ^^^^^^^^^^^^^^^^^^^^
note: inside `<guest::alloc::syscall::CommittedSyscall<guest::syscall::exit::Exit> as guest::alloc::Collect>::collect::<guest::alloc::phase_alloc::Alloc<guest::alloc::phase_alloc::phase::Collect>>` at src/guest/alloc/syscall.rs:116:36
   --> src/guest/alloc/syscall.rs:116:36
    |
116 |         T::collect(self.committed, self.ret_ref.read(col).into(), col)
    |                                    ^^^^^^^^^^^^^^^^^^^^^^
note: inside `guest::syscall::tests::Call::<guest::syscall::exit::Exit, (), 11_usize>::assert` at src/guest/syscall/tests.rs:55:25
   --> src/guest/syscall/tests.rs:55:25
    |
55  |         let collected = committed.collect(&alloc);
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `guest::syscall::tests::exit` at src/guest/syscall/tests.rs:62:5
   --> src/guest/syscall/tests.rs:62:5
    |
62  | /     Call::new(
63  | |         Exit { status: 2 },
64  | |         [
65  | |             SYSCALL_USIZE_COUNT * size_of::<usize>(),
...   |
79  | |     )
80  | |     .assert()
    | |_____________^
note: inside closure at src/guest/syscall/tests.rs:61:1
   --> src/guest/syscall/tests.rs:61:1
    |
60  |   #[test]
    |   ------- in this procedural macro expansion
61  | / fn exit() {
62  | |     Call::new(
63  | |         Exit { status: 2 },
64  | |         [
...   |
80  | |     .assert()
81  | | }
    | |_^
haraldh commented 2 years ago

Also for miri I had to patch:

diff --git a/src/v2/src/guest/syscall/result/linux/x86_64.rs b/src/v2/src/guest/syscall/result/linux/x86_64.rs
index 9e6ce8c..48005f8 100644
--- a/src/v2/src/guest/syscall/result/linux/x86_64.rs
+++ b/src/v2/src/guest/syscall/result/linux/x86_64.rs
@@ -66,9 +66,14 @@ mod tests {
     fn result() {
         const ERRNO: c_int = libc::EPERM;

+        let mut buf: [u8; 0] = [];
+
         let expected: Result<isize> = unsafe { super::Result::errno_unchecked(ERRNO) };
         assert_eq!(
-            Result::from([unsafe { libc::read(-1, null_mut(), 0) } as usize, 0]),
+            Result::from([
+                unsafe { libc::read(-1, buf.as_mut_ptr() as _, 0) } as usize,
+                0
+            ]),
             expected
         );
rvolosatovs commented 2 years ago

I changed the approach used in the test to prevent miri error regarding the syscall in the test. After spending some time debugging this, it turns out the allocator error was due to the way the mutable references were obtained in the test, changed that and miri now passes!