Skip to content
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

637 Solaris getrandom() support breaks illumos #730

Merged
merged 2 commits into from
Feb 22, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 4 additions & 2 deletions rand_os/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ trait OsRngImpl where Self: Sized {
#[cfg(any(target_os = "linux", target_os = "android",
target_os = "netbsd", target_os = "dragonfly",
target_os = "solaris", target_os = "redox",
target_os = "haiku", target_os = "emscripten"))]
target_os = "haiku", target_os = "emscripten",
target_os = "illumos"))]
mod random_device;

macro_rules! mod_use {
Expand All @@ -309,7 +310,7 @@ mod_use!(cfg(target_os = "macos"), macos);
mod_use!(cfg(target_os = "netbsd"), netbsd);
mod_use!(cfg(target_os = "openbsd"), openbsd_bitrig);
mod_use!(cfg(target_os = "redox"), redox);
mod_use!(cfg(target_os = "solaris"), solaris);
mod_use!(cfg(any(target_os = "solaris", target_os = "illumos")), solarish);
mod_use!(cfg(windows), windows);
mod_use!(cfg(target_env = "sgx"), sgx);

Expand Down Expand Up @@ -376,6 +377,7 @@ mod imp {
target_os = "openbsd",
target_os = "redox",
target_os = "solaris",
target_os = "illumos",
windows,
target_arch = "wasm32",
target_env = "sgx"
Expand Down
87 changes: 53 additions & 34 deletions rand_os/src/solaris.rs → rand_os/src/solarish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ use std::io;
use std::io::Read;
use std::fs::{File, OpenOptions};
use std::os::unix::fs::OpenOptionsExt;
use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};
use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering, AtomicUsize};
use std::cmp;
use std::mem;

#[derive(Clone, Debug)]
pub struct OsRng {
Expand Down Expand Up @@ -97,9 +98,10 @@ impl OsRngImpl for OsRng {
}

fn max_chunk_size(&self) -> usize {
// The documentation says 1024 is the maximum for getrandom, but
// 1040 for /dev/random.
1024
// This is the largest size that's guaranteed to not block across
// all the Solarish platforms, though some may allow for larger
// sizes.
256
}

fn method_str(&self) -> &'static str {
Expand All @@ -110,18 +112,50 @@ impl OsRngImpl for OsRng {
}
}

