Repository: bitcoin/bitcoin (language: cpp)

Issue:
GetRandBytes() Hangs on Samsung Galaxy S25 and OnePlus 13
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

We have been using Bitcoin Core natively in the Nunchuk Bitcoin wallet, available on both desktop and mobile. On Samsung Galaxy S25 and OnePlus 13 (both running Android 15 with the SM8750-AB Snapdragon 8 Elite 3nm processor), the app fails to initialize due to an issue in `GetRandBytes()`.

The problem occurs when `GetRandBytes()` calls `GetRNDRRS()`, resulting in an infinite loop without retrieving entropy from the hardware.

```
uint64_t GetRNDRRS() noexcept
{
    uint8_t ok;
    uint64_t r1;
    do {
        // https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDRRS--Reseeded-Random-Number
        __asm__ volatile("mrs %0, s3_3_c2_c4_1; cset %w1, ne;"
                         : "=r"(r1), "=r"(ok)::"cc");
        if (ok) break;
        __asm__ volatile("yield");
    } while (true);
    return r1;
}
```

Build with: Android NDK 25.1.8937393 & 27.2.12479018

Related PR: https://github.com/bitcoin/bitcoin/pull/26839

Tested on commit: `57b47c47ef0bd36e1c32d709c62998c51dc76f34`

### Expected behaviour

`GetRandBytes()` should return entropy without hanging.

### Steps to reproduce

- Build Bitcoin Core for Android on master or 57b47c47ef0bd36e1c32d709c62998c51dc76f34
- Call `GetRandBytes`
- Function gets stuck

### Relevant log output

_No response_

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

master@57b47c47ef0bd36e1c32d709c62998c51dc76f34

### Operating system and version

Android 15

### Machine specifications

_No response_


Discussion / hints:
I have Samsung Galaxy S23 Ultra:

