use globally shared pipe to validate memory (#198)

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

.github/workflows/rust.yml

@@ -19,7 +19,7 @@ jobs:
         uses: actions-rs/toolchain@v1.0.7
         with:
           profile: minimal
-          toolchain: stable
+          toolchain: 1.56.1
           override: true
           components: rustfmt, clippy
 
@@ -35,21 +35,29 @@ jobs:
       - name: Remove pre-generated prost files to force regeneration
         run: rm proto/*.rs
 
+      - name: Specify dependency version for old Rust
+        run: |
+          cp Cargo.toml Cargo.toml.bak
+          sed -i "s|inferno = { version = \"0.11\"|inferno = { version = \"=0.11.1\"|g" ./Cargo.toml
+          sed -i "s|criterion = \"0.4\"|criterion = \"=0.3.0\"\ncsv = \"=1.1.0\"|g" Cargo.toml
+
       - name: Run cargo clippy prost
         uses: actions-rs/cargo@v1.0.3
         with:
           command: clippy
           args: --all-targets --features flamegraph,prost-codec -- -D warnings
 
-      - name: Check if the prost file committed to git is up-to-date
-        run: git diff --no-ext-diff --exit-code
-
       - name: Run cargo clippy protobuf
         uses: actions-rs/cargo@v1.0.3
         with:
           command: clippy
           args: --all-targets --features flamegraph,protobuf-codec -- -D warnings
 
+      - name: Check if the prost file committed to git is up-to-date
+        run: |
+          mv Cargo.toml.bak Cargo.toml
+          git diff --no-ext-diff --exit-code
+
   build:
     name: Build
     strategy:
@@ -74,6 +82,7 @@ jobs:
             target: aarch64-unknown-linux-gnu
           - os: macos-latest
             target: x86_64-unknown-linux-musl
+      fail-fast: false
     runs-on: ${{ matrix.os }}
 
     steps:
@@ -88,6 +97,13 @@ jobs:
           target: ${{ matrix.target }}
           override: true
 
+      - name: Specify dependency version for old Rust
+        if: ${{ matrix.toolchain == '1.56.1' }}
+        run: |
+          # sed -i is not compatible with Mac OS
+          mv Cargo.toml Cargo.toml.bak
+          sed "s|inferno = { version = \"0.11\"|inferno = { version = \"=0.11.1\"|g" Cargo.toml.bak > ./Cargo.toml
+
       - name: Run cargo build prost
         uses: actions-rs/cargo@v1.0.3
         with:
@@ -125,6 +141,7 @@ jobs:
             target: x86_64-unknown-linux-gnu
           - os: macos-latest
             target: x86_64-unknown-linux-musl
+      fail-fast: false
     runs-on: ${{ matrix.os }}
 
     steps:

src/addr_validate.rs

@@ -1,14 +1,23 @@
-use std::{cell::RefCell, mem::size_of};
+use std::{
+    mem::size_of,
+    sync::atomic::{AtomicI32, Ordering},
+};
 
 use nix::{
     errno::Errno,
     unistd::{close, read, write},
 };
 
-thread_local! {
-    static MEM_VALIDATE_PIPE: RefCell<[i32; 2]> = RefCell::new([-1, -1]);
+struct Pipes {
+    read_fd: AtomicI32,
+    write_fd: AtomicI32,
 }
 
+static MEM_VALIDATE_PIPE: Pipes = Pipes {
+    read_fd: AtomicI32::new(-1),
+    write_fd: AtomicI32::new(-1),
+};
+
 #[inline]
 #[cfg(target_os = "linux")]
 fn create_pipe() -> nix::Result<(i32, i32)> {
@@ -42,56 +51,53 @@ fn create_pipe() -> nix::Result<(i32, i32)> {
 }
 
 fn open_pipe() -> nix::Result<()> {
-    MEM_VALIDATE_PIPE.with(|pipes| {
-        let mut pipes = pipes.borrow_mut();
+    // ignore the result
+    let _ = close(MEM_VALIDATE_PIPE.read_fd.load(Ordering::SeqCst));
+    let _ = close(MEM_VALIDATE_PIPE.write_fd.load(Ordering::SeqCst));
 
-        // ignore the result
-        let _ = close(pipes[0]);
-        let _ = close(pipes[1]);
+    let (read_fd, write_fd) = create_pipe()?;
 
-        let (read_fd, write_fd) = create_pipe()?;
+    MEM_VALIDATE_PIPE.read_fd.store(read_fd, Ordering::SeqCst);
+    MEM_VALIDATE_PIPE.write_fd.store(write_fd, Ordering::SeqCst);
 
-        pipes[0] = read_fd;
-        pipes[1] = write_fd;
-
-        Ok(())
-    })
+    Ok(())
 }
 
+// validate whether the address `addr` is readable through `write()` to a pipe
+//
+// if the second argument of `write(ptr, buf)` is not a valid address, the
+// `write()` will return an error the error number should be `EFAULT` in most
+// cases, but we regard all errors (except EINTR) as a failure of validation
 pub fn validate(addr: *const libc::c_void) -> bool {
     const CHECK_LENGTH: usize = 2 * size_of::<*const libc::c_void>() / size_of::<u8>();
 
     // read data in the pipe
-    let valid_read = MEM_VALIDATE_PIPE.with(|pipes| {
-        let pipes = pipes.borrow();
-        loop {
-            let mut buf = [0u8; CHECK_LENGTH];
-
-            match read(pipes[0], &mut buf) {
-                Ok(bytes) => break bytes > 0,
-                Err(_err @ Errno::EINTR) => continue,
-                Err(_err @ Errno::EAGAIN) => break true,
-                Err(_) => break false,
-            }
+    let read_fd = MEM_VALIDATE_PIPE.read_fd.load(Ordering::SeqCst);
+    let valid_read = loop {
+        let mut buf = [0u8; CHECK_LENGTH];
+
+        match read(read_fd, &mut buf) {
+            Ok(bytes) => break bytes > 0,
+            Err(_err @ Errno::EINTR) => continue,
+            Err(_err @ Errno::EAGAIN) => break true,
+            Err(_) => break false,
         }
-    });
+    };
 
     if !valid_read && open_pipe().is_err() {
         return false;
     }
 
-    MEM_VALIDATE_PIPE.with(|pipes| {
-        let pipes = pipes.borrow();
-        loop {
-            let buf = unsafe { std::slice::from_raw_parts(addr as *const u8, CHECK_LENGTH) };
+    let write_fd = MEM_VALIDATE_PIPE.write_fd.load(Ordering::SeqCst);
+    loop {
+        let buf = unsafe { std::slice::from_raw_parts(addr as *const u8, CHECK_LENGTH) };
 
-            match write(pipes[1], buf) {
-                Ok(bytes) => break bytes > 0,
-                Err(_err @ Errno::EINTR) => continue,
-                Err(_) => break false,
-            }
+        match write(write_fd, buf) {
+            Ok(bytes) => break bytes > 0,
+            Err(_err @ Errno::EINTR) => continue,
+            Err(_) => break false,
         }
-    })
+    }
 }
 
 #[cfg(test)]