Re: kern/54655: cpu_rng_rdseed() should check support of RDRAND instruction

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

> I don't know is there real CPU supports RDSEED without RDRAND or not.

I grepped InstLatx64(https://github.com/InstLatx64/InstLatx64)'s CPUID
outputs and there is no any CPU who has RDSEED but not RDRAND.

> But there is such a case on VPS service at least.

Even though there is no such real CPU, we should check the existence
individually because VM environment tend to drop some CPUID bits
because of the security, so I think your proposal patch is OK.

>> Fix:
> Check support of RDRAND in cpu_rng_rdseed():
> 
> diff --git a/sys/arch/x86/x86/cpu_rng.c b/sys/arch/x86/x86/cpu_rng.c
> index 3b79d768ea..c716eeaa26 100644
> --- a/sys/arch/x86/x86/cpu_rng.c
> +++ b/sys/arch/x86/x86/cpu_rng.c
> @@ -53,6 +53,8 @@ static enum {
>  	CPU_RNG_VIA
>  } cpu_rng_mode __read_mostly = CPU_RNG_NONE;
>  
> +static bool has_rdrand;
> +
>  bool
>  cpu_rng_init(void)
>  {
> @@ -131,7 +133,10 @@ cpu_rng_rdseed(cpu_rng_t *out)
>  	 * to be seeded even in this case.
>  	 */
>  exhausted:
> -	return cpu_rng_rdrand(out);
> +	if (has_rdrand)
> +		return cpu_rng_rdrand(out);
> +	else
> +		return 0;
>  }
>  
>  static size_t
> @@ -213,7 +218,7 @@ cpu_earlyrng(void *out, size_t sz)
>  	int i;
>  
>  	bool has_rdseed = (cpu_feature[5] & CPUID_SEF_RDSEED) != 0;
> -	bool has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;
> +	has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;

It's not related to this PR, we should avoid referring the boot
processor's cpu_feature[] in future for Lakefield and Elkhart Lake.

>  
>  	KASSERT(sz + sizeof(uint64_t) <= SHA512_DIGEST_LENGTH);
>  
> 
>> Unformatted:
>  	
>  	
> 


-- 
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)