linux_android_with_fallback: do not use dlsym on MUSL targets #602

newpavlov · 2025-02-05T11:45:16Z

Fixes #600

tamird · 2025-02-05T12:01:34Z

src/backends/linux_android_with_fallback.rs

+    #[cfg(target_env = "musl")]
+    let raw_ptr = {
+        let fptr: GetRandomFn = libc::getrandom;
+        unsafe { transmute::<GetRandomFn, *mut c_void>(fptr) }


This cast to a pointer and back smells like an indirect call.

There is effectively no overhead to this approach. "Direct" call to libc::getradom compiles down to:

xor edx, edx jmp qword ptr [rip + getrandom@GOTPCREL]

While the "indirect" call compiles to:

mov rax, qword ptr [rip + GETRANDOM_FN] xor edx, edx jmp rax

Yes, the former is a bit friendlier to CPUs, but compared to the syscall cost the difference is negligible. If you care about this difference, then you should use the linux_getrandom opt-in backend.

src/backends/linux_android_with_fallback.rs

tamird · 2025-02-05T16:51:00Z

@newpavlov just force push to remove the two spurious commits - you're going to clutter the git history this way =/

newpavlov · 2025-02-05T16:54:15Z

We always squash commits before merge, so local commit history in PR does not matter much.

tamird · 2025-02-05T16:57:15Z

OK. If this were my project I'd ask you to back out all the unnecessary import churn and random comment changes - but it's not!

tamird

@newpavlov can we please merge this one way or the other to fix the breakage and cut 0.3.2? Thank you.

tamird · 2025-02-10T14:47:29Z

src/backends/linux_android_with_fallback.rs

@@ -17,18 +17,28 @@ type GetRandomFn = unsafe extern "C" fn(*mut c_void, libc::size_t, libc::c_uint)
 /// 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(ptr::null_mut());
+static GETRANDOM_FN: AtomicPtr<c_void> = AtomicPtr::new(core::ptr::null_mut());


could we avoid this noise in the diff? doesn't seem related to the fix being made.

tamird · 2025-02-10T14:48:14Z

src/backends/linux_android_with_fallback.rs

    let res_ptr = match NonNull::new(raw_ptr) {
        Some(fptr) => {
-            let getrandom_fn = unsafe { mem::transmute::<NonNull<c_void>, GetRandomFn>(fptr) };
-            let dangling_ptr = ptr::NonNull::dangling().as_ptr();
+            let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) };


the diff on these 2 lines is also not necessary.

tamird · 2025-02-10T14:48:21Z

src/backends/linux_android_with_fallback.rs

@@ -54,7 +64,7 @@ fn init() -> NonNull<c_void> {
    res_ptr
 }

-// prevent inlining of the fallback implementation
+// Prevent inlining of the fallback implementation


not necessary

tamird · 2025-02-10T14:48:24Z

src/backends/linux_android_with_fallback.rs

@@ -78,7 +88,7 @@ pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
        use_file_fallback(dest)
    } else {
        // note: `transmute` is currently the only way to convert a pointer into a function reference
-        let getrandom_fn = unsafe { mem::transmute::<NonNull<c_void>, GetRandomFn>(fptr) };
+        let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) };


not necessary

newpavlov · 2025-02-12T15:28:24Z

I think it's fine to keep minor code tweaks in this PR.

Before releasing v0.3.2 I would like to also merge the linux_raw and efi_rng PRs.

### Added - `efi_rng` opt-in backend [#570] - `linux_raw` opt-in backend [#572] - `.cargo/config.toml` example in the crate-level docs [#591] - `getrandom_test_linux_without_fallback` configuration flag to test that file fallback is not triggered in the `linux_android_with_fallback` backend [#605] - Built-in support for `*-linux-none` targets [#618] - Cygwin support [#626] ### Changed - Update `wasi` dependency to v0.14 [#594] - Add `#[inline]` attribute to the inner functions [#596] - Update WASI and Emscripten links in the crate-level docs [#597] - Do not use `dlsym` on MUSL targets in the `linux_android_with_fallback` backend [#602] - Remove `linux_android.rs` and use `getrandom.rs` instead [#603] - Always use `RtlGenRandom` on Windows targets when compiling with pre-1.78 Rust [#610] - Internal representation of the `Error` type [#614] - Remove `windows-targets` dependency and use [`raw-dylib`] directly [#627] ### Removed - `Error::INTERNAL_START` and `Error::CUSTOM_START` associated constants [#614] [#570]: #570 [#572]: #572 [#591]: #591 [#594]: #594 [#596]: #596 [#597]: #597 [#602]: #602 [#603]: #603 [#605]: #605 [#610]: #610 [#614]: #614 [#618]: #618 [#626]: #626 [#627]: #627 [`raw-dylib`]: https://doc.rust-lang.org/reference/items/external-blocks.html?highlight=link#dylib-versus-raw-dylib

sbwml · 2025-03-20T14:40:57Z

好尴尬 🙃，当我为 mips musl 编译它时，遇到了一些奇奇怪怪的错误：

https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13956208010/job/39067818527#step:5:404

error[E0425]: cannot find value `getrandom` in crate `libc`
  --> /home/runner/work/shadowsocks-rust-mips/shadowsocks-rust-mips/install/registry/src/index.crates.io-1949cf8c6b5b557f/getrandom-0.3.2/src/backends/linux_android_with_fallback.rs:34:39
   |
34 |         let fptr: GetRandomFn = libc::getrandom;
   |                                       ^^^^^^^^^ not found in `libc`

For more information about this error, try `rustc --explain E0425`.

给它定义一个 getrandom 函数它才可以编译 https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13971234091/job/39113320608#step:5:419

newpavlov · 2025-03-20T15:16:27Z

