[PATCH 0/4] hw_random: introduce rng_quality sysfs attribute

[PATCH 0/4] hw_random: introduce rng_quality sysfs attribute

[Dominik Brodowski]

Currently, the hw_random core exposes a module parameter "current_quality".
However, that is fundamentally broken: If it is set at boot time, it is
overwritten once the first hw rng device is loaded. If it is set at
runtime, it is without effect if the hw rng device had its quality value
set to 0 (and no default_quality was set). If a new rng is selected, it
gets overwritten.

Therefore, let's mark it as obsolete, and replace it with a new sysfs
attribute named "rng_quality". This sets the quality setting of the
hw rng device currently loaded and active, and starts/stops the hwrng
kernel thread as required.

Dominik Brodowski (4):
  hw_random: do not bother to order list of devices by quality
  hw_random: start and stop in-kernel rngd in separate function
  hw_random: use per-rng quality value instead of global setting
  hw_random: introduce rng_quality sysfs attribute

 drivers/char/hw_random/core.c | 145 +++++++++++++++++++++++++---------
 1 file changed, 107 insertions(+), 38 deletions(-)

---
If preferred, the "current_quality" module parameter can be removed
altogether.

-- 
2.35.1

[PATCH 1/4] hw_random: do not bother to order list of devices by quality

[Dominik Brodowski]

There is no real reason why this list needs to be kept ordered by
the driver-provided quality value -- a value which is set only by
a handful of hw_random devices anyway.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/char/hw_random/core.c | 36 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index f327f7493585..6f09f4e5af20 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -31,7 +31,7 @@ static struct hwrng *current_rng;
 /* the current rng has been explicitly chosen by user via sysfs */
 static int cur_rng_set_by_user;
 static struct task_struct *hwrng_fill;
-/* list of registered rngs, sorted decending by quality */
+/* list of registered rngs */
 static LIST_HEAD(rng_list);
 /* Protects rng_list and current_rng */
 static DEFINE_MUTEX(rng_mutex);
