Messages in this thread | | | From | "Jason A. Donenfeld" <> | Date | Thu, 30 Dec 2021 15:40:04 +0100 | Subject | Re: [PATCH v8 6/7] random: early initialization of ChaCha constants |
| |
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.
| |