[Bug libstdc++/100444] New: std::random_device isn't random on AMD

[Bug libstdc++/100444] New: std::random_device isn't random on AMD

[ecree429 at virginmedia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

            Bug ID: 100444
           Summary: std::random_device isn't random on AMD
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ecree429 at virginmedia dot com
  Target Milestone: ---

On certain AMD processors (in my case Ryzen 5 3600, stepping 0, microcode
0x8701013), RDRAND instruction is broken and always returns all-1s.  Thus e.g.
    #include <iostream>
    #include <random>

    int main(void)
    {
      static std::random_device rd;
      std::cout << rd() << '\n';
    }
always prints 4294967295.

Other RNG implementations such as Qt's QRandomGenerator detect this brokenness
and work around it, falling back to an alternate source of entropy.  libstdc++
should ideally do the same, rather than silently failing to be random.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Isn't this worked around at kernel level by disabling RDRAND support on
affected CPUs?

[Bug libstdc++/100444] std::random_device isn't random on AMD

[ecree429 at virginmedia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #2 from Edward Cree <ecree429 at virginmedia dot com> ---
(In reply to Richard Biener from comment #1)
> Isn't this worked around at kernel level by disabling RDRAND support on
> affected CPUs?

Not sure.  I wondered if it might be, so I straced the process and it didn't
seem to be making a system call, i.e. it's using the RDRAND instruction
directly.

>From the kernel docs on nordrand:
> [X86] Disable kernel use of the RDRAND and
> RDSEED instructions even if they are supported
> by the processor.  RDRAND and RDSEED are still
> available to user space applications.

But I suppose it might affect the vdso, or something else the lib uses to
detect whether RDRAND is present.

I've added nordrand to my kernel cmdline, so next time I reboot I'll be able to
test this; idk when that'll be, though.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-*

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Hmm, according to this article, the bug was fixed in a BIOS update over a year
ago:
https://arstechnica.com/gadgets/2019/10/how-a-months-old-amd-microcode-bug-destroyed-my-weekend/

[Bug libstdc++/100444] std::random_device isn't random on AMD

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
So at least libstdc++ does check RDRAND availability via cpuid.  It also
already
checks the return value for "no avaialble randomness":

      while (__builtin_ia32_rdrand32_step(&val) == 0)
        if (--retries == 0)
          std::__throw_runtime_error(__N("random_device: rdrand failed"));

so here it could check for -1 as well though in theory that
can happen with true randomness as well, even if very unlikely.  Note
that it would never return -1 then (as it never returns 0 at the moment).

Thus I believe the issue is mitigated at the kernel level and people that
cannot be bothered to update their ucode or the kernel are not likely
bothered to update libstdc++ either.  Note handling it above would
raise a runtime error rather than falling back to other sources of randomness.
Doing the checking where we detect rdrand support would avoid that.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> Thus I believe the issue is mitigated at the kernel level and people that

Yes, that's my understanding.

> cannot be bothered to update their ucode or the kernel are not likely
> bothered to update libstdc++ either.  Note handling it above would
> raise a runtime error rather than falling back to other sources of
> randomness.
> Doing the checking where we detect rdrand support would avoid that.

I suppose we could use the rdrand instruction a couple of times when doing the
cpuid support, and if it gives the same value conclude it's broken and so not
available.

But as you say, people who haven't bothered to update microcode probably aren't
going to get a libstdc++ update either.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> so here it could check for -1 as well though in theory that
> can happen with true randomness as well, even if very unlikely.  Note
> that it would never return -1 then (as it never returns 0 at the moment).

The return value is not the random number, it's 0 for failure and 1 for
success. The random number is in val. So it can return 0.

The AMD bug is that it returns 1 when it shouldn't.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[ecree429 at virginmedia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #7 from Edward Cree <ecree429 at virginmedia dot com> ---
(In reply to Richard Biener from comment #4)
> people that
> cannot be bothered to update their ucode or the kernel are not likely
> bothered to update libstdc++ either.

Fwiw, my kernel is fairly up-to-date (5.9.0-3-amd64); it and my libstdc++ get
updates pretty regularly from the distro, whereas (like most normal folks) I
don't tend to reflash my BIOS without a good reason.  (Which this is, I grant
you, but I only found out recently that it was causing me any issues, and I'm
holding off on the update in case affected projects want me to test mitigations
/ run additional experiments on the buggy hw.)  And while Debian can apply
updated ucode at boot-time, it's not enabled by default because the ucode
binaries are non-free.

So a user of a freedom-respecting OS, following normal routine update
procedures, will automatically get libstdc++ updates but not ucode updates.

But if, having considered it, you decide it's not worth it for libstdc++ to
mitigate this, that's fine by me — after all, now that I know about the issue,
I can fix my system.  Question is whether you want to try to protect other
users from hitting the same thing.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
g:b0c0d878a8b5bf39dbea4c192fed26d340524439 enabled RDRAND and RDSEED for AMD
chips, where previously we'd only used RDRAND and only for Intel (which caused
one particular idiot to complain that we "hate AMD"). I think we originally
didn't do it for AMD because of an older (but similar) bug:
https://bugzilla.kernel.org/show_bug.cgi?id=85911

N.B. If you know you have a broken chip and can change the source you can
select a different source, e.g. std::random_device rd("/dev/urandom").


I think this should work:

--- a/libstdc++-v3/src/c++11/random.cc
+++ b/libstdc++-v3/src/c++11/random.cc
@@ -101,6 +101,19 @@ namespace std _GLIBCXX_VISIBILITY(default)

       return val;
     }
+
+    bool
+    __attribute__ ((target("rdrnd")))
+    __x86_rdrand_is_usable()
+    {
+      for (int borkcount = 0; borkcount < 10; ++borkcount)
+       {
+         // AMD Ryzen 3000 bug, see PR libstdc++/100444
+         if (__x86_rdrand(nullptr) != 0xffffffff)
+           return true;
+       }
+      return false;
+    }
 #endif

 #if USE_RDSEED
@@ -271,7 +284,7 @@ namespace std _GLIBCXX_VISIBILITY(default)
 #ifdef USE_RDRAND
              // CPUID.01H:ECX.RDRAND[bit 30]
              __cpuid(1, eax, ebx, ecx, edx);
-             if (ecx & bit_RDRND)
+             if (ecx & bit_RDRND && __x86_rdrand_is_usable())
                {
                  _M_func = &__x86_rdseed_rdrand;
                  return;
@@ -297,8 +310,13 @@ namespace std _GLIBCXX_VISIBILITY(default)
          __cpuid(1, eax, ebx, ecx, edx);
          if (ecx & bit_RDRND)
            {
-             _M_func = &__x86_rdrand;
-             return;
+             if (__x86_rdrand_is_usable())
+               {
+                 _M_func = &__x86_rdrand;
+                 return;
+               }
+             else if (which == rdrand)
+               __throw_runtime_error(__N("random_device: RDRAND is buggy"));
            }
        }
     }

If you ask for "rdrand" specifically you'll get an exception telling you it's
buggy. If you just let the library choose for you, then it will skip rdrand and
pick the next alternative, which will be "/dev/urandom" for most systems.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note that if you read the errata you figure that the broken RDRAND behavior can
occur after a suspend/resume cycle only which means that detecting this in
the RDRAND detection logic is too "early" if you are unlucky.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Maybe we should go back to only using RDRAND for Intel :-(

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We could use a different function for AMD, rather than __x86_rdrand. If it
reads a -1 value it could try again a few more times, and if it gets -1 every
time assume it's broken (and throw?), maybe after being suspended. If it
doesn't get -1 every time, it was a real -1 value, return that -1 (not the next
values that were read, because otherwise we would never be able to return -1).

[Bug libstdc++/100444] std::random_device isn't random on AMD

[ecree429 at virginmedia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #12 from Edward Cree <ecree429 at virginmedia dot com> ---
A datum: booting with 'nordrand' on the kernel command line does not affect
this, the test program still prints 4294967295.  (Yet '/proc/cpuinfo' no longer
mentions rdrand or rdseed in flags.  Stdlib must be looking at cpuid directly
rather than using anything the kernel can mediate?)

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Edward Cree from comment #12)
> Stdlib must be looking at cpuid
> directly rather than using anything the kernel can mediate?)

Correct.

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2021-09-25
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org

[Bug libstdc++/100444] std::random_device isn't random on AMD

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100444

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Created attachment 51547
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51547&action=edit
Proposed patch