drivers/char/hw_random/core.c | 39 ++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-)
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@gmail.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.0On Sun, Dec 21, 2025 at 09:24:48PM +0900, Lianjie Wang wrote: > > Besides, if the hwrng_unregister() call happens immediately after a > hwrng_register() before, the stopped thread may have never been running, How can this happen? Surely that would mean that the kthread_* API is broken? 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
On Fri, Jan 23, 2026 at 10:52:17AM +0800, Herbert Xu wrote: > On Sun, Dec 21, 2025 at 09:24:48PM +0900, Lianjie Wang wrote: > > > > Besides, if the hwrng_unregister() call happens immediately after a > > hwrng_register() before, the stopped thread may have never been running, > > How can this happen? Surely that would mean that the kthread_* API is > broken? OK, kthread_stop can stop the thread without ever calling threadfn. However, rather than having unregisters/registers firing off in parallel, let's just make sure that register cannot start until the unregister is actually done. The thing that's stopping us from waiting on kthread while holding the mutex is the get_current_rng() call in the kthread. We could get around this by converting the lock for get_current_rng to RCU instead. That should allow hwrng_unregister to safely wait on the kthread without dead-lock. Cheers, -- 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
© 2016 - 2026 Red Hat, Inc.