@@ -297,24 +297,28 @@ static struct miscdevice rng_miscdev = {
 
 static int enable_best_rng(void)
 {
+	struct hwrng *rng, *new_rng = NULL;
 	int ret = -ENODEV;
 
 	BUG_ON(!mutex_is_locked(&rng_mutex));
 
-	/* rng_list is sorted by quality, use the best (=first) one */
-	if (!list_empty(&rng_list)) {
-		struct hwrng *new_rng;
-
-		new_rng = list_entry(rng_list.next, struct hwrng, list);
-		ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
-		if (!ret)
-			cur_rng_set_by_user = 0;
-	} else {
+	/* no rng to use? */
+	if (list_empty(&rng_list)) {
 		drop_current_rng();
 		cur_rng_set_by_user = 0;
-		ret = 0;
+		return 0;
 	}
 
+	/* use the rng which offers the best quality */
+	list_for_each_entry(rng, &rng_list, list) {
+		if (!new_rng || rng->quality > new_rng->quality)
+			new_rng = rng;
+	}
+
+	ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
+	if (!ret)
+		cur_rng_set_by_user = 0;
+
 	return ret;
 }
 
@@ -475,7 +479,6 @@ int hwrng_register(struct hwrng *rng)
 {
 	int err = -EINVAL;
 	struct hwrng *tmp;
-	struct list_head *rng_list_ptr;
 	bool is_new_current = false;
 
 	if (!rng->name || (!rng->data_read && !rng->read))
@@ -489,18 +492,11 @@ int hwrng_register(struct hwrng *rng)
 		if (strcmp(tmp->name, rng->name) == 0)
 			goto out_unlock;
 	}
+	list_add_tail(&rng->list, &rng_list);
 
 	init_completion(&rng->cleanup_done);
 	complete(&rng->cleanup_done);
 
-	/* rng_list is sorted by decreasing quality */
-	list_for_each(rng_list_ptr, &rng_list) {
-		tmp = list_entry(rng_list_ptr, struct hwrng, list);
-		if (tmp->quality < rng->quality)
-			break;
-	}
-	list_add_tail(&rng->list, rng_list_ptr);
-
 	if (!current_rng ||
 	    (!cur_rng_set_by_user && rng->quality > current_rng->quality)) {
 		/*
-- 
2.35.1

[PATCH 2/4] hw_random: start and stop in-kernel rngd in separate function

[Dominik Brodowski]

Extract the start/stop logic for the in-kernel rngd thread to
a separate function.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/char/hw_random/core.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 6f09f4e5af20..29febf55b0d4 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -51,7 +51,7 @@ MODULE_PARM_DESC(default_quality,
 
 static void drop_current_rng(void);
 static int hwrng_init(struct hwrng *rng);
-static void start_khwrngd(void);
+static void hwrng_manage_rngd(void);
 
 static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
 			       int wait);
@@ -164,10 +164,7 @@ static int hwrng_init(struct hwrng *rng)
 	if (current_quality > 1024)
 		current_quality = 1024;
 
-	if (current_quality == 0 && hwrng_fill)
-		kthread_stop(hwrng_fill);
-	if (current_quality > 0 && !hwrng_fill)
-		start_khwrngd();
+	hwrng_manage_rngd();
 
 	return 0;
 }
@@ -466,12 +463,19 @@ static int hwrng_fillfn(void *unused)
 	return 0;
 }
 
-static void start_khwrngd(void)
+static void hwrng_manage_rngd(void)
 {
-	hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
-	if (IS_ERR(hwrng_fill)) {
-		pr_err("hwrng_fill thread creation failed\n");
-		hwrng_fill = NULL;
+	if (WARN_ON(!mutex_is_locked(&rng_mutex)))
+		return;
+
+	if (current_quality == 0 && hwrng_fill)
+		kthread_stop(hwrng_fill);
+	if (current_quality > 0 && !hwrng_fill) {
+		hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
+		if (IS_ERR(hwrng_fill)) {
+			pr_err("hwrng_fill thread creation failed\n");
+			hwrng_fill = NULL;
+		}
 	}
 }
 
-- 
2.35.1

[PATCH 3/4] hw_random: use per-rng quality value instead of global setting

[Dominik Brodowski]

The current_quality variable exposed as a module parameter is
fundamentally broken: If it is set at boot time, it is overwritten once
the first hw rng device is loaded; if it is set at runtime, it is
without effect if the hw rng device had its quality value set to 0 (and
no default_quality was set); and if a new rng is selected, it gets
overwritten. Therefore, mark it as obsolete, and replace it by the
per-rng quality setting.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/char/hw_random/core.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 29febf55b0d4..8df102b39b35 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -44,14 +44,14 @@ static unsigned short default_quality; /* = 0; default to "off" */
 
 module_param(current_quality, ushort, 0644);
 MODULE_PARM_DESC(current_quality,
-		 "current hwrng entropy estimation per 1024 bits of input");
+		 "current hwrng entropy estimation per 1024 bits of input -- obsolete");
 module_param(default_quality, ushort, 0644);
 MODULE_PARM_DESC(default_quality,
 		 "default entropy content of hwrng per 1024 bits of input");
 
 static void drop_current_rng(void);
 static int hwrng_init(struct hwrng *rng);
-static void hwrng_manage_rngd(void);
+static void hwrng_manage_rngd(struct hwrng *rng);
 
 static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
 			       int wait);
@@ -160,11 +160,13 @@ static int hwrng_init(struct hwrng *rng)
 	reinit_completion(&rng->cleanup_done);
 
 skip_init:
-	current_quality = rng->quality ? : default_quality;
-	if (current_quality > 1024)
-		current_quality = 1024;
+	if (!rng->quality)
+		rng->quality = default_quality;
+	if (rng->quality > 1024)
+		rng->quality = 1024;
+	current_quality = rng->quality; /* obsolete */
 
-	hwrng_manage_rngd();
+	hwrng_manage_rngd(rng);
 
 	return 0;
 }
