-
Notifications
You must be signed in to change notification settings - Fork 240
linux_android_with_fallback: detect getrandom #758
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 |
|---|---|---|
|
|
@@ -2,9 +2,7 @@ name: Build | |
|
|
||
| on: | ||
| push: | ||
| branches: master | ||
| pull_request: | ||
| branches: master | ||
| schedule: | ||
| - cron: "0 12 * * 1" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,7 @@ name: Test | |
|
|
||
| on: | ||
| push: | ||
| branches: master | ||
| pull_request: | ||
| branches: master | ||
| schedule: | ||
| - cron: "0 12 * * 1" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,7 @@ name: Workspace | |
|
|
||
| on: | ||
| push: | ||
| branches: master | ||
| pull_request: | ||
| branches: master | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,11 +9,13 @@ use core::{ | |
| ffi::c_void, | ||
| mem::{self, MaybeUninit}, | ||
| ptr, | ||
| sync::atomic::{AtomicPtr, Ordering}, | ||
| }; | ||
|
|
||
| pub use crate::util::{inner_u32, inner_u64}; | ||
|
|
||
| #[expect(dead_code, reason = "LazyBool is not used")] | ||
| #[path = "../lazy.rs"] | ||
| mod lazy; | ||
| #[path = "../util_libc.rs"] | ||
| mod util_libc; | ||
|
|
||
|
|
@@ -42,36 +44,24 @@ 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) }; | ||
| fn init() -> usize { | ||
| let mut ptr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"getrandom".as_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; | ||
| } | ||
| GETRANDOM.store(ptr, Ordering::Release); | ||
| ptr | ||
| ptr as usize | ||
| } | ||
|
|
||
| #[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) }; | ||
| static GETRANDOM_FN: lazy::LazyUsize = lazy::LazyUsize::new(); | ||
|
|
||
| let fptr = GETRANDOM_FN.unsync_init(init); | ||
| let fptr = unsafe { mem::transmute::<usize, GetRandomFn>(fptr) }; | ||
|
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. Note that this is probably incorrect from the strict pointer provenance point of view.
Contributor
Author
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. I don't think so. The provenance of the original pointer is the same as the provenance of the pointer constructed here. |
||
| util_libc::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.
These changes are not relevant.
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.
Sure, they're in a separate commit with reasoning given (it's a bummer you guys insist on squashing on merge).
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.
@tamird I think this is a reasonable thing to do, but changing on which branches CI runs is tricky from a security perspective, and it deserves a dedicated PR.