linux-kernel - Re: [PATCH 1/2] x86/random: Retry on RDSEED failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHmME9pOt=uEmuBzBpgUHw9DqAD2FZTZ3v53AOZbQ3Cd2p97xQ@mail.gmail.com>
Date: Tue, 30 Jan 2024 13:29:10 +0100
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org, 
	"Theodore Ts'o" <tytso@....edu>, 
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>, 
	Elena Reshetova <elena.reshetova@...el.com>, Jun Nakajima <jun.nakajima@...el.com>, 
	Tom Lendacky <thomas.lendacky@....com>, "Kalra, Ashish" <ashish.kalra@....com>, 
	Sean Christopherson <seanjc@...gle.com>, linux-coco@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] x86/random: Retry on RDSEED failure

Hi Kirill,

I've been following the other discussion closely thinking about the
matter, but I suppose I'll jump in here directly on this patch, if
this is the approach the discussion is congealing around.

A comment below:

On Tue, Jan 30, 2024 at 9:30 AM Kirill A. Shutemov
<kirill.shutemov@...ux.intel.com> wrote:
>  static inline bool __must_check rdseed_long(unsigned long *v)
>  {
> +       unsigned int retry = RDRAND_RETRY_LOOPS;
>         bool ok;
> -       asm volatile("rdseed %[out]"
> -                    CC_SET(c)
> -                    : CC_OUT(c) (ok), [out] "=r" (*v));
> -       return ok;
> +
> +       do {
> +               asm volatile("rdseed %[out]"
> +                            CC_SET(c)
> +                            : CC_OUT(c) (ok), [out] "=r" (*v));
> +
> +               if (ok)
> +                       return true;
> +       } while (--retry);
> +
> +       return false;
>  }

So, my understanding of RDRAND vs RDSEED -- deliberately leaving out
any cryptographic discussion here -- is roughly that RDRAND will
expand the seed material for longer, while RDSEED will mostly always
try to sample more bits from the environment. AES is fast, while
sampling is slow, so RDRAND gives better performance and is less
likely to fail, whereas RDSEED always has to wait on the hardware to
collect some bits, so is more likely to fail.

For that reason, most of the usage of RDRAND and RDSEED inside of
random.c is something to the tune of `if (!rdseed(out)) rdrand(out);`,
first trying RDSEED but falling back to RDRAND if it's busy. That
still seems to me like a reasonable approach, which this patch would
partly undermine (in concert with the next patch, which I'll comment
on in a follow up email there).

So maybe this patch #1 (of 2) can be dropped?

Jason