From 1705a6c45b4a95b72dcc448e03b55971abbcd427 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: Wed, 8 Nov 2023 10:06:33 +0300 Subject: [PATCH 1/3] Improve robustness of the Hermit backend and `sys_fill_exact` --- src/error.rs | 2 ++ src/hermit.rs | 21 +++++++++++++-------- src/util_libc.rs | 21 +++++++++++---------- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/error.rs b/src/error.rs index ab39a3c3..2f66afb2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -35,6 +35,8 @@ impl Error { pub const UNSUPPORTED: Error = internal_error(0); /// The platform-specific `errno` returned a non-positive value. pub const ERRNO_NOT_POSITIVE: Error = internal_error(1); + /// Encountered an unexpected situation which should not happen in practice. + pub const UNEXPECTED: Error = internal_error(2); /// Call to iOS [`SecRandomCopyBytes`](https://developer.apple.com/documentation/security/1399291-secrandomcopybytes) failed. pub const IOS_SEC_RANDOM: Error = internal_error(3); /// Call to Windows [`RtlGenRandom`](https://docs.microsoft.com/en-us/windows/win32/api/ntsecapi/nf-ntsecapi-rtlgenrandom) failed. diff --git a/src/hermit.rs b/src/hermit.rs index 570b03d9..5791ec08 100644 --- a/src/hermit.rs +++ b/src/hermit.rs @@ -1,5 +1,5 @@ use crate::Error; -use core::{cmp::min, mem::MaybeUninit, num::NonZeroU32}; +use core::{convert::TryInto, mem::MaybeUninit, num::NonZeroU32}; extern "C" { fn sys_read_entropy(buffer: *mut u8, length: usize, flags: u32) -> isize; @@ -8,14 +8,19 @@ extern "C" { pub fn getrandom_inner(mut dest: &mut [MaybeUninit]) -> Result<(), Error> { while !dest.is_empty() { let res = unsafe { sys_read_entropy(dest.as_mut_ptr() as *mut u8, dest.len(), 0) }; - if res < 0 { - // SAFETY: all Hermit error codes use i32 under the hood: - // https://github.com/hermitcore/libhermit-rs/blob/master/src/errno.rs - let code = unsafe { NonZeroU32::new_unchecked((-res) as u32) }; - return Err(code.into()); + if res > 0 { + dest = dest.get_mut(res as usize..).ok_or(Error::UNEXPECTED)?; + } else { + // We should not get `res` equal to zero or smaller than `-i32::MAX`. + // If we get such unexpected value after all, we will return `Error::UNEXPECTED`. + let err = res + .checked_neg() + .and_then(|val| val.try_into().ok()) + .and_then(NonZeroU32::new) + .map(Into::into) + .unwrap_or(Error::UNEXPECTED); + return Err(err); } - let len = min(res as usize, dest.len()); - dest = &mut dest[len..]; } Ok(()) } diff --git a/src/util_libc.rs b/src/util_libc.rs index 99bee382..f64cc312 100644 --- a/src/util_libc.rs +++ b/src/util_libc.rs @@ -8,7 +8,6 @@ #![allow(dead_code)] use crate::Error; use core::{ - cmp::min, mem::MaybeUninit, num::NonZeroU32, ptr::NonNull, @@ -70,17 +69,19 @@ pub fn sys_fill_exact( ) -> Result<(), Error> { while !buf.is_empty() { let res = sys_fill(buf); - if res < 0 { - let err = last_os_error(); - // We should try again if the call was interrupted. - if err.raw_os_error() != Some(libc::EINTR) { - return Err(err); + match res { + res if res > 0 => buf = buf.get_mut(res as usize..).ok_or(Error::UNEXPECTED)?, + -1 => { + let err = last_os_error(); + // We should try again if the call was interrupted. + if err.raw_os_error() != Some(libc::EINTR) { + return Err(err); + } } - } else { - // We don't check for EOF (ret = 0) as the data we are reading + // Negative return codes not equal to -1 should be impossible. + // EOF (ret = 0) should be impossible, as the data we are reading // should be an infinite stream of random bytes. - let len = min(res as usize, buf.len()); - buf = &mut buf[len..]; + _ => return Err(Error::UNEXPECTED), } } Ok(()) From d6e8464ade1fd0fcbc1f722a2f38734db40d2312 Mon Sep 17 00:00:00 2001 From: Artyom Pavlov Date: Sat, 2 Dec 2023 18:54:59 +0300 Subject: [PATCH 2/3] Add UNEXPECTED error description --- src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/error.rs b/src/error.rs index 2f66afb2..efdc6484 100644 --- a/src/error.rs +++ b/src/error.rs @@ -166,6 +166,7 @@ fn internal_desc(error: Error) -> Option<&'static str> { match error { Error::UNSUPPORTED => Some("getrandom: this target is not supported"), Error::ERRNO_NOT_POSITIVE => Some("errno: did not return a positive value"), + Error::UNEXPECTED => Some("unexpected situation"), Error::IOS_SEC_RANDOM => Some("SecRandomCopyBytes: iOS Security framework failure"), Error::WINDOWS_RTL_GEN_RANDOM => Some("RtlGenRandom: Windows system function failure"), Error::FAILED_RDRAND => Some("RDRAND: failed multiple times: CPU issue likely"), From cfdcfc46a24cdfb0e0c8185d4e84519165470f79 Mon Sep 17 00:00:00 2001 From: Artyom Pavlov Date: Sat, 2 Dec 2023 23:33:42 +0300 Subject: [PATCH 3/3] Tweak Hermit code --- src/hermit.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/hermit.rs b/src/hermit.rs index 5791ec08..21649d1f 100644 --- a/src/hermit.rs +++ b/src/hermit.rs @@ -1,5 +1,10 @@ use crate::Error; -use core::{convert::TryInto, mem::MaybeUninit, num::NonZeroU32}; +use core::{mem::MaybeUninit, num::NonZeroU32}; + +/// Minimum return value which we should get from syscalls in practice, +/// because Hermit uses positive `i32`s for error codes: +/// https://github.com/hermitcore/libhermit-rs/blob/main/src/errno.rs +const MIN_RET_CODE: isize = -(i32::MAX as isize); extern "C" { fn sys_read_entropy(buffer: *mut u8, length: usize, flags: u32) -> isize; @@ -8,17 +13,14 @@ extern "C" { pub fn getrandom_inner(mut dest: &mut [MaybeUninit]) -> Result<(), Error> { while !dest.is_empty() { let res = unsafe { sys_read_entropy(dest.as_mut_ptr() as *mut u8, dest.len(), 0) }; - if res > 0 { - dest = dest.get_mut(res as usize..).ok_or(Error::UNEXPECTED)?; + // Positive `isize`s can be safely casted to `usize` + if res > 0 && (res as usize) <= dest.len() { + dest = &mut dest[res as usize..]; } else { - // We should not get `res` equal to zero or smaller than `-i32::MAX`. - // If we get such unexpected value after all, we will return `Error::UNEXPECTED`. - let err = res - .checked_neg() - .and_then(|val| val.try_into().ok()) - .and_then(NonZeroU32::new) - .map(Into::into) - .unwrap_or(Error::UNEXPECTED); + let err = match res { + MIN_RET_CODE..=-1 => NonZeroU32::new(-res as u32).unwrap().into(), + _ => Error::UNEXPECTED, + }; return Err(err); } }