-
Notifications
You must be signed in to change notification settings - Fork 248
Unify lazy atomics in entropy backends #804
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,7 @@ use crate::Error; | |
| use core::{ | ||
| ffi::c_void, | ||
| mem::{MaybeUninit, transmute}, | ||
| ptr::NonNull, | ||
| sync::atomic::{AtomicPtr, Ordering}, | ||
| ptr::{self, NonNull}, | ||
| }; | ||
| use use_file::utils; | ||
|
|
||
|
|
@@ -15,20 +14,14 @@ type GetRandomFn = unsafe extern "C" fn(*mut c_void, libc::size_t, libc::c_uint) | |
|
|
||
| /// Sentinel value which indicates that `libc::getrandom` either not available, | ||
| /// or not supported by kernel. | ||
| const NOT_AVAILABLE: NonNull<c_void> = unsafe { NonNull::new_unchecked(usize::MAX as *mut c_void) }; | ||
|
|
||
| static GETRANDOM_FN: AtomicPtr<c_void> = AtomicPtr::new(core::ptr::null_mut()); | ||
| const NOT_AVAILABLE: NonNull<c_void> = NonNull::dangling(); | ||
|
|
||
| #[cold] | ||
| #[inline(never)] | ||
| fn init() -> NonNull<c_void> { | ||
| // Use static linking to `libc::getrandom` on MUSL targets and `dlsym` everywhere else | ||
| #[cfg(not(target_env = "musl"))] | ||
| let raw_ptr = { | ||
| static NAME: &[u8] = b"getrandom\0"; | ||
| let name_ptr = NAME.as_ptr().cast::<libc::c_char>(); | ||
| unsafe { libc::dlsym(libc::RTLD_DEFAULT, name_ptr) } | ||
| }; | ||
| let raw_ptr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"getrandom".as_ptr()) }; | ||
| #[cfg(target_env = "musl")] | ||
| let raw_ptr = { | ||
| let fptr: GetRandomFn = libc::getrandom; | ||
|
|
@@ -37,10 +30,9 @@ fn init() -> NonNull<c_void> { | |
|
|
||
| let res_ptr = match NonNull::new(raw_ptr) { | ||
| Some(fptr) => { | ||
| let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) }; | ||
| let dangling_ptr = NonNull::dangling().as_ptr(); | ||
| let getrandom_fn = unsafe { transmute::<*mut c_void, GetRandomFn>(fptr.as_ptr()) }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you casting from |
||
| // Check that `getrandom` syscall is supported by kernel | ||
| let res = unsafe { getrandom_fn(dangling_ptr, 0, 0) }; | ||
| let res = unsafe { getrandom_fn(ptr::dangling_mut(), 0, 0) }; | ||
| if cfg!(getrandom_test_linux_fallback) { | ||
| NOT_AVAILABLE | ||
| } else if res.is_negative() { | ||
|
|
@@ -65,7 +57,6 @@ fn init() -> NonNull<c_void> { | |
| panic!("Fallback is triggered with enabled `getrandom_test_linux_without_fallback`") | ||
| } | ||
|
|
||
| GETRANDOM_FN.store(res_ptr.as_ptr(), Ordering::Release); | ||
| res_ptr | ||
| } | ||
|
|
||
|
|
@@ -77,23 +68,17 @@ fn use_file_fallback(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> { | |
|
|
||
| #[inline] | ||
| pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> { | ||
| // Despite being only a single atomic variable, we still cannot always use | ||
| // Ordering::Relaxed, as we need to make sure a successful call to `init` | ||
| // is "ordered before" any data read through the returned pointer (which | ||
| // occurs when the function is called). Our implementation mirrors that of | ||
| // the one in libstd, meaning that the use of non-Relaxed operations is | ||
| // probably unnecessary. | ||
| let raw_ptr = GETRANDOM_FN.load(Ordering::Acquire); | ||
| let fptr = match NonNull::new(raw_ptr) { | ||
| Some(p) => p, | ||
| None => init(), | ||
| }; | ||
| #[path = "../utils/lazy.rs"] | ||
| mod lazy; | ||
|
|
||
| static GETRANDOM_FN: lazy::LazyNonNull<c_void> = lazy::LazyNonNull::new(); | ||
| let fptr = GETRANDOM_FN.unsync_init(init); | ||
|
|
||
| if fptr == NOT_AVAILABLE { | ||
| use_file_fallback(dest) | ||
| } else { | ||
| // note: `transmute` is currently the only way to convert a pointer into a function reference | ||
| let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) }; | ||
| let getrandom_fn = unsafe { transmute::<*mut c_void, GetRandomFn>(fptr.as_ptr()) }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same. |
||
| utils::sys_fill_exact(dest, |buf| unsafe { | ||
| getrandom_fn(buf.as_mut_ptr().cast(), buf.len(), 0) | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,8 +8,7 @@ use core::{ | |
| cmp, | ||
| ffi::c_void, | ||
| mem::{self, MaybeUninit}, | ||
| ptr, | ||
| sync::atomic::{AtomicPtr, Ordering}, | ||
| ptr::{self, NonNull}, | ||
| }; | ||
|
|
||
| pub use crate::util::{inner_u32, inner_u64}; | ||
|
|
@@ -42,36 +41,28 @@ unsafe extern "C" fn polyfill_using_kern_arand( | |
|
|
||
| type GetRandomFn = unsafe extern "C" fn(*mut c_void, libc::size_t, libc::c_uint) -> libc::ssize_t; | ||
|
|
||
| static GETRANDOM: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut()); | ||
|
|
||
| #[cold] | ||
| #[inline(never)] | ||
| fn init() -> *mut c_void { | ||
| static NAME: &[u8] = b"getrandom\0"; | ||
| let name_ptr = NAME.as_ptr().cast::<libc::c_char>(); | ||
| let mut ptr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, name_ptr) }; | ||
| if ptr.is_null() || cfg!(getrandom_test_netbsd_fallback) { | ||
| // Verify `polyfill_using_kern_arand` has the right signature. | ||
| const POLYFILL: GetRandomFn = polyfill_using_kern_arand; | ||
| ptr = POLYFILL as *mut c_void; | ||
| fn init() -> NonNull<c_void> { | ||
| let ptr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"getrandom".as_ptr()) }; | ||
| if !cfg!(getrandom_test_netbsd_fallback) { | ||
| if let Some(ptr) = NonNull::new(ptr) { | ||
| return ptr; | ||
| } | ||
| } | ||
| GETRANDOM.store(ptr, Ordering::Release); | ||
| ptr | ||
| const POLYFILL: GetRandomFn = polyfill_using_kern_arand; | ||
| unsafe { NonNull::new_unchecked(POLYFILL as *mut c_void) } | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> { | ||
| // Despite being only a single atomic variable, we still cannot always use | ||
| // Ordering::Relaxed, as we need to make sure a successful call to `init` | ||
| // is "ordered before" any data read through the returned pointer (which | ||
| // occurs when the function is called). Our implementation mirrors that of | ||
| // the one in libstd, meaning that the use of non-Relaxed operations is | ||
| // probably unnecessary. | ||
| let mut fptr = GETRANDOM.load(Ordering::Acquire); | ||
| if fptr.is_null() { | ||
| fptr = init(); | ||
| } | ||
| let fptr = unsafe { mem::transmute::<*mut c_void, GetRandomFn>(fptr) }; | ||
| #[path = "../utils/lazy.rs"] | ||
| mod lazy; | ||
|
|
||
| static GETRANDOM_FN: lazy::LazyNonNull<c_void> = lazy::LazyNonNull::new(); | ||
|
|
||
| let fptr = GETRANDOM_FN.unsync_init(init); | ||
| let fptr = unsafe { mem::transmute::<*mut c_void, GetRandomFn>(fptr.as_ptr()) }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same. |
||
| utils::sys_fill_exact(dest, |buf| unsafe { | ||
| fptr(buf.as_mut_ptr().cast::<c_void>(), buf.len(), 0) | ||
| }) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is correct. Quoting the method docs:
Meanwhile with the
usize::MAXvariant we can argue that it fundamentally can not be a valid function pointer.