All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
@ 2022-01-13 13:12 Ard Biesheuvel
  2022-01-13 13:39 ` Mark Brown
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2022-01-13 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, will, mark.rutland, jason, Ard Biesheuvel,
	Andre Przywara, Mark Brown

When support for RNDR/RNDRRS was introduced, we elected to only
implement arch_get_random_seed_int/_long(), and back them by RNDR
instead of RNDRRS. This was needed to prevent potential performance
and/or starvation issues resulting from the fact that the /dev/random
driver used to invoke these routines on various hot paths.

These issues have all been addressed now [0] [1], and so we can wire up
this API more straight-forwardly:

- map arch_get_random_int/_long() onto RNDR, which returns the output of
  a DRBG that is reseeded at an implemented defined rate;
- map arch_get_random_seed_int/_long() onto the TRNG firmware service,
  which returns true, conditioned entropy, or onto RNDRRS if the TRNG
  service is unavailable, which returns the output of a DRBG that is
  reseeded every time it is used.

[0] 390596c9959c random: avoid arch_get_random_seed_long() when collecting IRQ randomness
[1] 2ee25b6968b1 random: avoid superfluous call to RDRAND in CRNG extraction

Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/archrandom.h | 45 +++++++++++++++++---
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
index 09e43272ccb0..d1bb5e71df25 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -42,13 +42,47 @@ static inline bool __arm64_rndr(unsigned long *v)
 	return ok;
 }
 
+static inline bool __arm64_rndrrs(unsigned long *v)
+{
+	bool ok;
+
+	/*
+	 * Reads of RNDRRS set PSTATE.NZCV to 0b0000 on success,
+	 * and set PSTATE.NZCV to 0b0100 otherwise.
+	 */
+	asm volatile(
+		__mrs_s("%0", SYS_RNDRRS_EL0) "\n"
+	"	cset %w1, ne\n"
+	: "=r" (*v), "=r" (ok)
+	:
+	: "cc");
+
+	return ok;
+}
+
 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;
 }
 
 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;
 }
 
@@ -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.
 	 */
-	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;
 		}
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:12 [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR Ard Biesheuvel
@ 2022-01-13 13:39 ` Mark Brown
  2022-01-13 13:41 ` Jason A. Donenfeld
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2022-01-13 13:39 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, catalin.marinas, will, mark.rutland, jason,
	Andre Przywara


[-- Attachment #1.1: Type: text/plain, Size: 447 bytes --]

On Thu, Jan 13, 2022 at 02:12:39PM +0100, Ard Biesheuvel wrote:
> When support for RNDR/RNDRRS was introduced, we elected to only
> implement arch_get_random_seed_int/_long(), and back them by RNDR
> instead of RNDRRS. This was needed to prevent potential performance
> and/or starvation issues resulting from the fact that the /dev/random
> driver used to invoke these routines on various hot paths.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:12 [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR Ard Biesheuvel
  2022-01-13 13:39 ` Mark Brown
@ 2022-01-13 13:41 ` Jason A. Donenfeld
  2022-01-13 13:49   ` Ard Biesheuvel
  2022-01-14 15:04   ` Andre Przywara
  2022-01-14 15:07 ` Andre Przywara
  2022-02-15 23:18 ` Will Deacon
  3 siblings, 2 replies; 8+ messages in thread
From: Jason A. Donenfeld @ 2022-01-13 13:41 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, catalin.marinas, will, mark.rutland,
	Andre Przywara, Mark Brown

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:41 ` Jason A. Donenfeld
@ 2022-01-13 13:49   ` Ard Biesheuvel
  2022-01-13 13:52     ` Jason A. Donenfeld
  2022-01-14 15:04   ` Andre Przywara
  1 sibling, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2022-01-13 13:49 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Linux ARM, Catalin Marinas, Will Deacon, Mark Rutland,
	Andre Przywara, Mark Brown

On Thu, 13 Jan 2022 at 14:41, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> 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?
>

Ack

> >  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);
>

Sure, but I just retained the original style.

> >
> >  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;
> }
>
>

If I was interested in rewriting this header file, I might consider
all these options. For now, I am just trying to focus the change on
the parts that actually matter.

> > @@ -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.
>

No it does not. RNDR and RNDRRS both return the output of a DRBG, the
only difference is the reseed interval.

Specifically, this means that, even though the ARM ARM references NIST
SP800-90B directly, the RNDRRS construction is a black box containing
a entropy source + DRBG, and so we shouldn't pretend that RNDRRS
itself can be treated as a source of true entropy. This is especially
relevant when it comes to seeding a DRBG of a certain security
strength N >= the security strength of the hidden DRBG, as
concatenating multiple RNDRRS results does not satisfy the
requirements for seeding a DRBG of security strength N.

> >        */
> > -     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,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:49   ` Ard Biesheuvel
@ 2022-01-13 13:52     ` Jason A. Donenfeld
  0 siblings, 0 replies; 8+ messages in thread
From: Jason A. Donenfeld @ 2022-01-13 13:52 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux ARM, Catalin Marinas, Will Deacon, Mark Rutland,
	Andre Przywara, Mark Brown

On Thu, Jan 13, 2022 at 2:49 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> Sure, but I just retained the original style.
> If I was interested in rewriting this header file, I might consider
> all these options. For now, I am just trying to focus the change on
> the parts that actually matter.

Okay, no problem.

> No it does not. RNDR and RNDRRS both return the output of a DRBG, the
> only difference is the reseed interval.
>
> Specifically, this means that, even though the ARM ARM references NIST
> SP800-90B directly, the RNDRRS construction is a black box containing
> a entropy source + DRBG, and so we shouldn't pretend that RNDRRS
> itself can be treated as a source of true entropy. This is especially
> relevant when it comes to seeding a DRBG of a certain security
> strength N >= the security strength of the hidden DRBG, as
> concatenating multiple RNDRRS results does not satisfy the
> requirements for seeding a DRBG of security strength N.

Huh, interesting. I wonder why that arm.com documentation page is wrong.

Jason

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:41 ` Jason A. Donenfeld
  2022-01-13 13:49   ` Ard Biesheuvel
@ 2022-01-14 15:04   ` Andre Przywara
  1 sibling, 0 replies; 8+ messages in thread
From: Andre Przywara @ 2022-01-14 15:04 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Ard Biesheuvel, linux-arm-kernel, catalin.marinas, will,
	mark.rutland, Mark Brown

On Thu, 13 Jan 2022 14:41:16 +0100
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

Hi Jason,

> 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);

It could, but this pattern is used in the other functions to simplify
having extensions later.

> >  
> >  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:

arch_get_random_long() might drain more entropy than needed, at least this
is a problem for the TRNG version. For the RNDR* instructions this
doesn't really matter, since they are always using 64-bit system
registers, but for the SMC interface there is a difference between
asking for 32 or 64 bits.
So this is admittedly not a problem in *this particular case*, but I'd
consider this an implementation detail of the sources used in the current
code, and for the sake of expandability it seems more robust to use this
approach.

Cheers,
Andre

> 
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:12 [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR Ard Biesheuvel
  2022-01-13 13:39 ` Mark Brown
  2022-01-13 13:41 ` Jason A. Donenfeld
@ 2022-01-14 15:07 ` Andre Przywara
  2022-02-15 23:18 ` Will Deacon
  3 siblings, 0 replies; 8+ messages in thread
From: Andre Przywara @ 2022-01-14 15:07 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, catalin.marinas, will, mark.rutland, jason, Mark Brown

On Thu, 13 Jan 2022 14:12:39 +0100
Ard Biesheuvel <ardb@kernel.org> wrote:

Hi,

> When support for RNDR/RNDRRS was introduced, we elected to only
> implement arch_get_random_seed_int/_long(), and back them by RNDR
> instead of RNDRRS. This was needed to prevent potential performance
> and/or starvation issues resulting from the fact that the /dev/random
> driver used to invoke these routines on various hot paths.
> 
> These issues have all been addressed now [0] [1], and so we can wire up
> this API more straight-forwardly:
> 
> - map arch_get_random_int/_long() onto RNDR, which returns the output of
>   a DRBG that is reseeded at an implemented defined rate;
> - map arch_get_random_seed_int/_long() onto the TRNG firmware service,
>   which returns true, conditioned entropy, or onto RNDRRS if the TRNG
>   service is unavailable, which returns the output of a DRBG that is
>   reseeded every time it is used.
> 
> [0] 390596c9959c random: avoid arch_get_random_seed_long() when collecting IRQ randomness
> [1] 2ee25b6968b1 random: avoid superfluous call to RDRAND in CRNG extraction
> 
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Looks alright to me, at least from the code point of view: I am not a
"crypto guy".

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/arm64/include/asm/archrandom.h | 45 +++++++++++++++++---
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index 09e43272ccb0..d1bb5e71df25 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -42,13 +42,47 @@ static inline bool __arm64_rndr(unsigned long *v)
>  	return ok;
>  }
>  
> +static inline bool __arm64_rndrrs(unsigned long *v)
> +{
> +	bool ok;
> +
> +	/*
> +	 * Reads of RNDRRS set PSTATE.NZCV to 0b0000 on success,
> +	 * and set PSTATE.NZCV to 0b0100 otherwise.
> +	 */
> +	asm volatile(
> +		__mrs_s("%0", SYS_RNDRRS_EL0) "\n"
> +	"	cset %w1, ne\n"
> +	: "=r" (*v), "=r" (ok)
> +	:
> +	: "cc");
> +
> +	return ok;
> +}
> +
>  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;
>  }
>  
>  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;
>  }
>  
> @@ -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.
>  	 */
> -	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;
>  		}


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR
  2022-01-13 13:12 [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2022-01-14 15:07 ` Andre Przywara
@ 2022-02-15 23:18 ` Will Deacon
  3 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2022-02-15 23:18 UTC (permalink / raw)
  To: linux-arm-kernel, Ard Biesheuvel
  Cc: catalin.marinas, kernel-team, Will Deacon, mark.rutland,
	Mark Brown, Andre Przywara, jason

On Thu, 13 Jan 2022 14:12:39 +0100, Ard Biesheuvel wrote:
> When support for RNDR/RNDRRS was introduced, we elected to only
> implement arch_get_random_seed_int/_long(), and back them by RNDR
> instead of RNDRRS. This was needed to prevent potential performance
> and/or starvation issues resulting from the fact that the /dev/random
> driver used to invoke these routines on various hot paths.
> 
> These issues have all been addressed now [0] [1], and so we can wire up
> this API more straight-forwardly:
> 
> [...]

Applied to arm64 (for-next/rng), thanks!

[1/1] arm64: random: implement arch_get_random_int/_long based on RNDR
      https://git.kernel.org/arm64/c/35bde68bba54

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-02-15 23:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 13:12 [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR Ard Biesheuvel
2022-01-13 13:39 ` Mark Brown
2022-01-13 13:41 ` Jason A. Donenfeld
2022-01-13 13:49   ` Ard Biesheuvel
2022-01-13 13:52     ` Jason A. Donenfeld
2022-01-14 15:04   ` Andre Przywara
2022-01-14 15:07 ` Andre Przywara
2022-02-15 23:18 ` Will Deacon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.