mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] random: publish crng_init with release semantics
@ 2026-09-22  1:26 Jaidev Shastri via B4 Relay
  2026-09-23 22:46 ` Jason A. Donenfeld
  0 siblings, 1 reply; 2+ messages in thread
From: Jaidev Shastri via B4 Relay @ 2026-09-22  1:26 UTC (permalink / raw)
  To: Theodore Ts'o, Jason A. Donenfeld; +Cc: linux-kernel, Jaidev Shastri

From: Jaidev Shastri <jaidevshastri@vt.edu>

crng_reseed() writes the new base key and bumps base_crng.generation
under base_crng.lock, then sets crng_init to CRNG_READY with a plain
store. crng_ready() reads crng_init with a plain load and without the
lock, on the get_random_u8(), u16(), u32() and u64() fast paths and in
crng_make_state().

Set the state with smp_store_release() and read it with
smp_load_acquire(), so that a reader observing CRNG_READY also observes
the key and the generation written before it. crng_ready() becomes a
static inline function so that the acquire can carry its comment.

Found with MBCheck, a static herd7-based memory consistency checker.

Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
---
 drivers/char/random.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b4da1fb97..2418c787c 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -83,7 +83,11 @@ static enum {
 	CRNG_READY = 2  /* Fully initialized with POOL_READY_BITS collected */
 } crng_init __read_mostly = CRNG_EMPTY;
 static DEFINE_STATIC_KEY_FALSE(crng_is_ready);
-#define crng_ready() (static_branch_likely(&crng_is_ready) || crng_init >= CRNG_READY)
+static inline bool crng_ready(void)
+{
+	/* Pairs with the smp_store_release() of crng_init in crng_reseed(). */
+	return static_branch_likely(&crng_is_ready) || smp_load_acquire(&crng_init) >= CRNG_READY;
+}
 /* Various types of waiters for crng_init->CRNG_READY transition. */
 static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);
 static struct fasync_struct *fasync;
@@ -282,8 +286,14 @@ static void crng_reseed(struct work_struct *work)
 	if (IS_ENABLED(CONFIG_VDSO_GETRANDOM))
 		smp_store_release((unsigned long *)&vdso_k_rng_data->generation, next_gen + 1);
 
-	if (!static_branch_likely(&crng_is_ready))
-		crng_init = CRNG_READY;
+	if (!static_branch_likely(&crng_is_ready)) {
+		/*
+		 * crng_ready() tests crng_init without base_crng.lock on the
+		 * fast paths. Publish the state after the new key and the
+		 * generation with release semantics.
+		 */
+		smp_store_release(&crng_init, CRNG_READY);
+	}
 	spin_unlock_irqrestore(&base_crng.lock, flags);
 	memzero_explicit(key, sizeof(key));
 }

---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260921-mb-random-9b19c9136838

Best regards,
--  
Jaidev Shastri <jaidevshastri@vt.edu>



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

* Re: [PATCH] random: publish crng_init with release semantics
  2026-09-22  1:26 [PATCH] random: publish crng_init with release semantics Jaidev Shastri via B4 Relay
@ 2026-09-23 22:46 ` Jason A. Donenfeld
  0 siblings, 0 replies; 2+ messages in thread
From: Jason A. Donenfeld @ 2026-09-23 22:46 UTC (permalink / raw)
  To: jaidevshastri; +Cc: Theodore Ts'o, linux-kernel

Hi Jaidev,

On Mon, Sep 21, 2026 at 09:26:21PM -0400, Jaidev Shastri via B4 Relay wrote:
> From: Jaidev Shastri <jaidevshastri@vt.edu>
> 
> crng_reseed() writes the new base key and bumps base_crng.generation
> under base_crng.lock, then sets crng_init to CRNG_READY with a plain
> store. crng_ready() reads crng_init with a plain load and without the
> lock, on the get_random_u8(), u16(), u32() and u64() fast paths and in
> crng_make_state().
> 
> Set the state with smp_store_release() and read it with
> smp_load_acquire(), so that a reader observing CRNG_READY also observes
> the key and the generation written before it. crng_ready() becomes a
> static inline function so that the acquire can carry its comment.
> 
> Found with MBCheck, a static herd7-based memory consistency checker.

It's "intentionally" like this actually. By that, I mean that I thought
about it when writing it and decided against it. But maybe my analysis
is silly. Here's the thinking:

crng_init only ever becomes CRNG_READY. It never goes from a ready state
to an unready state. And it becomes ready at a quasi "random" time. So
all the code around it is used to various things happening in a
potentially unready state. Once it's ready, however, it never becomes
unready.

So if it changes to ready, but the cores don't see the change for, say 5
whole seconds (several orders of magnitude longer than what's
realistic), then that's fine. They will _eventually_ see that it's
ready, in the same way that the rng itself _eventually_ becomes ready.
So I didn't think it was necessary for the cores to see that it's ready
at exactly the moment that it is ready and not a second after.

In other words, the consequences of this racing are basically nothing.

Does this make sense? If I've overlooked something, please do let me
know.

Thanks,
Jason

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

end of thread, other threads:[~2026-09-23 22:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22  1:26 [PATCH] random: publish crng_init with release semantics Jaidev Shastri via B4 Relay
2026-09-23 22:46 ` Jason A. Donenfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®