diff mbox series

ath9k: backport hw_random API support

Message ID 20220302133126.3182044-1-rsalvaterra@gmail.com
State Superseded
Delegated to: Rui Salvaterra
Headers show
Series ath9k: backport hw_random API support | expand

Commit Message

Rui Salvaterra March 2, 2022, 1:31 p.m. UTC
Backport Jason's patch [1] implementing proper hw_random API support for the
ath9k hwrng. The original code invoked add_hwgenerator_randomness to directly
feed ADC entropy into the system pool. Since add_hwgenerator_randomness blocks
until the system is low on entropy, if there is another hw_random API-based
hwrng selected in the system, both of them will race to feed the entropy pool
on wake-up. This unpredictability of the entropy source is a potential security
risk. By turning supported ath9k devices into proper hwrngs, we allow users to
choose one, if any, as they see fit.

[1] https://lore.kernel.org/all/20220216113323.53332-1-Jason@zx2c4.com/

Acked-by: Petr Štetiar <ynezz@true.cz>
Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>
---
 ...dom-API-instead-of-directly-dumping-.patch | 145 ++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100644 package/kernel/mac80211/patches/ath9k/020-v5.18-ath9k-use-hw_random-API-instead-of-directly-dumping-.patch

Comments

Rui Salvaterra March 23, 2022, 1:25 p.m. UTC | #1
Hi again,

On Wed, 2 Mar 2022 at 13:31, Rui Salvaterra <rsalvaterra@gmail.com> wrote:
>
> Backport Jason's patch [1] implementing proper hw_random API support for the
> ath9k hwrng. The original code invoked add_hwgenerator_randomness to directly
> feed ADC entropy into the system pool. Since add_hwgenerator_randomness blocks
> until the system is low on entropy, if there is another hw_random API-based
> hwrng selected in the system, both of them will race to feed the entropy pool
> on wake-up. This unpredictability of the entropy source is a potential security
> risk. By turning supported ath9k devices into proper hwrngs, we allow users to
> choose one, if any, as they see fit.
>
> [1] https://lore.kernel.org/all/20220216113323.53332-1-Jason@zx2c4.com/
>
> Acked-by: Petr Štetiar <ynezz@true.cz>
> Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>

[patch snipped]

Rats. I just noticed a build failure with the default configuration
because the kmod-ath9k package is missing the kmod-random-core
dependency when it's built with CONFIG_ATH9K_HWRNG. I hadn't noticed
it before because I compile my kernels with as many built-in modules
as possible. Will send a v2 shortly.

Cheers,
Rui
diff mbox series

Patch

diff --git a/package/kernel/mac80211/patches/ath9k/020-v5.18-ath9k-use-hw_random-API-instead-of-directly-dumping-.patch b/package/kernel/mac80211/patches/ath9k/020-v5.18-ath9k-use-hw_random-API-instead-of-directly-dumping-.patch
new file mode 100644
index 0000000000..e6fcba8a9a
--- /dev/null
+++ b/package/kernel/mac80211/patches/ath9k/020-v5.18-ath9k-use-hw_random-API-instead-of-directly-dumping-.patch
@@ -0,0 +1,145 @@ 
+From 26a3c8256d1940dbaf0449f0cc4f4c94e321e054 Mon Sep 17 00:00:00 2001
+From: "Jason A. Donenfeld" <Jason@zx2c4.com>
+Date: Wed, 16 Feb 2022 12:33:23 +0100
+Subject: [PATCH] ath9k: use hw_random API instead of directly dumping into
+ random.c
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Hardware random number generators are supposed to use the hw_random
+framework. This commit turns ath9k's kthread-based design into a proper
+hw_random driver.
+
+Cc: Toke Høiland-Jørgensen <toke@redhat.com>
+Cc: Kalle Valo <kvalo@kernel.org>
+Cc: Rui Salvaterra <rsalvaterra@gmail.com>
+Cc: Dominik Brodowski <linux@dominikbrodowski.net>
+Cc: Herbert Xu <herbert@gondor.apana.org.au>
+Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
+Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>
+---
+ drivers/net/wireless/ath/ath9k/ath9k.h |  3 +-
+ drivers/net/wireless/ath/ath9k/rng.c   | 72 +++++++++++---------------
+ 2 files changed, 33 insertions(+), 42 deletions(-)
+
+--- a/drivers/net/wireless/ath/ath9k/ath9k.h
++++ b/drivers/net/wireless/ath/ath9k/ath9k.h
+@@ -1071,8 +1071,9 @@ struct ath_softc {
+ #endif
+ 
+ #ifdef CPTCFG_ATH9K_HWRNG
++	struct hwrng rng_ops;
+ 	u32 rng_last;
+-	struct task_struct *rng_task;
++	char rng_name[sizeof("ath9k_65535")];
+ #endif
+ };
+ 
+--- a/drivers/net/wireless/ath/ath9k/rng.c
++++ b/drivers/net/wireless/ath/ath9k/rng.c
+@@ -21,11 +21,6 @@
+ #include "hw.h"
+ #include "ar9003_phy.h"
+ 
+-#define ATH9K_RNG_BUF_SIZE	320
+-#define ATH9K_RNG_ENTROPY(x)	(((x) * 8 * 10) >> 5) /* quality: 10/32 */
+-
+-static DECLARE_WAIT_QUEUE_HEAD(rng_queue);
+-
+ static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size)
+ {
+ 	int i, j;
+@@ -71,61 +66,56 @@ static u32 ath9k_rng_delay_get(u32 fail_
+ 	return delay;
+ }
+ 
+-static int ath9k_rng_kthread(void *data)
++static int ath9k_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+ {
+-	int bytes_read;
+-	struct ath_softc *sc = data;
+-	u32 *rng_buf;
+-	u32 delay, fail_stats = 0;
+-
+-	rng_buf = kmalloc_array(ATH9K_RNG_BUF_SIZE, sizeof(u32), GFP_KERNEL);
+-	if (!rng_buf)
+-		goto out;
+-
+-	while (!kthread_should_stop()) {
+-		bytes_read = ath9k_rng_data_read(sc, rng_buf,
+-						 ATH9K_RNG_BUF_SIZE);
+-		if (unlikely(!bytes_read)) {
+-			delay = ath9k_rng_delay_get(++fail_stats);
+-			wait_event_interruptible_timeout(rng_queue,
+-							 kthread_should_stop(),
+-							 msecs_to_jiffies(delay));
+-			continue;
++	struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops);
++	u32 fail_stats = 0, word;
++	int bytes_read = 0;
++
++	for (;;) {
++		if (max & ~3UL)
++			bytes_read = ath9k_rng_data_read(sc, buf, max >> 2);
++		if ((max & 3UL) && ath9k_rng_data_read(sc, &word, 1)) {
++			memcpy(buf + bytes_read, &word, max & 3UL);
++			bytes_read += max & 3UL;
++			memzero_explicit(&word, sizeof(word));
+ 		}
++		if (!wait || !max || likely(bytes_read) || fail_stats > 110)
++			break;
+ 
+-		fail_stats = 0;
+-
+-		/* sleep until entropy bits under write_wakeup_threshold */
+-		add_hwgenerator_randomness((void *)rng_buf, bytes_read,
+-					   ATH9K_RNG_ENTROPY(bytes_read));
++		msleep_interruptible(ath9k_rng_delay_get(++fail_stats));
+ 	}
+ 
+-	kfree(rng_buf);
+-out:
+-	sc->rng_task = NULL;
+-
+-	return 0;
++	if (wait && !bytes_read && max)
++		bytes_read = -EIO;
++	return bytes_read;
+ }
+ 
+ void ath9k_rng_start(struct ath_softc *sc)
+ {
++	static atomic_t serial = ATOMIC_INIT(0);
+ 	struct ath_hw *ah = sc->sc_ah;
+ 
+-	if (sc->rng_task)
++	if (sc->rng_ops.read)
+ 		return;
+ 
+ 	if (!AR_SREV_9300_20_OR_LATER(ah))
+ 		return;
+ 
+-	sc->rng_task = kthread_run(ath9k_rng_kthread, sc, "ath9k-hwrng");
+-	if (IS_ERR(sc->rng_task))
+-		sc->rng_task = NULL;
++	snprintf(sc->rng_name, sizeof(sc->rng_name), "ath9k_%u",
++		 (atomic_inc_return(&serial) - 1) & U16_MAX);
++	sc->rng_ops.name = sc->rng_name;
++	sc->rng_ops.read = ath9k_rng_read;
++	sc->rng_ops.quality = 320;
++
++	if (devm_hwrng_register(sc->dev, &sc->rng_ops))
++		sc->rng_ops.read = NULL;
+ }
+ 
+ void ath9k_rng_stop(struct ath_softc *sc)
+ {
+-	if (sc->rng_task) {
+-		kthread_stop(sc->rng_task);
+-		sc->rng_task = NULL;
++	if (sc->rng_ops.read) {
++		devm_hwrng_unregister(sc->dev, &sc->rng_ops);
++		sc->rng_ops.read = NULL;
+ 	}
+ }