![Image](https://github.com/user-attachments/assets/26d0c0cd-54fd-45fd-9f15-82ad34c3e52b)

![Image](https://github.com/user-attachments/assets/c5c9c148-9f05-4023-8253-f94a10699f91)
> Build with: Android NDK 25.1.8937393 & 27.2.12479018

We've had some problems building on android (including NDK 25, see #29360), and are curious how you do the builds for that platform, for testing, can you list more specific build instructions please?
Hey @laanwj ! Our Nunchuk Android app is open source at https://github.com/nunchuk-io/nunchuk-android. You can take a look at the build instructions included in the repo. Let us know if you have any questions!
Thanks! i'd looked there but couldn't find anything that compiles Bitcoin Core, or any C++ code for that matter.
You need to first compile the SDK, which is a wrapper for libnunchuk (written in C++ and reuses Core code). @laanwj 

https://github.com/nunchuk-io/nunchuk-android-nativesdk
Has not been fixed since #31908 reverted #31826, but also out of scope - see https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669309907.  I believe this should stay closed.
This should be re-opened as per latest discussion in https://github.com/bitcoin/bitcoin/pull/31908 @glozow 

TL;DR: Either test and merge the updated fix (https://github.com/bitcoin/bitcoin/pull/31912), or revert the RNDR/RNDRRS feature entirely.
> TL;DR: Either test and merge the updated fix ([#31912](https://github.com/bitcoin/bitcoin/pull/31912)), or revert the RNDR/RNDRRS feature entirely.

31912 is currently a draft, so before further review and testing (and merge), it would be good to get something non-draft ready first.
Reflecting on this after a while, i'm still not convinced that a hardware feature not working to spec needs to be handled in user space. IMO this needs to be fixed (or worked around otherwise) in the kernel, as per https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669309907.

Edit: If you can point ot another project (such as OpenSSL) merging code to handle this edge case i'd be more easily convinced we need this.
> This should be re-opened as per latest discussion in https://github.com/bitcoin/bitcoin/pull/31908 @glozow 

I don't see any recent discussion on that thread? I'm not trying to die on this hill or say this problem isn't bad, but there are 414 issues open and imo we should only keep the ones that fall within the scope of what this project supports.
@laanwj Reposting my earlier comment here:

“I think one could argue that the fix should be made both in the kernel and in Core.

The design of GetRandBytes() and GetStrongRandBytes() is about being robust and sourcing entropy from multiple sources, so that even if one or some of those sources fail, Core can still obtain sufficiently good entropy. There could be multiple reasons hardware entropy fails to return lower down in the stack.”

Since NOT getting entropy from RNDR/RNDRRS is not a dealbreaker, Core shouldn’t freeze if the RNDR/RNDRRS stack fails for any reason (hardware or otherwise), and there should exist a sensible timeout. The question is about the robustness of the feature design itself.
@hugohn The reason it was reverted wasn't because the patch didn't work - it could have been fixed instead. It was reverted because merging it broke the build for more platforms than ones suffering just this issue, and this went undetected by all processes we have in place (both manual review and CI infrastructure). It's one thing to accept a patch to improve compatibility with an unsupported platform if it doesn't affect others, but this event clearly showed we weren't equipped to properly assess that. My view is that we shouldn't try to fix this again until/unless there is infrastructure in place that would help us detect build failures like this going forward.
@sipa What would it take to add the CI infrastructure needed? From @maflcko's comments it sounds like it’s possible?

In the meantime, given that RNDR/RNDRRS is only supported on a small subset of hardware, it might be prudent to temporarily revert the feature until CI support or hardware adoption is more robust. This would align with a conservative approach.
The discussion is a bit spread out, but when I looked in https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2687917758, the feature wasn't compiled into the release binaries. This raises two questions:

* Why? My understanding is that the kernel headers in guix should provide the `HWCAP2_RNG` macro?
* What is the target audience? Currently it only seems to be self-compiled executables.

About the CI: It is possible, but testing compilation or the path on a "happy" device doesn't help with the issue here on "sad" devices. See https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2676190482
Agree, let's just drop the support for now, it's rarely supported (broken on like half of the chips that support it in the first place), apparently not compiled into the release binary anyway, hard to test, and there are plenty of other sources of entropy. 

It's not worth so much discussion, or CI work.

Fix patch:
diff --git a/src/random.cpp b/src/random.cpp
index 5da053f74e4a6..fa3d8ec3f1e07 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -41,9 +41,6 @@
 #ifdef HAVE_SYSCTL_ARND
 #include <sys/sysctl.h>
 #endif
-#if defined(HAVE_STRONG_GETAUXVAL) && defined(__aarch64__)
-#include <sys/auxv.h>
-#endif
 
 namespace {
 
@@ -189,62 +186,6 @@ uint64_t GetRdSeed() noexcept
 #endif
 }
 
-#elif defined(__aarch64__) && defined(HWCAP2_RNG)
-
-bool g_rndr_supported = false;
-
-void InitHardwareRand()
-{
-    if (getauxval(AT_HWCAP2) & HWCAP2_RNG) {
-        g_rndr_supported = true;
-    }
-}
-
-void ReportHardwareRand()
-{
-    // This must be done in a separate function, as InitHardwareRand() may be indirectly called
-    // from global constructors, before logging is initialized.
-    if (g_rndr_supported) {
-        LogPrintf("Using RNDR and RNDRRS as additional entropy sources\n");
-    }
-}
-
-/** Read 64 bits of entropy using rndr.
- *
- * Must only be called when RNDR is supported.
- */
-uint64_t GetRNDR() noexcept
-{
-    uint8_t ok = 0;
-    uint64_t r1;
-    do {
-        // https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDR--Random-Number
-        __asm__ volatile("mrs %0, s3_3_c2_c4_0; cset %w1, ne;"
-                         : "=r"(r1), "=r"(ok)::"cc");
-        if (ok) break;
-        __asm__ volatile("yield");
-    } while (true);
-    return r1;
-}
-
-/** Read 64 bits of entropy using rndrrs.
- *
- * Must only be called when RNDRRS is supported.
- */
-uint64_t GetRNDRRS() noexcept
-{
-    uint8_t ok = 0;
-    uint64_t r1;
-    do {
-        // https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDRRS--Reseeded-Random-Number
-        __asm__ volatile("mrs %0, s3_3_c2_c4_1; cset %w1, ne;"
-                         : "=r"(r1), "=r"(ok)::"cc");
-        if (ok) break;
-        __asm__ volatile("yield");
-    } while (true);
-    return r1;
-}
-
 #else
 /* Access to other hardware random number generators could be added here later,
  * assuming it is sufficiently fast (in the order of a few hundred CPU cycles).
@@ -263,12 +204,6 @@ void SeedHardwareFast(CSHA512& hasher) noexcept {
         hasher.Write((const unsigned char*)&out, sizeof(out));
         return;
     }
-#elif defined(__aarch64__) && defined(HWCAP2_RNG)
-    if (g_rndr_supported) {
-        uint64_t out = GetRNDR();
-        hasher.Write((const unsigned char*)&out, sizeof(out));
-        return;
-    }
 #endif
 }
 
@@ -294,14 +229,6 @@ void SeedHardwareSlow(CSHA512& hasher) noexcept {
         }
         return;
     }
-#elif defined(__aarch64__) && defined(HWCAP2_RNG)
-    if (g_rndr_supported) {
-        for (int i = 0; i < 4; ++i) {
-            uint64_t out = GetRNDRRS();
-            hasher.Write((const unsigned char*)&out, sizeof(out));
-        }
-        return;
-    }
 #endif
 }
 


Test patch: