diff mbox series

[v8,6/7] random: early initialization of ChaCha constants

Message ID 20211229211009.108091-6-linux@dominikbrodowski.net
State New
Headers show
Series None | expand

Commit Message

Dominik Brodowski Dec. 29, 2021, 9:10 p.m. UTC
Previously, the ChaCha constants for the primary pool were only
initialized once rand_initialize() calls crng_initialize_primary().
However, some randomness is actually extracted from the primary pool
beforehand, e.g. by kmem_cache_create(). Therefore, statically
initialize the ChaCha constants for the primary pool.

In exchange, we can remove the dynamic initialization in
crng_initialize_primary(), as it is only called - as the name suggests -
for the primary pool. Therefore, no parameter to this function is needed.

Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/char/random.c   | 10 +++++++---
 include/crypto/chacha.h | 15 +++++++++++----
 2 files changed, 18 insertions(+), 7 deletions(-)

Comments

Jason A. Donenfeld Dec. 30, 2021, 2:40 p.m. UTC | #1
Thanks for the patch. Comments are inline below.

On Wed, Dec 29, 2021 at 10:13 PM Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
>  drivers/char/random.c   | 10 +++++++---
>  include/crypto/chacha.h | 15 +++++++++++----

For the next submission of this (which you can do standalone and call
a v2), please Cc linux-crypto and Herbert as part of the commit body.
I still intend to take this through the random tree, since that's the
purpose of it, but because it touches the lib/crypto code, they should
be in the loop.

>  static struct crng_state primary_crng = {
>         .lock = __SPIN_LOCK_UNLOCKED(primary_crng.lock),
> +       .state[0] = CHACHA_CONSTANT_EXPA, /* "expa" */
> +       .state[1] = CHACHA_CONSTANT_ND_3, /* "nd 3" */
> +       .state[2] = CHACHA_CONSTANT_2_BY, /* "2-by" */
> +       .state[3] = CHACHA_CONSTANT_TE_K, /* "te k" */
>  };

I don't think you need the comments here, since the constant is
already descriptive.

>
>  /*
> @@ -823,9 +827,9 @@ static void __maybe_unused crng_initialize_secondary(struct crng_state *crng)
>         crng->init_time = jiffies - CRNG_RESEED_INTERVAL - 1;
>  }
>
> -static void __init crng_initialize_primary(struct crng_state *crng)
> +static void __init crng_initialize_primary(void)
>  {
> +       struct crng_state *crng = &primary_crng;
> -       crng_initialize_primary(&primary_crng);
> +       crng_initialize_primary();

There are a bunch of places where we're passing around globals when we
could collapse them down. It probably makes sense to do that in a
separate cleanup series (please feel free!), rather than here, since
the init-time constants issue doesn't really change anything with
regards to this function signature.

>  static inline void chacha_init_consts(u32 *state)
>  {
> -       state[0]  = 0x61707865; /* "expa" */
> -       state[1]  = 0x3320646e; /* "nd 3" */
> -       state[2]  = 0x79622d32; /* "2-by" */
> -       state[3]  = 0x6b206574; /* "te k" */
> +       state[0]  = CHACHA_CONSTANT_EXPA; /* "expa" */
> +       state[1]  = CHACHA_CONSTANT_ND_3; /* "nd 3" */
> +       state[2]  = CHACHA_CONSTANT_2_BY; /* "2-by" */
> +       state[3]  = CHACHA_CONSTANT_TE_K; /* "te k" */
>  }

I don't think you need the comments here, since the constant is
already descriptive.
diff mbox series

Patch

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9b5eb6cf82ce..a5bf662578cb 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -457,6 +457,10 @@  struct crng_state {
 
 static struct crng_state primary_crng = {
 	.lock = __SPIN_LOCK_UNLOCKED(primary_crng.lock),
+	.state[0] = CHACHA_CONSTANT_EXPA, /* "expa" */
+	.state[1] = CHACHA_CONSTANT_ND_3, /* "nd 3" */
+	.state[2] = CHACHA_CONSTANT_2_BY, /* "2-by" */
+	.state[3] = CHACHA_CONSTANT_TE_K, /* "te k" */
 };
 
 /*
@@ -823,9 +827,9 @@  static void __maybe_unused crng_initialize_secondary(struct crng_state *crng)
 	crng->init_time = jiffies - CRNG_RESEED_INTERVAL - 1;
 }
 
-static void __init crng_initialize_primary(struct crng_state *crng)
+static void __init crng_initialize_primary(void)
 {
-	chacha_init_consts(crng->state);
+	struct crng_state *crng = &primary_crng;
 	_extract_entropy(&input_pool, &crng->state[4], sizeof(__u32) * 12, 0);
 	if (crng_init_try_arch_early(crng) && trust_cpu && crng_init < 2) {
 		invalidate_batched_entropy();
@@ -1797,7 +1801,7 @@  int __init rand_initialize(void)
 	init_std_data(&input_pool);
 	if (crng_need_final_init)
 		crng_finalize_init(&primary_crng);
-	crng_initialize_primary(&primary_crng);
+	crng_initialize_primary();
 	crng_global_init_time = jiffies;
 	if (ratelimit_disable) {
 		urandom_warning.interval = 0;
diff --git a/include/crypto/chacha.h b/include/crypto/chacha.h
index dabaee698718..147e56fc755e 100644
--- a/include/crypto/chacha.h
+++ b/include/crypto/chacha.h
@@ -47,12 +47,19 @@  static inline void hchacha_block(const u32 *state, u32 *out, int nrounds)
 		hchacha_block_generic(state, out, nrounds);
 }
 
+enum chacha_constants { /* expand 32-byte k */
+	CHACHA_CONSTANT_EXPA = 0x61707865U,
+	CHACHA_CONSTANT_ND_3 = 0x3320646eU,
+	CHACHA_CONSTANT_2_BY = 0x79622d32U,
+	CHACHA_CONSTANT_TE_K = 0x6b206574U
+};
+
 static inline void chacha_init_consts(u32 *state)
 {
-	state[0]  = 0x61707865; /* "expa" */
-	state[1]  = 0x3320646e; /* "nd 3" */
-	state[2]  = 0x79622d32; /* "2-by" */
-	state[3]  = 0x6b206574; /* "te k" */
+	state[0]  = CHACHA_CONSTANT_EXPA; /* "expa" */
+	state[1]  = CHACHA_CONSTANT_ND_3; /* "nd 3" */
+	state[2]  = CHACHA_CONSTANT_2_BY; /* "2-by" */
+	state[3]  = CHACHA_CONSTANT_TE_K; /* "te k" */
 }
 
 void chacha_init_arch(u32 *state, const u32 *key, const u8 *iv);