@sbwml
Please use English here, even if it's somewhat broken.

Rust officially uses musl v1.2.3 for the mips-unknown-linux-musl target. Meanwhile, getrandom was added in musl v1.1.20. So it may be a problem with the libc crate, which for some reason does not expose the symbol on this target. It may be worth to open an issue in its repository.

For now you could downgrade getrandom to an older version as a temporary workaround.

tamird · 2025-03-20T15:21:17Z

Indeed, fn getrandom is missing on mips-unknown-linux-musl even though SYS_getrandom is defined: https://github.com/rust-lang/libc/blob/4159080fc4dcec769703d79994ab725a4e6d26b4/src/unix/linux_like/linux/musl/b32/mips/mod.rs#L727

Looks like that was somewhat intentional: rust-lang/libc@457c02d

newpavlov · 2025-03-20T15:34:01Z

Looks like that was somewhat intentional

Either way, it's worth to clarify it with libc developers. The reason for that may not be longer applicable and, at the very least, it should be fine to add it for the MUSL target.

tamird · 2025-03-20T15:39:59Z

The discussion in that PR (rust-lang/libc#1399) suggests that it was failing CI at the time. The musl version was bumped in rust-lang/rust#107129, though. I'll send a patch and see what happens.

tamird · 2025-03-20T15:56:04Z

rust-lang/libc#4346

eareimu · 2025-03-21T02:25:41Z

好尴尬 🙃，当我为 mips musl 编译它时，遇到了一些奇奇怪怪的错误：

https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13956208010/job/39067818527#step:5:404
error[E0425]: cannot find value `getrandom` in crate `libc`
  --> /home/runner/work/shadowsocks-rust-mips/shadowsocks-rust-mips/install/registry/src/index.crates.io-1949cf8c6b5b557f/getrandom-0.3.2/src/backends/linux_android_with_fallback.rs:34:39
   |
34 |         let fptr: GetRandomFn = libc::getrandom;
   |                                       ^^^^^^^^^ not found in `libc`

For more information about this error, try `rustc --explain E0425`.
给它定义一个 getrandom 函数它才可以编译 https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13971234091/job/39113320608#step:5:419

Same issue, the problem was temporarily solved by specifying the getramdom version in Cargo.toml:

getrandom = "=0.3.1"

Experienced supply poisoning :(

josephlr · 2025-03-22T06:42:54Z

The fix has been merged in rust-lang/libc#4346, once it's released on libc's stable 0.2 branch, the issue should be resolved.

linux_android_with_fallback: do not use dlsym on MUSL targets

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

5c46936

newpavlov mentioned this pull request Feb 5, 2025

linux_android_with_fallback: avoid dlsym #598

Closed

newpavlov requested a review from josephlr February 5, 2025 11:59

tamird reviewed Feb 5, 2025

View reviewed changes

Add #[cold] to use_file_fallback

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

60f2393

tamird reviewed Feb 5, 2025

View reviewed changes

src/backends/linux_android_with_fallback.rs Outdated Show resolved Hide resolved

Revert addition of #[cold]

Loading
Loading status checks…

3f2d0e5

tamird reviewed Feb 10, 2025

View reviewed changes

newpavlov merged commit b75db5c into master Feb 12, 2025
58 checks passed

newpavlov deleted the fix_musl branch February 12, 2025 15:28

newpavlov mentioned this pull request Mar 7, 2025

Release v0.3.2 #617

Merged

newpavlov mentioned this pull request Mar 21, 2025

error[E0425]: cannot find value getrandom in crate libc on target mipsel-unknown-linux-musl v0.3.2 #636

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linux_android_with_fallback: do not use dlsym on MUSL targets #602

linux_android_with_fallback: do not use dlsym on MUSL targets #602

newpavlov commented Feb 5, 2025

tamird Feb 5, 2025

newpavlov Feb 5, 2025 •

edited

Loading

tamird commented Feb 5, 2025

newpavlov commented Feb 5, 2025

tamird commented Feb 5, 2025

tamird left a comment

tamird Feb 10, 2025

tamird Feb 10, 2025

tamird Feb 10, 2025

tamird Feb 10, 2025

newpavlov commented Feb 12, 2025

sbwml commented Mar 20, 2025

newpavlov commented Mar 20, 2025 •

edited

Loading

tamird commented Mar 20, 2025

newpavlov commented Mar 20, 2025

tamird commented Mar 20, 2025

tamird commented Mar 20, 2025

eareimu commented Mar 21, 2025

josephlr commented Mar 22, 2025

linux_android_with_fallback: do not use dlsym on MUSL targets #602

linux_android_with_fallback: do not use dlsym on MUSL targets #602

Conversation

newpavlov commented Feb 5, 2025

tamird Feb 5, 2025

Choose a reason for hiding this comment

newpavlov Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

tamird commented Feb 5, 2025

newpavlov commented Feb 5, 2025

tamird commented Feb 5, 2025

tamird left a comment

Choose a reason for hiding this comment

tamird Feb 10, 2025

Choose a reason for hiding this comment

tamird Feb 10, 2025

Choose a reason for hiding this comment

tamird Feb 10, 2025

Choose a reason for hiding this comment

tamird Feb 10, 2025

Choose a reason for hiding this comment

newpavlov commented Feb 12, 2025

sbwml commented Mar 20, 2025

newpavlov commented Mar 20, 2025 • edited Loading

tamird commented Mar 20, 2025

newpavlov commented Mar 20, 2025

tamird commented Mar 20, 2025

tamird commented Mar 20, 2025

eareimu commented Mar 21, 2025

josephlr commented Mar 22, 2025

newpavlov Feb 5, 2025 •

edited

Loading

newpavlov commented Mar 20, 2025 •

edited

Loading