Module Name: src
Committed By: riastradh
Date: Sun Mar 20 14:30:57 UTC 2022
Modified Files:
src/sys/kern: kern_entropy.c
Log Message:
entropy(9): Fix premature optimization deadlock in entropy_request.
- For synchronous queries from /dev/random, which are waiting for
entropy to be ready, wait for concurrent access -- e.g., concurrent
rnd_detach_source -- to finish, and make sure to request entropy
from all sources (unless we're interrupted by a signal).
- When invoked through softint context (e.g., cprng_fast_intr ->
cprng_strong -> entropy_extract), don't wait, because we're
forbidden from waiting anyway.
- For entropy_bootrequest, wait but don't bother failing on signal
because this only happens in kthread context, not in userland
process context, so there can't be signals.
Nix rnd_trylock_sources; use the same entropy_extract flags
(ENTROPY_WAIT, ENTROPY_SIG) for rnd_lock_sources.
To generate a diff of this commit:
cvs rdiff -u -r1.48 -r1.49 src/sys/kern/kern_entropy.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_entropy.c
diff -u src/sys/kern/kern_entropy.c:1.48 src/sys/kern/kern_entropy.c:1.49
--- src/sys/kern/kern_entropy.c:1.48 Sun Mar 20 14:05:41 2022
+++ src/sys/kern/kern_entropy.c Sun Mar 20 14:30:56 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_entropy.c,v 1.48 2022/03/20 14:05:41 riastradh Exp $ */
+/* $NetBSD: kern_entropy.c,v 1.49 2022/03/20 14:30:56 riastradh Exp $ */
/*-
* Copyright (c) 2019 The NetBSD Foundation, Inc.
@@ -75,7 +75,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_entropy.c,v 1.48 2022/03/20 14:05:41 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_entropy.c,v 1.49 2022/03/20 14:30:56 riastradh Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -260,7 +260,7 @@ static int sysctl_entropy_consolidate(SY
static int sysctl_entropy_gather(SYSCTLFN_ARGS);
static void filt_entropy_read_detach(struct knote *);
static int filt_entropy_read_event(struct knote *, long);
-static void entropy_request(size_t);
+static int entropy_request(size_t, int);
static void rnd_add_data_1(struct krndsource *, const void *, uint32_t,
uint32_t, uint32_t);
static unsigned rndsource_entropybits(struct krndsource *);
@@ -648,6 +648,7 @@ entropy_seed(rndsave_t *seed)
void
entropy_bootrequest(void)
{
+ int error;
KASSERT(E->stage >= ENTROPY_WARM);
@@ -656,7 +657,8 @@ entropy_bootrequest(void)
* This is harmless overkill if the bootloader provided a seed.
*/
mutex_enter(&E->lock);
- entropy_request(ENTROPY_CAPACITY);
+ error = entropy_request(ENTROPY_CAPACITY, ENTROPY_WAIT);
+ KASSERT(error == 0);
mutex_exit(&E->lock);
}
@@ -1300,7 +1302,8 @@ sysctl_entropy_gather(SYSCTLFN_ARGS)
return error;
if (arg) {
mutex_enter(&E->lock);
- entropy_request(ENTROPY_CAPACITY);
+ error = entropy_request(ENTROPY_CAPACITY,
+ ENTROPY_WAIT|ENTROPY_SIG);
mutex_exit(&E->lock);
}
@@ -1353,7 +1356,9 @@ entropy_extract(void *buf, size_t len, i
error = 0;
while (E->needed) {
/* Ask for more, synchronously if possible. */
- entropy_request(len);
+ error = entropy_request(len, flags);
+ if (error)
+ break;
/* If we got enough, we're done. */
if (E->needed == 0) {
@@ -1677,24 +1682,33 @@ rnd_detach_source(struct krndsource *rs)
}
/*
- * rnd_lock_sources()
+ * rnd_lock_sources(flags)
+ *
+ * Lock the list of entropy sources. Caller must hold the global
+ * entropy lock. If successful, no rndsource will go away until
+ * rnd_unlock_sources even while the caller releases the global
+ * entropy lock.
*
- * Prevent changes to the list of rndsources while we iterate it.
- * Interruptible. Caller must hold the global entropy lock. If
- * successful, no rndsource will go away until rnd_unlock_sources
- * even while the caller releases the global entropy lock.
+ * If flags & ENTROPY_WAIT, wait for concurrent access to finish.
+ * If flags & ENTROPY_SIG, allow interruption by signal.
*/
-static int
-rnd_lock_sources(void)
+static int __attribute__((warn_unused_result))
+rnd_lock_sources(int flags)
{
int error;
KASSERT(mutex_owned(&E->lock));
while (E->sourcelock) {
- error = cv_wait_sig(&E->sourcelock_cv, &E->lock);
- if (error)
- return error;
+ if (!ISSET(flags, ENTROPY_WAIT))
+ return EWOULDBLOCK;
+ if (ISSET(flags, ENTROPY_SIG)) {
+ error = cv_wait_sig(&E->sourcelock_cv, &E->lock);
+ if (error)
+ return error;
+ } else {
+ cv_wait(&E->sourcelock_cv, &E->lock);
+ }
}
E->sourcelock = curlwp;
@@ -1702,30 +1716,10 @@ rnd_lock_sources(void)
}
/*
- * rnd_trylock_sources()
- *
- * Try to lock the list of sources, but if it's already locked,
- * fail. Caller must hold the global entropy lock. If
- * successful, no rndsource will go away until rnd_unlock_sources
- * even while the caller releases the global entropy lock.
- */
-static bool
-rnd_trylock_sources(void)
-{
-
- KASSERT(E->stage == ENTROPY_COLD || mutex_owned(&E->lock));
-
- if (E->sourcelock)
- return false;
- E->sourcelock = curlwp;
- return true;
-}
-
-/*
* rnd_unlock_sources()
*
- * Unlock the list of sources after rnd_lock_sources or
- * rnd_trylock_sources. Caller must hold the global entropy lock.
+ * Unlock the list of sources after rnd_lock_sources. Caller must
+ * hold the global entropy lock.
*/
static void
rnd_unlock_sources(void)
@@ -1754,26 +1748,33 @@ rnd_sources_locked(void)
}
/*
- * entropy_request(nbytes)
+ * entropy_request(nbytes, flags)
*
* Request nbytes bytes of entropy from all sources in the system.
* OK if we overdo it. Caller must hold the global entropy lock;
* will release and re-acquire it.
+ *
+ * If flags & ENTROPY_WAIT, wait for concurrent access to finish.
+ * If flags & ENTROPY_SIG, allow interruption by signal.
*/
-static void
-entropy_request(size_t nbytes)
+static int
+entropy_request(size_t nbytes, int flags)
{
struct krndsource *rs;
+ int error;
KASSERT(E->stage == ENTROPY_COLD || mutex_owned(&E->lock));
+ if (flags & ENTROPY_WAIT)
+ ASSERT_SLEEPABLE();
/*
- * If there is a request in progress, let it proceed.
- * Otherwise, note that a request is in progress to avoid
- * reentry and to block rnd_detach_source until we're done.
+ * Lock the list of entropy sources to block rnd_detach_source
+ * until we're done, and to serialize calls to the entropy
+ * callbacks as guaranteed to drivers.
*/
- if (!rnd_trylock_sources())
- return;
+ error = rnd_lock_sources(flags);
+ if (error)
+ return error;
entropy_request_evcnt.ev_count++;
/* Clamp to the maximum reasonable request. */
@@ -1800,8 +1801,9 @@ entropy_request(size_t nbytes)
mutex_enter(&E->lock);
}
- /* Notify rnd_detach_source that the request is done. */
+ /* Request done; unlock the list of entropy sources. */
rnd_unlock_sources();
+ return 0;
}
/*
@@ -2208,7 +2210,7 @@ entropy_ioctl(unsigned long cmd, void *d
* as requested, and report how many we copied out.
*/
mutex_enter(&E->lock);
- error = rnd_lock_sources();
+ error = rnd_lock_sources(ENTROPY_WAIT|ENTROPY_SIG);
if (error) {
mutex_exit(&E->lock);
return error;
@@ -2244,7 +2246,7 @@ entropy_ioctl(unsigned long cmd, void *d
* as requested, and report how many we copied out.
*/
mutex_enter(&E->lock);
- error = rnd_lock_sources();
+ error = rnd_lock_sources(ENTROPY_WAIT|ENTROPY_SIG);
if (error) {
mutex_exit(&E->lock);
return error;
@@ -2276,7 +2278,7 @@ entropy_ioctl(unsigned long cmd, void *d
* out; if not found, fail with ENOENT.
*/
mutex_enter(&E->lock);
- error = rnd_lock_sources();
+ error = rnd_lock_sources(ENTROPY_WAIT|ENTROPY_SIG);
if (error) {
mutex_exit(&E->lock);
return error;
@@ -2307,7 +2309,7 @@ entropy_ioctl(unsigned long cmd, void *d
* out; if not found, fail with ENOENT.
*/
mutex_enter(&E->lock);
- error = rnd_lock_sources();
+ error = rnd_lock_sources(ENTROPY_WAIT|ENTROPY_SIG);
if (error) {
mutex_exit(&E->lock);
return error;
@@ -2381,10 +2383,16 @@ entropy_ioctl(unsigned long cmd, void *d
* flags, request new samples from everyone -- either
* to make up for what we just lost, or to get new
* samples from what we just added.
+ *
+ * Failing on signal, while waiting for another process
+ * to finish requesting entropy, is OK here even though
+ * we have committed side effects, because this ioctl
+ * command is idempotent, so repeating it is safe.
*/
if (request) {
mutex_enter(&E->lock);
- entropy_request(ENTROPY_CAPACITY);
+ error = entropy_request(ENTROPY_CAPACITY,
+ ENTROPY_WAIT|ENTROPY_SIG);
mutex_exit(&E->lock);
}
break;
