Message-ID: <1964951.LkxdtWsSYb@tjmaciei-mobl5>
Date: Tue, 04 Nov 2025 10:08:48 -0800
From: Thiago Macieira <thiago.macieira@...el.com>
To: Borislav Petkov <bp@...en8.de>, "Jason A. Donenfeld" <Jason@...c4.com>
Cc: Christopher Snowhill <chris@...e54.net>,
 Gregory Price <gourry@...rry.net>, x86@...nel.org,
 linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
 dave.hansen@...ux.intel.com, hpa@...or.com, peterz@...radead.org,
 mario.limonciello@....com, riel@...riel.com, yazen.ghannam@....com,
 me@...aill.net, kai.huang@...el.com, sandipan.das@....com,
 darwi@...utronix.de, stable@...r.kernel.org
Subject:
 Re: [PATCH v2] x86/amd: Disable RDSEED on AMD Zen5 because of an error.

On Tuesday, 4 November 2025 06:28:07 Pacific Standard Time Jason A. Donenfeld 
wrote:
> On Tue, Nov 4, 2025 at 2:21 PM Borislav Petkov <bp@...en8.de> wrote:
> > And the problem here is that, AFAICT, Qt is not providing a proper
> > fallback
> > for !RDSEED. Dunno, maybe getrandom(2) or so. It is only a syscall which
> > has been there since forever. Rather, it would simply throw hands in the
> > air.
> Qt seems to be kinda wild.

Hello Jason

> When you use -mcpu=, you generally can then omit cpuid checks. That's
> the whole idea. But then Qt checks cpuid anyway and compares it to the
> -mcpu= feature set and aborts early. This mismatch happens in the case
> Christopher is complaining about when the kernel has masked that out
> of the cpuid, due to bugs. But I guess if it just wouldn't check the
> cpuid, it would have worked anyway, modulo the actual cpu bug. But
> regarding rdseed/rand bugs, there's a workaround for earlier AMD
> rdrand bugs:
> https://github.com/qt/qtbase/blob/dev/src/corelib/global/qsimd.cpp#L781 But
> then it skips that for -mcpu= with `(_compilerCpuFeatures &
> CpuFeatureRDRND)`. Weird.

The general theory is that if you ask the compiler to enable a feature, it's 
because you know you're going to run on a CPU with that particular feature and 
therefore we can remove the check.

That includes RDRND: checkRdrndWorks() has:

    // But if the RDRND feature was statically enabled by the compiler, we
    // assume that the RNG works. That's because the calls to qRandomCpu() 
will
    // be guarded by qCpuHasFeature(RDRND) and that will be a constant true.
    if (_compilerCpuFeatures & CpuFeatureRDRND)
        return true;

The code you pointed out above is guarded by this particular piece of code:

    if (features & CpuFeatureRDRND && !checkRdrndWorks(features))
        features &= ~(CpuFeatureRDRND | CpuFeatureRDSEED);

As you said, we do have some code to print the CPU feature mismatch on load, 
so as to avoid crashing with SIGILL. But it won't apply for the broken RDRND 
case, because the side effect of that code is we assume it isn't broken in the 
first place. That's because we're optimising for the case where it isn't 
broken, which I find reasonable.

> Another strange thing, though, is the way this is actually used. As
> far as I can tell from reading this messy source,
> QRandomGenerator::SystemGenerator::generate() tries in order:
> 
> 1. rdseed
> 2. rdrand
> 3. getentropy (getrandom)
> 4. /dev/urandom
> 5. /dev/random
> 6. Something ridiculous using mt19937

#1 and #2 are a runtime decision. If they fail due to lack of entropy or are 
unavailable, we will use getentropy().

#3 is mutually exclusive with #4 and #5. We enable getentropy() if your glibc 
has it at compile time, or we use /dev/urandom if it doesn't. There's a marker 
in the ELF header then indicating we can't run in a kernel without 
getrandom().

#6 will never be used on Linux. That monstrosity is actually compiled out of 
existence on Linux, BSDs, and Windows (in spite of mentioning Linux in the 
source). It's only there as a final fallback for systems I don't really care 
about and can't test anyway.

> In addition to rdseed really not being appropriate here, in order to
> have seeds for option (6), no matter what happens with 1,2,3,4,5, it
> always stores the first 4 bytes of output from previous calls, just in
> case at some point it needs to use (6). Goodbye forward secrecy? And
> does this mt19937 stuff leak? And also, wtf?

See above.

What do you mean about RDSEED? Should it not be used? Note that 
QRandomGenerator is often used to seed a PRNG, so it seemed correct to me to 
use it.

> This is totally strange. It should just be using getrandom() and
> falling back to /dev/urandom for old kernels unavailable. Full stop.
> Actually, src/corelib/global/minimum-linux_p.h suggests 4.11 is
> required ("We require the following features in Qt (unconditional, no
> fallback)"), so it could replace basically this entire file with
> getentropy() for unix and rtlgenrandom for windows.

When this was originally written, getrandom() wasn't generally available and 
the glibc wrapper even less so, meaning the code path usually went through the 
read() syscall. Using RDRAND seemed like a good idea to avoid the transition 
into kernel mode.

I still think so, even with getrandom(). Though, with the new vDSO support for 
userspace generation, that bears reevaluation. 

There's also the issue of being cross-platform. Because my primary system is 
Linux, I prefer to have as little differentiation from it as I can get away 
with, so I can test what other users may see. However, I will not hesitate to 
write code that is fast only on Linux and let other OSes deal with their own 
shortcomings (q.v. qstorageinfo_linux.cpp, qnetworkinterface_linux.cpp, 
support for glibc-hwcaps). In this case, I'm not convinced there's benefit for 
Linux by bypassing the RDRND check and going straight to getentropy()/
getrandom().

> Sounds great, like we should just use QRandomGenerator::global() for
> everything, right? Wrong. It turns out QRandomGenerator::system() is
> the one that uses 1,2,3,4,5,(6godforbid) in my email above.
> QRandomGenerator::global(), on the contrary uses
> "std::mersenne_twister_engine<quint32,32,624,397,31,0x9908b0df,11,0xffffffff
> ,7,0x9d2c5680,15,0xefc60000,18,1812433253>".

The separation is because I was convinced, at the time of developing the code, 
that advocating that people use a system-wide resource like the RDRND or 
getrandom() entropy for everything was bad advice. So, instead, we create our 
own per-process PRNG, securely seed it from that shared resource, and then let 
people use it for their own weird needs. Like creating random strings.

And it's also expected that if you know you need something more than baseline, 
you'll be well-versed in the lingo to understand the difference between 
global() and system().

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Principal Engineer - Intel DCG - Platform & Sys. Eng.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5150 bytes)

Powered by blists - more mailing lists