Re: [PATCH v13 6/7] random: introduce generic vDSO getrandom() implementation

On Wed, Dec 21, 2022 at 02:23:39PM -0800, Eric Biggers wrote:

On Wed, Dec 21, 2022 at 03:23:26PM +0100, Jason A. Donenfeld wrote:

quoted

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 6425f5f838e0..660cd15b6228 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c

@@ -60,6 +60,7 @@
 #include <crypto/blake2s.h>
 #ifdef CONFIG_VDSO_GETRANDOM
 #include <vdso/getrandom.h>
+#include <vdso/datapage.h>
 #endif
 #include <asm/archrandom.h>
 #include <asm/processor.h>

@@ -407,6 +408,9 @@ static void crng_reseed(struct work_struct *work)

/*
 * We copy the new key into the base_crng, overwriting the old one,
 * and update the generation counter. We avoid hitting ULONG_MAX,
 * because the per-cpu crngs are initialized to ULONG_MAX, so this
 * forces new CPUs that come online to always initialize.
 */
spin_lock_irqsave(&base_crng.lock, flags);
memcpy(base_crng.key, key, sizeof(base_crng.key));
next_gen = base_crng.generation + 1;
 	if (next_gen == ULONG_MAX)
 		++next_gen;
 	WRITE_ONCE(base_crng.generation, next_gen);
+#ifdef CONFIG_VDSO_GETRANDOM
+	smp_store_release(&_vdso_rng_data.generation, next_gen + 1);
+#endif

It's confusing that "uninitialized generation" is ULONG_MAX in the per-cpu
crngs, but 0 in the vdso_rng_data.  That results in a weird off-by one thing,
where the vdso_rng_data generation number has to be 1 higher.

Would it be possible to use 0 for both?

It might be, but this will involve some changes to how the batching
works too, so I think I'd like to do that separately, if at all.
However, I'll add a comment there noting what's happening so it's a bit
less confusing.

Jason