@@ -429,19 +431,24 @@ static int hwrng_fillfn(void *unused)
 	long rc;
 
 	while (!kthread_should_stop()) {
+		unsigned short quality;
 		struct hwrng *rng;
 
-		if (!current_quality)
-			break;
-
 		rng = get_current_rng();
 		if (IS_ERR(rng) || !rng)
 			break;
 		mutex_lock(&reading_mutex);
 		rc = rng_get_data(rng, rng_fillbuf,
 				  rng_buffer_size(), 1);
+		if (current_quality != rng->quality)
+			rng->quality = current_quality; /* obsolete */
+		quality = rng->quality;
 		mutex_unlock(&reading_mutex);
 		put_rng(rng);
+
+		if (!quality)
+			break;
+
 		if (rc <= 0) {
 			pr_warn("hwrng: no data available\n");
 			msleep_interruptible(10000);
@@ -451,7 +458,7 @@ static int hwrng_fillfn(void *unused)
 		/* If we cannot credit at least one bit of entropy,
 		 * keep track of the remainder for the next iteration
 		 */
-		entropy = rc * current_quality * 8 + entropy_credit;
+		entropy = rc * quality * 8 + entropy_credit;
 		if ((entropy >> 10) == 0)
 			entropy_credit = entropy;
 
@@ -463,14 +470,14 @@ static int hwrng_fillfn(void *unused)
 	return 0;
 }
 
-static void hwrng_manage_rngd(void)
+static void hwrng_manage_rngd(struct hwrng *rng)
 {
 	if (WARN_ON(!mutex_is_locked(&rng_mutex)))
 		return;
 
-	if (current_quality == 0 && hwrng_fill)
+	if (rng->quality == 0 && hwrng_fill)
 		kthread_stop(hwrng_fill);
-	if (current_quality > 0 && !hwrng_fill) {
+	if (rng->quality > 0 && !hwrng_fill) {
 		hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
 		if (IS_ERR(hwrng_fill)) {
 			pr_err("hwrng_fill thread creation failed\n");
-- 
2.35.1

[PATCH 4/4] hw_random: introduce rng_quality sysfs attribute

[Dominik Brodowski]

The rng_quality sysfs attribute returns the quality setting for the
currently active hw_random device, in entropy bits per 1024 bits of
input. Storing a value between 0 and 1024 to this file updates this
estimate accordingly.

Based on the updates to the quality setting, the rngd kernel thread
may be stopped (if no hw_random device is trusted to return entropy),
may be started (if the quality setting is increased from zero), or
may use a different hw_random source (if that has higher quality
output).

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/char/hw_random/core.c | 64 ++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 8df102b39b35..71eafcc451e1 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -44,7 +44,7 @@ static unsigned short default_quality; /* = 0; default to "off" */
 
 module_param(current_quality, ushort, 0644);
 MODULE_PARM_DESC(current_quality,
-		 "current hwrng entropy estimation per 1024 bits of input -- obsolete");
+		 "current hwrng entropy estimation per 1024 bits of input -- obsolete, use rng_quality instead");
 module_param(default_quality, ushort, 0644);
 MODULE_PARM_DESC(default_quality,
 		 "default entropy content of hwrng per 1024 bits of input");
@@ -402,14 +402,76 @@ static ssize_t rng_selected_show(struct device *dev,
 	return sysfs_emit(buf, "%d\n", cur_rng_set_by_user);
 }
 
+static ssize_t rng_quality_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	ssize_t ret;
+	struct hwrng *rng;
+
+	rng = get_current_rng();
+	if (IS_ERR(rng))
+		return PTR_ERR(rng);
+
+	if (!rng) /* no need to put_rng */
+		return -ENODEV;
+
+	ret = sysfs_emit(buf, "%hu\n", rng->quality);
+	put_rng(rng);
+
+	return ret;
+}
+
+static ssize_t rng_quality_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t len)
+{
+	u16 quality;
+	int ret = -EINVAL;
+
+	if (len < 2)
+		return -EINVAL;
+
+	ret = mutex_lock_interruptible(&rng_mutex);
+	if (ret)
+		return -ERESTARTSYS;
+
+	ret = kstrtou16(buf, 0, &quality);
+	if (ret || quality > 1024) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!current_rng) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	current_rng->quality = quality;
+	current_quality = quality; /* obsolete */
+
+	/* the best available RNG may have changed */
+	ret = enable_best_rng();
+
+	/* start/stop rngd if necessary */
+	if (current_rng)
+		hwrng_manage_rngd(current_rng);
+
+out:
+	mutex_unlock(&rng_mutex);
+	return ret ? ret : len;
+}
+
 static DEVICE_ATTR_RW(rng_current);
 static DEVICE_ATTR_RO(rng_available);
 static DEVICE_ATTR_RO(rng_selected);
+static DEVICE_ATTR_RW(rng_quality);
 
 static struct attribute *rng_dev_attrs[] = {
 	&dev_attr_rng_current.attr,
 	&dev_attr_rng_available.attr,
 	&dev_attr_rng_selected.attr,
+	&dev_attr_rng_quality.attr,
 	NULL
 };
 
-- 
2.35.1

Re: [PATCH 0/4] hw_random: introduce rng_quality sysfs attribute

[Herbert Xu]

On Sun, Feb 13, 2022 at 09:46:27PM +0100, Dominik Brodowski wrote:
> Currently, the hw_random core exposes a module parameter "current_quality".
> However, that is fundamentally broken: If it is set at boot time, it is
> overwritten once the first hw rng device is loaded. If it is set at
> runtime, it is without effect if the hw rng device had its quality value
> set to 0 (and no default_quality was set). If a new rng is selected, it
> gets overwritten.
> 
> Therefore, let's mark it as obsolete, and replace it with a new sysfs
> attribute named "rng_quality". This sets the quality setting of the
> hw rng device currently loaded and active, and starts/stops the hwrng
> kernel thread as required.
> 
> Dominik Brodowski (4):
>   hw_random: do not bother to order list of devices by quality
>   hw_random: start and stop in-kernel rngd in separate function
>   hw_random: use per-rng quality value instead of global setting
>   hw_random: introduce rng_quality sysfs attribute
> 
>  drivers/char/hw_random/core.c | 145 +++++++++++++++++++++++++---------
>  1 file changed, 107 insertions(+), 38 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt