| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251221122448.246531-1-karin0.zst@gmail.com>
Date: Sun, 21 Dec 2025 21:24:48 +0900
From: Lianjie Wang <karin0.zst@...il.com>
To: Olivia Mackall <olivia@...enic.com>
Cc: Herbert Xu <herbert@...dor.apana.org.au>,
David Laight <david.laight.linux@...il.com>,
Jonathan McDowell <noodles@...a.com>,
linux-crypto@...r.kernel.org,
linux-kernel@...r.kernel.org,
Lianjie Wang <karin0.zst@...il.com>
Subject: [PATCH] hwrng: core - fix racing condition when stopping hwrng_fill
Previously, hwrng_fill is not cleared until the hwrng_fillfn() thread
exits. Since hwrng_unregister() reads hwrng_fill outside the rng_mutex
lock, a concurrent hwrng_unregister() may call kthread_stop() again on
the same task.
Besides, if the hwrng_unregister() call happens immediately after a
hwrng_register() before, the stopped thread may have never been running,
and thus hwrng_fill remains dirty even after the hwrng_unregister() call
returns. In this case, further calls to hwrng_register() may not start
new threads, and hwrng_unregister() will also call kthread_stop() on the
same task, causing use-after-free and sometimes lockups:
refcount_t: addition on 0; use-after-free.
WARNING: ... at lib/refcount.c:25 refcount_warn_saturate+0xec/0x1c0
Call Trace:
<TASK>
kthread_stop+0x181/0x360
hwrng_unregister+0x288/0x380
virtrng_remove+0xe3/0x200
WARNING: ... at kernel/fork.c:735 __put_task_struct+0x287/0x3d0
Call Trace:
<IRQ>
? __pfx___put_task_struct_rcu_cb+0x10/0x10
rcu_core+0xa30/0x1ab0
? __pfx_rcu_core+0x10/0x10
? hrtimer_interrupt+0x527/0x710
handle_softirqs+0x14b/0x470
This patch fixes this by protecting the global hwrng_fill inside the
rng_mutex lock. hwrng_unregister() calls kthread_stop() on the copied
pointer after releasing the lock, ensuring each hwrng_fillfn() thread is
stopped only once, and hwrng_fillfn() itself only clears hwrng_fill on
the error path.
In this case, since hwrng_fill is cleared before the thread exits,
hwrng_register() may start another thread while one is being stopped, so
rng_fillbuf has to be moved to the private stack of hwrng_fillfn() to
avoid races.
Fixes: be4000bc4644 ("hwrng: create filler thread")
Signed-off-by: Lianjie Wang <karin0.zst@...il.com>
---
drivers/char/hw_random/core.c | 39 ++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 96d7fe41b373..69ed503b50bb 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -36,12 +36,12 @@ static int cur_rng_set_by_user;
static struct task_struct *hwrng_fill;
/* list of registered rngs */
static LIST_HEAD(rng_list);
-/* Protects rng_list and current_rng */
+/* Protects rng_list, current_rng and hwrng_fill */
static DEFINE_MUTEX(rng_mutex);
-/* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */
+/* Protects rng read functions, data_avail and rng_buffer */
static DEFINE_MUTEX(reading_mutex);
static int data_avail;
-static u8 *rng_buffer, *rng_fillbuf;
+static u8 *rng_buffer;
static unsigned short current_quality;
static unsigned short default_quality = 1024; /* default to maximum */
@@ -484,16 +484,24 @@ static int hwrng_fillfn(void *unused)
size_t entropy, entropy_credit = 0; /* in 1/1024 of a bit */
long rc;
+ /* Use a private buffer to avoid races with dying threads */
+ u8 buffer[RNG_BUFFER_SIZE];
+
while (!kthread_should_stop()) {
unsigned short quality;
struct hwrng *rng;
rng = get_current_rng();
- if (IS_ERR(rng) || !rng)
+ if (IS_ERR(rng) || !rng) {
+ /* Clear hwrng_fill on the error path */
+ mutex_lock(&rng_mutex);
+ if (hwrng_fill == current)
+ hwrng_fill = NULL;
+ mutex_unlock(&rng_mutex);
break;
+ }
mutex_lock(&reading_mutex);
- rc = rng_get_data(rng, rng_fillbuf,
- rng_buffer_size(), 1);
+ rc = rng_get_data(rng, buffer, sizeof(buffer), 1);
if (current_quality != rng->quality)
rng->quality = current_quality; /* obsolete */
quality = rng->quality;
@@ -515,10 +523,9 @@ static int hwrng_fillfn(void *unused)
entropy_credit = entropy;
/* Outside lock, sure, but y'know: randomness. */
- add_hwgenerator_randomness((void *)rng_fillbuf, rc,
+ add_hwgenerator_randomness((void *)buffer, rc,
entropy >> 10, true);
}
- hwrng_fill = NULL;
return 0;
}
@@ -570,6 +577,7 @@ EXPORT_SYMBOL_GPL(hwrng_register);
void hwrng_unregister(struct hwrng *rng)
{
struct hwrng *new_rng;
+ struct task_struct *to_stop;
int err;
mutex_lock(&rng_mutex);
@@ -585,10 +593,11 @@ void hwrng_unregister(struct hwrng *rng)
}
new_rng = get_current_rng_nolock();
- if (list_empty(&rng_list)) {
+ if (list_empty(&rng_list) && hwrng_fill) {
+ to_stop = hwrng_fill;
+ hwrng_fill = NULL;
mutex_unlock(&rng_mutex);
- if (hwrng_fill)
- kthread_stop(hwrng_fill);
+ kthread_stop(to_stop);
} else
mutex_unlock(&rng_mutex);
@@ -664,15 +673,8 @@ static int __init hwrng_modinit(void)
if (!rng_buffer)
return -ENOMEM;
- rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL);
- if (!rng_fillbuf) {
- kfree(rng_buffer);
- return -ENOMEM;
- }
-
ret = misc_register(&rng_miscdev);
if (ret) {
- kfree(rng_fillbuf);
kfree(rng_buffer);
}
@@ -684,7 +686,6 @@ static void __exit hwrng_modexit(void)
mutex_lock(&rng_mutex);
BUG_ON(current_rng);
kfree(rng_buffer);
- kfree(rng_fillbuf);
mutex_unlock(&rng_mutex);
misc_deregister(&rng_miscdev);
--
2.52.0
Powered by blists - more mailing lists