Skip to content

Improve robustness of the Hermit backend and sys_fill_exact #386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 2, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -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.
@@ -164,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"),
23 changes: 15 additions & 8 deletions src/hermit.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use crate::Error;
use core::{cmp::min, 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,14 +13,16 @@ extern "C" {
pub fn getrandom_inner(mut dest: &mut [MaybeUninit<u8>]) -> 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());
// Positive `isize`s can be safely casted to `usize`
if res > 0 && (res as usize) <= dest.len() {
dest = &mut dest[res as usize..];
} else {
let err = match res {
MIN_RET_CODE..=-1 => NonZeroU32::new(-res as u32).unwrap().into(),
_ => Error::UNEXPECTED,
};
return Err(err);
}
let len = min(res as usize, dest.len());
dest = &mut dest[len..];
}
Ok(())
}
21 changes: 11 additions & 10 deletions src/util_libc.rs
Original file line number Diff line number Diff line change
@@ -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(())