Re: [PATCH] random: remove CONFIG_ARCH_RANDOM and "nordrand"

Jason A. Donenfeld
2022-07-05T19:44:29Z

Hey Borislav,

On Tue, Jul 05, 2022 at 09:36:20PM +0200, Borislav Petkov wrote:
> On Tue, Jul 05, 2022 at 09:01:21PM +0200, Jason A. Donenfeld wrote:
> > @@ -26,7 +18,6 @@ __setup("nordrand", x86_rdrand_setup);
> > */
> > #define SANITY_CHECK_LOOPS 8
> >
> > -#ifdef CONFIG_ARCH_RANDOM
> > void x86_init_rdrand(struct cpuinfo_x86 *c)
> > {
> > unsigned int changed = 0;
> > @@ -59,8 +50,6 @@ void x86_init_rdrand(struct cpuinfo_x86 *c)
> > }
> >
> > if (WARN_ON_ONCE(!changed))
> > - pr_emerg(
> > -"RDRAND gives funky smelling output, might consider not using it by booting with \"nordrand\"");
> > + pr_emerg("RDRAND gives funky smelling output; update microcode or firmware.");
>
> It is highly unlikely to get a BIOS or microcode update for that matter,
> for old systems:
>
> 7879fc4bdc75 ("x86/rdrand: Sanity-check RDRAND output")
>
> so I guess here you're better off saying that the kernel simply disables
> rdrand support and do
>
> clear_cpu_cap(c, X86_FEATURE_RDRAND);
>
> here too.

Oh, huh. Maybe in that case I should adjust the message to say "consider
using `random.trust_cpu=0`," which is the thing that would actually make
a security difference.

But actually, one thing that wasn't clear to me was: does `nordrand`
affect what userspace sees? While random.c is okay in lots of
circumstances, I could imagine `nordrand` playing a role in preventing
userspace from using it, which might be desirable. Is this the case? If
so, I can remove the nordrand chunk from this patch for v2. If not, I'll
adjust the text to mention `random.trust_cpu=0`.

> If I read the commit message above correctly, it sounds like RDRAND
> output is not that important anyway...

In the sense that random.c can handle mostly any input without making
the quality worse. So, you can't accidentally taint it. The only risk is
if it thinks RDRAND is good and trustable when it isn't, but that's what
`random.trust_cpu=0` is for.

Jason