From 6a6be4ab3673487f5d831faf280923849f5aff6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:02:28 +0300 Subject: [PATCH 01/10] Add no-panic check --- .github/workflows/nopanic.yaml | 58 ++++++++++++++++++++++++++++++++++ nopanic_check/Cargo.toml | 16 ++++++++++ nopanic_check/src/lib.rs | 18 +++++++++++ 3 files changed, 92 insertions(+) create mode 100644 .github/workflows/nopanic.yaml create mode 100644 nopanic_check/Cargo.toml create mode 100644 nopanic_check/src/lib.rs diff --git a/.github/workflows/nopanic.yaml b/.github/workflows/nopanic.yaml new file mode 100644 index 00000000..c0c13b80 --- /dev/null +++ b/.github/workflows/nopanic.yaml @@ -0,0 +1,58 @@ +name: No panic + +on: + push: + branches: master + pull_request: + branches: master + +permissions: + contents: read + +defaults: + run: + working-directory: nopanic_check + +env: + RUSTFLAGS: "-Dwarnings" + +jobs: + linux: + name: No panic (Linux) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@master + with: + # We need Nightly for the rust-std component for wasm32-wasip2 + toolchain: nightly-2024-10-14 + targets: wasm32-wasip1, wasm32-wasip2 + + - name: Build (linux_android_with_fallback.rs) + run: cargo build --release + - name: Check (linux_android_with_fallback.rs) + run: ret=$(grep panic target/release/libgetrandom_wrapper.so; echo $?); [ $ret -eq 1 ] + + - name: Build (linux_android.rs) + env: + RUSTFLAGS: -Dwarnings --cfg getrandom_backend="linux_getrandom" + run: cargo build --release + - name: Check (linux_android.rs) + run: ret=$(grep panic target/release/libgetrandom_wrapper.so; echo $?); [ $ret -eq 1 ] + + - name: Build (rdrand.rs) + env: + RUSTFLAGS: -Dwarnings --cfg getrandom_backend="rdrand" + run: cargo build --release + - name: Check (rdrand.rs) + run: ret=$(grep panic target/release/libgetrandom_wrapper.so; echo $?); [ $ret -eq 1 ] + + - name: Build (wasi.rs, preview 1) + run: cargo build --release --target wasm32-wasip1 + - name: Check (wasi.rs, preview 1) + run: ret=$(grep panic target/wasm32-wasip1/release/getrandom_wrapper.wasm; echo $?); [ $ret -eq 1 ] + + - name: Build (wasi.rs, preview 2) + run: cargo build --release --target wasm32-wasip2 + - name: Check (wasi.rs, preview 2) + run: ret=$(grep panic target/wasm32-wasip2/release/getrandom_wrapper.wasm; echo $?); [ $ret -eq 1 ] diff --git a/nopanic_check/Cargo.toml b/nopanic_check/Cargo.toml new file mode 100644 index 00000000..11b51024 --- /dev/null +++ b/nopanic_check/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "nopanic_check" +description = "Helper crate for checking that getrandom implementation does not contain potential panics" +version = "0.1.0" +edition = "2021" +publish = false + +[lib] +name = "getrandom_wrapper" +crate-type = ["cdylib"] + +[dependencies] +getrandom = { path = ".." } + +[profile.release] +panic = "abort" diff --git a/nopanic_check/src/lib.rs b/nopanic_check/src/lib.rs new file mode 100644 index 00000000..0c0a7a0d --- /dev/null +++ b/nopanic_check/src/lib.rs @@ -0,0 +1,18 @@ +// WASI preview 2 requires enabled std +#![cfg_attr(not(all(target_arch = "wasm32", target_env = "p2")), no_std)] + +#[cfg(not(any(test, all(target_arch = "wasm32", target_env = "p2"))))] +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + extern "C" { + fn panic_nonexistent() -> !; + } + unsafe { panic_nonexistent() } +} + +#[no_mangle] +pub extern "C" fn getrandom_wrapper(buf_ptr: *mut u8, buf_len: usize) -> u32 { + let buf = unsafe { core::slice::from_raw_parts_mut(buf_ptr.cast(), buf_len) }; + let res = getrandom::getrandom_uninit(buf).map(|_| ()); + unsafe { core::mem::transmute(res) } +} From fe7b169bb878de7c16b22d91f12e04afb28b572c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:04:34 +0300 Subject: [PATCH 02/10] Add panic to `linux_android_with_fallback` --- src/linux_android_with_fallback.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/linux_android_with_fallback.rs b/src/linux_android_with_fallback.rs index ec7a1216..840ce9e9 100644 --- a/src/linux_android_with_fallback.rs +++ b/src/linux_android_with_fallback.rs @@ -29,6 +29,7 @@ fn is_getrandom_available() -> bool { // on Android. See https://github.com/rust-random/getrandom/issues/229. #[cfg(target_os = "linux")] Some(libc::EPERM) => false, // Blocked by seccomp + Some(42) => panic!(), _ => true, } } else { From 7e96bbdb71d5567a14ba3a83888fb35cd9a112cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:05:29 +0300 Subject: [PATCH 03/10] Add panic to `linux_android` --- src/linux_android.rs | 4 ++++ src/linux_android_with_fallback.rs | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/linux_android.rs b/src/linux_android.rs index 35c368a5..f103a9d0 100644 --- a/src/linux_android.rs +++ b/src/linux_android.rs @@ -19,6 +19,10 @@ pub fn getrandom_syscall(buf: &mut [MaybeUninit]) -> libc::ssize_t { ) }; + if res == 42 { + panic!(); + } + const _: () = assert!(core::mem::size_of::() == core::mem::size_of::()); res.try_into() diff --git a/src/linux_android_with_fallback.rs b/src/linux_android_with_fallback.rs index 840ce9e9..ec7a1216 100644 --- a/src/linux_android_with_fallback.rs +++ b/src/linux_android_with_fallback.rs @@ -29,7 +29,6 @@ fn is_getrandom_available() -> bool { // on Android. See https://github.com/rust-random/getrandom/issues/229. #[cfg(target_os = "linux")] Some(libc::EPERM) => false, // Blocked by seccomp - Some(42) => panic!(), _ => true, } } else { From f6eed09dbb93448380c6f2d99b2e59ef63cfa579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:06:41 +0300 Subject: [PATCH 04/10] Add panic to `rdrand` --- src/linux_android.rs | 4 ---- src/rdrand.rs | 3 +++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/linux_android.rs b/src/linux_android.rs index f103a9d0..35c368a5 100644 --- a/src/linux_android.rs +++ b/src/linux_android.rs @@ -19,10 +19,6 @@ pub fn getrandom_syscall(buf: &mut [MaybeUninit]) -> libc::ssize_t { ) }; - if res == 42 { - panic!(); - } - const _: () = assert!(core::mem::size_of::() == core::mem::size_of::()); res.try_into() diff --git a/src/rdrand.rs b/src/rdrand.rs index 68f9878c..c9a8746d 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -27,6 +27,9 @@ unsafe fn rdrand() -> Option { for _ in 0..RETRY_LIMIT { let mut val = 0; if rdrand_step(&mut val) == 1 { + if val = 42 { + panic!(); + } return Some(val); } } From 9ff58a482d978aa8bc9f23453c00f3d5506f8d47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:08:27 +0300 Subject: [PATCH 05/10] Add panic to `wasi-p1` --- src/rdrand.rs | 3 --- src/wasi.rs | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index c9a8746d..68f9878c 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -27,9 +27,6 @@ unsafe fn rdrand() -> Option { for _ in 0..RETRY_LIMIT { let mut val = 0; if rdrand_step(&mut val) == 1 { - if val = 42 { - panic!(); - } return Some(val); } } diff --git a/src/wasi.rs b/src/wasi.rs index 9df9bf26..a57290b5 100644 --- a/src/wasi.rs +++ b/src/wasi.rs @@ -28,6 +28,7 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { let ret = unsafe { random_get(dest.as_mut_ptr() as i32, dest.len() as i32) }; match ret { 0 => Ok(()), + 42 => panic!(), code => { let err = u32::try_from(code) .map(Error::from_os_error) From 372ac2eab215590bd51d2c50293120845bbfe8e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:09:37 +0300 Subject: [PATCH 06/10] Add panic to `wasi-p2` --- src/wasi.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wasi.rs b/src/wasi.rs index a57290b5..a13a83f0 100644 --- a/src/wasi.rs +++ b/src/wasi.rs @@ -28,7 +28,6 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { let ret = unsafe { random_get(dest.as_mut_ptr() as i32, dest.len() as i32) }; match ret { 0 => Ok(()), - 42 => panic!(), code => { let err = u32::try_from(code) .map(Error::from_os_error) @@ -52,6 +51,9 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { // [0]: https://github.com/WebAssembly/wasi-random/issues/27 if !prefix.is_empty() { let val = get_random_u64(); + if val == 42 { + panic!(); + } let src = (&val as *const u64).cast(); unsafe { copy_nonoverlapping(src, prefix.as_mut_ptr(), prefix.len()); From 019a1e580ed6fa8ed9635e4e254076c47a1b3479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:10:25 +0300 Subject: [PATCH 07/10] Add panic to `rdrand` --- src/rdrand.rs | 3 +++ src/wasi.rs | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 68f9878c..3845d84f 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -27,6 +27,9 @@ unsafe fn rdrand() -> Option { for _ in 0..RETRY_LIMIT { let mut val = 0; if rdrand_step(&mut val) == 1 { + if val == 42 { + panic!(); + } return Some(val); } } diff --git a/src/wasi.rs b/src/wasi.rs index a13a83f0..9df9bf26 100644 --- a/src/wasi.rs +++ b/src/wasi.rs @@ -51,9 +51,6 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { // [0]: https://github.com/WebAssembly/wasi-random/issues/27 if !prefix.is_empty() { let val = get_random_u64(); - if val == 42 { - panic!(); - } let src = (&val as *const u64).cast(); unsafe { copy_nonoverlapping(src, prefix.as_mut_ptr(), prefix.len()); From 37cf0e2d69e6317d3aecd2648cee5c4b7bb6ad09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:10:56 +0300 Subject: [PATCH 08/10] Remove panics --- src/rdrand.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 3845d84f..68f9878c 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -27,9 +27,6 @@ unsafe fn rdrand() -> Option { for _ in 0..RETRY_LIMIT { let mut val = 0; if rdrand_step(&mut val) == 1 { - if val == 42 { - panic!(); - } return Some(val); } } From a7c8155e548c8f64cbc58b7f553070a582ab901c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:19:54 +0300 Subject: [PATCH 09/10] Add doc section about panic handling --- src/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 005c7a75..67a4addc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -195,6 +195,15 @@ //! on every call to `getrandom`, hence after the first successful call one //! can be reasonably confident that no errors will occur. //! +//! ## Panic handling +//! +//! We strive to eliminate all potential panics from our implementation. +//! In other words, when compiled with enabled optimizations, generated +//! binary code for `getrandom` functions should not contain any panic +//! branches. Even if platform misbiheaves and returns an unexpected +//! result, our code should correctly handle it and return an error like +//! [`Error::UNEXPECTED`]. +//! //! [1]: https://manned.org/getrandom.2 //! [2]: https://manned.org/urandom.4 //! [3]: https://www.unix.com/man-page/mojave/2/getentropy/ From feceebf3f9a443ed3077a50562824102cffb766b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 14 Oct 2024 19:43:19 +0300 Subject: [PATCH 10/10] Add macOS job --- .github/workflows/nopanic.yaml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nopanic.yaml b/.github/workflows/nopanic.yaml index c0c13b80..a4c24173 100644 --- a/.github/workflows/nopanic.yaml +++ b/.github/workflows/nopanic.yaml @@ -18,7 +18,7 @@ env: jobs: linux: - name: No panic (Linux) + name: Linux runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -56,3 +56,17 @@ jobs: run: cargo build --release --target wasm32-wasip2 - name: Check (wasi.rs, preview 2) run: ret=$(grep panic target/wasm32-wasip2/release/getrandom_wrapper.wasm; echo $?); [ $ret -eq 1 ] + + macos: + name: macOS + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: stable + + - name: Build (getentropy.rs) + run: cargo build --release + - name: Check (getentropy.rs) + run: ret=$(grep panic target/release/libgetrandom_wrapper.so; echo $?); [ $ret -eq 1 ]