crypto: jitter - reuse allocated entropy collector

Commit Message

Stephan Mueller Oct. 7, 2023, 7:10 a.m. UTC

In case a health test error occurs during runtime, the power-up health
tests are rerun to verify that the noise source is still good and
that the reported health test error was an outlier. For performing this
power-up health test, the already existing entropy collector instance
is used instead of allocating a new one. This change has the following
implications:

* The noise that is collected as part of the newly run health tests is
  inserted into the entropy collector and thus stirs the existing
  data present in there further. Thus, the entropy collected during
  the health test is not wasted. This is also allowed by SP800-90B.

* The power-on health test is not affected by the state of the entropy
  collector, because it resets the APT / RCT state. The remainder of
  the state is unrelated to the health test as it is only applied to
  newly obtained time stamps.

This change also fixes a bug report about an allocation while in an
atomic lock (the lock is taken in jent_kcapi_random, jent_read_entropy
is called and this can call jent_entropy_init).

Reported-by: Dan Carpenter <[e-mail]>
Signed-off-by: Stephan Mueller <[e-mail]>
---
 crypto/jitterentropy-kcapi.c |  2 +-
 crypto/jitterentropy.c       | 36 ++++++++++++++++++++++++++----------
 crypto/jitterentropy.h       |  2 +-
 3 files changed, 28 insertions(+), 12 deletions(-)

Comments

Herbert Xu Oct. 13, 2023, 10:38 a.m. UTC | #1

On Sat, Oct 07, 2023 at 09:10:43AM +0200, Stephan Müller wrote:
> In case a health test error occurs during runtime, the power-up health
> tests are rerun to verify that the noise source is still good and
> that the reported health test error was an outlier. For performing this
> power-up health test, the already existing entropy collector instance
> is used instead of allocating a new one. This change has the following
> implications:
> 
> * The noise that is collected as part of the newly run health tests is
>   inserted into the entropy collector and thus stirs the existing
>   data present in there further. Thus, the entropy collected during
>   the health test is not wasted. This is also allowed by SP800-90B.
> 
> * The power-on health test is not affected by the state of the entropy
>   collector, because it resets the APT / RCT state. The remainder of
>   the state is unrelated to the health test as it is only applied to
>   newly obtained time stamps.
> 
> This change also fixes a bug report about an allocation while in an
> atomic lock (the lock is taken in jent_kcapi_random, jent_read_entropy
> is called and this can call jent_entropy_init).
> 
> Reported-by: Dan Carpenter <[e-mail]>
> Signed-off-by: Stephan Mueller <[e-mail]>
> ---
>  crypto/jitterentropy-kcapi.c |  2 +-
>  crypto/jitterentropy.c       | 36 ++++++++++++++++++++++++++----------
>  crypto/jitterentropy.h       |  2 +-
>  3 files changed, 28 insertions(+), 12 deletions(-)

Patch applied.  Thanks.

Patch

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 0c6752221451..76edbf8af0ac 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -347,7 +347,7 @@  static int __init jent_mod_init(void)
 
 	desc->tfm = tfm;
 	crypto_shash_init(desc);
-	ret = jent_entropy_init(CONFIG_CRYPTO_JITTERENTROPY_OSR, 0, desc);
+	ret = jent_entropy_init(CONFIG_CRYPTO_JITTERENTROPY_OSR, 0, desc, NULL);
 	shash_desc_zero(desc);
 	crypto_free_shash(tfm);
 	if (ret) {
diff --git a/crypto/jitterentropy.c b/crypto/jitterentropy.c
index 18bbe2b89a94..86ffdd6735cc 100644
--- a/crypto/jitterentropy.c
+++ b/crypto/jitterentropy.c
@@ -611,8 +611,7 @@  int jent_read_entropy(struct rand_data *ec, unsigned char *data,
 			 * Perform startup health tests and return permanent
 			 * error if it fails.
 			 */
-			if (jent_entropy_init(ec->osr, ec->flags,
-					      ec->hash_state))
+			if (jent_entropy_init(0, 0, NULL, ec))
 				return -3;
 
 			return -2;
@@ -686,14 +685,30 @@  void jent_entropy_collector_free(struct rand_data *entropy_collector)
 	jent_zfree(entropy_collector);
 }
 
-int jent_entropy_init(unsigned int osr, unsigned int flags, void *hash_state)
+int jent_entropy_init(unsigned int osr, unsigned int flags, void *hash_state,
+		      struct rand_data *p_ec)
 {
-	struct rand_data *ec;
-	int i, time_backwards = 0, ret = 0;
-
-	ec = jent_entropy_collector_alloc(osr, flags, hash_state);
-	if (!ec)
-		return JENT_EMEM;
+	/*
+	 * If caller provides an allocated ec, reuse it which implies that the
+	 * health test entropy data is used to further still the available
+	 * entropy pool.
+	 */
+	struct rand_data *ec = p_ec;
+	int i, time_backwards = 0, ret = 0, ec_free = 0;
+
+	if (!ec) {
+		ec = jent_entropy_collector_alloc(osr, flags, hash_state);
+		if (!ec)
+			return JENT_EMEM;
+		ec_free = 1;
+	} else {
+		/* Reset the APT */
+		jent_apt_reset(ec, 0);
+		/* Ensure that a new APT base is obtained */
+		ec->apt_base_set = 0;
+		/* Reset the RCT */
+		ec->rct_count = 0;
+	}
 
 	/* We could perform statistical tests here, but the problem is
 	 * that we only have a few loop counts to do testing. These
@@ -783,7 +798,8 @@  int jent_entropy_init(unsigned int osr, unsigned int flags, void *hash_state)
 	}
 
 out:
-	jent_entropy_collector_free(ec);
+	if (ec_free)
+		jent_entropy_collector_free(ec);
 
 	return ret;
 }
diff --git a/crypto/jitterentropy.h b/crypto/jitterentropy.h
index e31661ee00d3..aa4728675ae2 100644
--- a/crypto/jitterentropy.h
+++ b/crypto/jitterentropy.h
@@ -12,7 +12,7 @@  int jent_read_random_block(void *hash_state, char *dst, unsigned int dst_len);
 
 struct rand_data;
 extern int jent_entropy_init(unsigned int osr, unsigned int flags,
-			     void *hash_state);
+			     void *hash_state, struct rand_data *p_ec);
 extern int jent_read_entropy(struct rand_data *ec, unsigned char *data,
 			     unsigned int len);