* [PATCH 0/2] random: fix write locking for crng_init
@ 2022-02-05 10:34 Dominik Brodowski
2022-02-05 10:34 ` [PATCH 1/2] random: fix locking in crng_fast_load() Dominik Brodowski
2022-02-05 10:34 ` [PATCH 2/2] random: fix locking for crng_init in crng_reseed() Dominik Brodowski
0 siblings, 2 replies; 6+ messages in thread
From: Dominik Brodowski @ 2022-02-05 10:34 UTC (permalink / raw)
To: Jason A . Donenfeld, Herbert Xu; +Cc: linux-kernel, linux-crypto
According to a comment in random.c, crng_init is protected by
primary_crng->lock. These two patches fix the locking for
writing tp (that is: increasing) crng_init in call sites where it
may matter. At rand_initialize() time (precisely: either in
crng_initialize_primary() or in crng_finalize_init()), crng_init
is set to 2 without the lock being held. However, then the
kernel is running with IRQs disabled and only the boot CPU
active (but not yet in PID 1).
Dominik Brodowski (2):
random: fix locking in crng_fast_load()
random: fix locking for crng_init in crng_reseed()
drivers/char/random.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] random: fix locking in crng_fast_load()
2022-02-05 10:34 [PATCH 0/2] random: fix write locking for crng_init Dominik Brodowski
@ 2022-02-05 10:34 ` Dominik Brodowski
2022-02-05 13:17 ` Jason A. Donenfeld
2022-02-21 2:32 ` Eric Biggers
2022-02-05 10:34 ` [PATCH 2/2] random: fix locking for crng_init in crng_reseed() Dominik Brodowski
1 sibling, 2 replies; 6+ messages in thread
From: Dominik Brodowski @ 2022-02-05 10:34 UTC (permalink / raw)
To: Jason A . Donenfeld, Herbert Xu; +Cc: linux-kernel, linux-crypto
crng_init is protected by primary_crng->lock, so keep holding that lock
when incrementing crng_init from 0 to 1 in crng_fast_load(). The call to
pr_notice() can wait until the lock is released; this code path cannot
be reached twice, as crng_fast_load() aborts early if crng_init > 0.
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
drivers/char/random.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5d7d6e01bbc4..2df08d05e850 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -653,12 +653,13 @@ static size_t crng_fast_load(const u8 *cp, size_t len)
p[crng_init_cnt % CHACHA_KEY_SIZE] ^= *cp;
cp++; crng_init_cnt++; len--; ret++;
}
- spin_unlock_irqrestore(&primary_crng.lock, flags);
if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
invalidate_batched_entropy();
crng_init = 1;
- pr_notice("fast init done\n");
}
+ spin_unlock_irqrestore(&primary_crng.lock, flags);
+ if (crng_init == 1)
+ pr_notice("fast init done\n");
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] random: fix locking for crng_init in crng_reseed()
2022-02-05 10:34 [PATCH 0/2] random: fix write locking for crng_init Dominik Brodowski
2022-02-05 10:34 ` [PATCH 1/2] random: fix locking in crng_fast_load() Dominik Brodowski
@ 2022-02-05 10:34 ` Dominik Brodowski
1 sibling, 0 replies; 6+ messages in thread
From: Dominik Brodowski @ 2022-02-05 10:34 UTC (permalink / raw)
To: Jason A . Donenfeld, Herbert Xu; +Cc: linux-kernel, linux-crypto
crng_init is protected by primary_crng->lock. Therefore, we need
to hold this lock when increasing crng_init to 2. As we shouldn't
hold this lock for too long, only hold it for crng_finalize_init(),
and split out the parts which can be delayed to crng_late_init().
If crng_finalize_init() cannot proceed due to workqueues not yet
being available, it is called again in rand_initialize(). Then, we
do not need to call crng_late_init(): At this time, the boot
process is still so early that there are no other processes to wake
up.
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
drivers/char/random.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 2df08d05e850..c70a9abbd8cb 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -557,19 +557,24 @@ static void __init crng_initialize_primary(void)
primary_crng.init_time = jiffies - CRNG_RESEED_INTERVAL - 1;
}
-static void crng_finalize_init(void)
+static bool crng_finalize_init(void)
{
if (!system_wq) {
/* We can't call numa_crng_init until we have workqueues,
* so mark this for processing later. */
crng_need_final_init = true;
- return;
+ return false;
}
invalidate_batched_entropy();
numa_crng_init();
crng_init = 2;
crng_need_final_init = false;
+ return true;
+}
+
+static void crng_late_init(void)
+{
process_random_ready_list();
wake_up_interruptible(&crng_init_wait);
kill_fasync(&fasync, SIGIO, POLL_IN);
@@ -710,6 +715,7 @@ static int crng_slow_load(const u8 *cp, size_t len)
static void crng_reseed(struct crng_state *crng)
{
+ bool needs_late_init = false;
unsigned long flags;
int i;
union {
@@ -744,9 +750,11 @@ static void crng_reseed(struct crng_state *crng)
}
memzero_explicit(&buf, sizeof(buf));
WRITE_ONCE(crng->init_time, jiffies);
- spin_unlock_irqrestore(&crng->lock, flags);
if (crng == &primary_crng && crng_init < 2)
- crng_finalize_init();
+ needs_late_init = crng_finalize_init();
+ spin_unlock_irqrestore(&crng->lock, flags);
+ if (needs_late_init)
+ crng_late_init();
}
static void _extract_crng(struct crng_state *crng, u8 out[CHACHA_BLOCK_SIZE])
@@ -1383,8 +1391,10 @@ static void __init init_std_data(void)
int __init rand_initialize(void)
{
init_std_data();
- if (crng_need_final_init)
+ if (crng_need_final_init) {
crng_finalize_init();
+ pr_notice("crng init done\n");
+ }
crng_initialize_primary();
crng_global_init_time = jiffies;
if (ratelimit_disable) {
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] random: fix locking in crng_fast_load()
2022-02-05 10:34 ` [PATCH 1/2] random: fix locking in crng_fast_load() Dominik Brodowski
@ 2022-02-05 13:17 ` Jason A. Donenfeld
2022-02-05 13:32 ` Dominik Brodowski
2022-02-21 2:32 ` Eric Biggers
1 sibling, 1 reply; 6+ messages in thread
From: Jason A. Donenfeld @ 2022-02-05 13:17 UTC (permalink / raw)
To: Dominik Brodowski; +Cc: LKML, Linux Crypto Mailing List, Theodore Ts'o
Hi Dominik,
Does this introduce a lock nesting inversion situation?
With your patch, crng_fast_load() now does:
lock(primary_crng)
invalidate_batched_entropy()
lock(batch_lock)
unlock(batch_lock)
unlock(primary_crng)
While get_random_{u32,u64}() does:
lock(batch_lock)
extract_crng()
lock(primary_crng)
unlock(primary_crng)
unlock(batch_lock)
Is this correct? If so, we might have to defer this patch until after
<https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/commit/?id=2dfab1b1>
or something like it lands, which attempts to get rid of the batched
entropy lock.
If that analysis seems right to you, I could pull this patch into that
development branch for poking and prodding.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] random: fix locking in crng_fast_load()
2022-02-05 13:17 ` Jason A. Donenfeld
@ 2022-02-05 13:32 ` Dominik Brodowski
0 siblings, 0 replies; 6+ messages in thread
From: Dominik Brodowski @ 2022-02-05 13:32 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: LKML, Linux Crypto Mailing List, Theodore Ts'o
Hi Jason,
Am Sat, Feb 05, 2022 at 02:17:15PM +0100 schrieb Jason A. Donenfeld:
> Does this introduce a lock nesting inversion situation?
>
> With your patch, crng_fast_load() now does:
>
> lock(primary_crng)
> invalidate_batched_entropy()
> lock(batch_lock)
> unlock(batch_lock)
> unlock(primary_crng)
>
> While get_random_{u32,u64}() does:
>
> lock(batch_lock)
> extract_crng()
> lock(primary_crng)
> unlock(primary_crng)
> unlock(batch_lock)
>
> Is this correct? If so, we might have to defer this patch until after
> <https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/commit/?id=2dfab1b1>
> or something like it lands, which attempts to get rid of the batched
> entropy lock.
>
> If that analysis seems right to you, I could pull this patch into that
> development branch for poking and prodding.
Right, this makes sense -- I already "read" invalidate_batched_entropy() as
being just a call to atomic_inc().
Thanks,
Dominik
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] random: fix locking in crng_fast_load()
2022-02-05 10:34 ` [PATCH 1/2] random: fix locking in crng_fast_load() Dominik Brodowski
2022-02-05 13:17 ` Jason A. Donenfeld
@ 2022-02-21 2:32 ` Eric Biggers
1 sibling, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2022-02-21 2:32 UTC (permalink / raw)
To: Dominik Brodowski
Cc: Jason A . Donenfeld, Herbert Xu, linux-kernel, linux-crypto
On Sat, Feb 05, 2022 at 11:34:57AM +0100, Dominik Brodowski wrote:
> crng_init is protected by primary_crng->lock, so keep holding that lock
> when incrementing crng_init from 0 to 1 in crng_fast_load(). The call to
> pr_notice() can wait until the lock is released; this code path cannot
> be reached twice, as crng_fast_load() aborts early if crng_init > 0.
>
> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
> ---
> drivers/char/random.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Eric Biggers <ebiggers@google.com>
- Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-21 2:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-05 10:34 [PATCH 0/2] random: fix write locking for crng_init Dominik Brodowski
2022-02-05 10:34 ` [PATCH 1/2] random: fix locking in crng_fast_load() Dominik Brodowski
2022-02-05 13:17 ` Jason A. Donenfeld
2022-02-05 13:32 ` Dominik Brodowski
2022-02-21 2:32 ` Eric Biggers
2022-02-05 10:34 ` [PATCH 2/2] random: fix locking for crng_init in crng_reseed() Dominik Brodowski
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.