Conversation
newpavlov
force-pushed
the
linux_android/libc
branch
from
52d3e0d to
c10e466
Compare
| let getrandom_fn = unsafe { mem::transmute::<NonNull<c_void>, GetRandomFn>(fptr) }; | ||
| let dangling_ptr = ptr::NonNull::dangling().as_ptr(); | ||
| // Check that `getrandom` syscall is supported by kernel | ||
| let res = unsafe { getrandom_fn(dangling_ptr, 0, 0) }; |
There was a problem hiding this comment.
Maybe we can skip this check and assume that if libc exposes the getrandom function, then the syscall is supported by the kernel?
There was a problem hiding this comment.
No, getrandom is documented as returning ENOSYS.
| use_file::getrandom_inner(dest) | ||
| type GetRandomFn = unsafe extern "C" fn(*mut c_void, libc::size_t, libc::c_uint) -> libc::ssize_t; | ||
|
|
||
| /// Sentinel value which indicates that `libc::getrandom` either not available, |
There was a problem hiding this comment.
This is complicated. Is there really such a big performance penalty for unconditionally calling getrandom each time and then falling back to /dev/urandom when getrandom returns ENOSYS?
There was a problem hiding this comment.
...or just using an AtomicBool like before?
There was a problem hiding this comment.
What do you mean by "complicated"? This value is guaranteed to not be a valid function pointer, so we use it as "uninitialized" marker.
dlsym can be a relatively heavy operation, so I would prefer to cache its result.
With AtomicBool we would need a separate atomic to store function pointer. Why introduce two atomic loads when one would suffice?
There was a problem hiding this comment.
Ah, I missed the dlsym bit and assumed you were calling libc::getrandom.
|
This new implementation seems to fall back where the old one didn't. I believe that's because I'm statically linking musl. The result is a crash, because I'm doing this in a very special "distro" which doesn't have /dev/urandom mounted. See https://github.com/aya-rs/aya/blob/665d4f20bb53de0aa10545bb897ab73f0661a337/init/src/main.rs. Anyway, I'm pretty sure use dlsym in this way isn't going to work for anyone statically linking musl (which is a major reason for using musl). |
|
Hm, we could use the As a temporary workaround you could use the |
|
Yeah, exactly - there is no test to check that musl works correctly (i.e. no fallback). I'll leave you to add that test, but I'm sending a PR that I think fixes this by not using dlsym on musl. EDIT: #598 |
Use of `libc::getrandom` will automatically give us optimizations like vDSO (rust-random#503) and can help with testing of fallback logic (rust-random#289). It was also requested in rust-random#285. In that discussion we decided against using this approach, but in the light of the vDSO optimization it may be worth to reconsider it. In `linux_android_with_fallback` use of `libc::syscall` is replaced by `dlsym`-based code similar to what we use in the `netbsd` backend.
3 participants
Use of
libc::getrandomwill automatically give us optimizations like vDSO (#503) and can help with testing of fallback logic (#289).It was also requested in #285. In that discussion we decided against using this approach, but in the light of the vDSO optimization it may be worth to reconsider it.
In
linux_android_with_fallbackuse oflibc::syscallis replaced bydlsym-based code similar to what we use in thenetbsdbackend.