fn getrandom(buf: &mut [u8], blocking: bool) -> libc::c_long {
extern "C" {
fn syscall(number: libc::c_long, ...) -> libc::c_long;
}
#[cfg(target_os = "illumos")]
type GetRandomFn = unsafe extern fn(*mut u8, libc::size_t, libc::c_uint)
-> libc::ssize_t;
#[cfg(target_os = "solaris")]
type GetRandomFn = unsafe extern fn(*mut u8, libc::size_t, libc::c_uint)
-> libc::c_int;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stumbled upon this while investigating rust-lang/miri#3924. In this code, the return type is different between Illumos and Solaris. However, in today's libc, the return type is the same for both OSes. (This was added in rust-lang/libc#2236.)

Do you remember where you got the different return type from?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was like this since getrandom v0.1 days: https://docs.rs/crate/getrandom/0.1.0/source/src/solaris_illumos.rs#28-31

We probably just copied what std done at the time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current version of getrandom we use libc::getrandom, so it should no longer be an issue with up-to-date dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah getrandom is older than this? Okay.

so it should no longer be an issue with up-to-date dependencies.

Yeah, I am just trying to figure out where the other return type comes from and which one is correct...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rand_os is an older crate. It was eventually deprecated in favor of getrandom.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean the version pre-change.

The getrandom() function was something that was added to both OSes after both forked. There is no cross-pollination between the two projects. Oracle does their own thing and the only notice you get about anything is when a release comes out. illumos does what we feel is best for us -- while we strive to not break existing stuff, when adding new interfaces, while compatibility with Solaris may be a factor, it is far from the only (or most important) factor.

In this case, getrandom() was added after illumos forked from Solaris. I'm not sure which one got it first, but the illumos version was modeled after FreeBSD. It appears that Oracle changed the function signature in Solaris 11.4 so it was the same as the illumos and FreeBSD version (you can look at the section 2 man pages of Solaris 11.3 and Solaris 11.4).

The use of dlsym was to better support the existing is_getrandom_available test. Issuing unsupported syscalls on Solaris and illumos systems (as noted in the comments) generates a SYSSIG signal which is usually unhandled by a program (and thus usually crashes it). Checking for the libc function is less likely to cause problems.

Copy link
Contributor

@RalfJung RalfJung Sep 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why you are even talking about Illumos. My question is entirely and exclusively about Solaris. For the purpose of this discussion, I am treating the two as completely independent OSes.

EDIT: I probably wasn't sufficiently clear in the original question here. That's why I linked to rust-lang/miri#3924 for context. :)

Copy link
Contributor

@RalfJung RalfJung Sep 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically, the question is, what is the return type for getrandom on Solaris? This PR made it c_int. But today in the libc crate, it is ssize_t. Who is right? Or did Solaris change the return type of this function?

Copy link
Contributor

@RalfJung RalfJung Sep 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that Oracle changed the function signature in Solaris 11.4

That's our best running theory. It seems very strange to me that one can just change such a function signature but maybe that works for Oracle for some reason?

That hypothesis explains basically everything, except for https://www.gnu.org/software/gnulib/manual/html_node/getrandom.html which says

This function has a different return type on some platforms: Solaris 11.4.


// Use dlsym to determine if getrandom(2) is present in libc. On Solarish
// systems, the syscall interface is not stable and can change between
// updates. Even worse, issuing unsupported syscalls will cause the system
// to generate a SIGSYS signal (usually terminating the program).
// Instead the stable APIs are exposed via libc. Cache the result of the
// lookup for future calls. This is loosely modeled after the
// libstd::sys::unix::weak macro which unfortunately is not exported.
fn fetch() -> Option<&'static GetRandomFn> {
static FPTR: AtomicUsize = AtomicUsize::new(1);

const SYS_GETRANDOM: libc::c_long = 143;
unsafe {
dhardy marked this conversation as resolved.
Show resolved Hide resolved
if FPTR.load(Ordering::SeqCst) == 1 {
let name = "getrandom\0";
let addr = libc::dlsym(libc::RTLD_DEFAULT,
name.as_ptr() as *const _) as usize;
FPTR.store(addr, Ordering::SeqCst);
}

if FPTR.load(Ordering::SeqCst) == 0 {
return None;
} else {
mem::transmute::<&AtomicUsize, Option<&GetRandomFn>>(&FPTR)
dhardy marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

fn getrandom(buf: &mut [u8], blocking: bool) -> libc::ssize_t {
const GRND_NONBLOCK: libc::c_uint = 0x0001;
const GRND_RANDOM: libc::c_uint = 0x0002;

unsafe {
syscall(SYS_GETRANDOM, buf.as_mut_ptr(), buf.len(),
if blocking { 0 } else { GRND_NONBLOCK } | GRND_RANDOM)
if let Some(rand) = fetch() {
unsafe {
rand(buf.as_mut_ptr(), buf.len(),
if blocking { 0 } else { GRND_NONBLOCK } | GRND_RANDOM) as libc::ssize_t
dhardy marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
-1
}
}

Expand All @@ -143,33 +177,18 @@ fn getrandom_try_fill(dest: &mut [u8], blocking: bool) -> Result<(), Error> {
err,
));
}
} else if result != dest.len() as i64 {
} else if result != dest.len() as libc::ssize_t {
return Err(Error::new(ErrorKind::Unavailable,
"unexpected getrandom error"));
}
Ok(())
}

fn is_getrandom_available() -> bool {
use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};
use std::sync::{Once, ONCE_INIT};

static CHECKER: Once = ONCE_INIT;
static AVAILABLE: AtomicBool = ATOMIC_BOOL_INIT;

CHECKER.call_once(|| {
debug!("OsRng: testing getrandom");
let mut buf: [u8; 0] = [];
let result = getrandom(&mut buf, false);
let available = if result == -1 {
let err = io::Error::last_os_error().raw_os_error();
err != Some(libc::ENOSYS)
} else {
true
};
AVAILABLE.store(available, Ordering::Relaxed);
info!("OsRng: using {}", if available { "getrandom" } else { "/dev/random" });
});

AVAILABLE.load(Ordering::Relaxed)
let available = match fetch() {
Some(_) => true,
None => false,
};
info!("OsRng: using {}", if available { "getrandom" } else { "/dev/random" });
available
}