Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR

["Jason A. Donenfeld" <Jason@zx2c4.com>]

Hi Ard,

Wow, didn't expect for this to come so fast. Excellent.

On Thu, Jan 13, 2022 at 02:12:39PM +0100, Ard Biesheuvel wrote:
> - map arch_get_random_int/_long() onto RNDR, which returns the output of
>   a DRBG that is reseeded at an implemented defined rate;

implemented -> implementation?

>  static inline bool __must_check arch_get_random_long(unsigned long *v)
>  {
> +	/*
> +	 * Only support the generic interface after we have detected
> +	 * the system wide capability, avoiding complexity with the
> +	 * cpufeature code and with potential scheduling between CPUs
> +	 * with and without the feature.
> +	 */
> +	if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v))
> +		return true;
>  	return false;
>  }

Can't this just become:

  return cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v);

>  
>  static inline bool __must_check arch_get_random_int(unsigned int *v)
>  {
> +	if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> +		unsigned long val;
> +
> +		if (__arm64_rndr(&val)) {
> +			*v = val;
> +			return true;
> +		}
> +	}
>  	return false;
>  }

Why not implement arch_get_random_int with the same type of flow as
arch_get_random_long?

static inline bool __must_check arch_get_random_int(unsigned int *v)
{
	unsigned long val;
	if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(&val))) {
		*v = val;
		return true;
	}
	return false;
}

Or, even better, just define arch_get_random_int in terms of
arch_get_random_long:

static inline bool __must_check arch_get_random_int(unsigned int *v)
{
	unsigned long val;
	if (arch_get_random_long(&val)) {
		*v = val;
		return true;
	}
	return false;
}


> @@ -71,12 +105,11 @@ static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
>  	}
>  
>  	/*
> -	 * Only support the generic interface after we have detected
> -	 * the system wide capability, avoiding complexity with the
> -	 * cpufeature code and with potential scheduling between CPUs
> -	 * with and without the feature.
> +	 * RNDRRS is not backed by an entropy source but by a DRBG that is
> +	 * reseeded after each invocation. This is not a 100% fit but good
> +	 * enough to implement this API if no other entropy source exists.

The docs are actually a bit more optimistic than that:

https://developer.arm.com/documentation/ddi0595/2021-03/AArch64-Registers/RNDRRS--Reseeded-Random-Number

 ~ Reseeded Random Number. Returns a 64-bit random number which is reseeded
 ~ from the True Random Number source immediately before the read of the
 ~ random number.

If I'm reading that correctly, it looks like the reseeding happens
*before* the read, and it looks like it comes from a TRNG. In
other words, it sounds to me like it's just doing something like
HASH(READ_TRNG()). That would be pretty darn good.

>  	 */
> -	if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v))
> +	if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndrrs(v))
>  		return true;
>  
>  	return false;
> @@ -96,7 +129,7 @@ static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
>  	}
>  
>  	if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> -		if (__arm64_rndr(&val)) {
> +		if (__arm64_rndrrs(&val)) {
>  			*v = val;
>  			return true;
>  		}

I suppose the same control flow simplification stuff mentioned above
could be done here too, if you feel like what I mentioned earlier is
worthwhile.

From a randomness perspective:

Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Thanks,
Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel