From nobody Sat Feb 7 12:40:26 2026 Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com [209.85.215.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 189A3311C21 for ; Thu, 29 Jan 2026 21:50:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.196 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769723451; cv=none; b=qCK0KelXoMFfi4xcqTkyNOvmReRXKCv2+JGzzb4zNBrqGDtqiZFqmDfNUMktrREs2YY2O7604GqGi9ALkLDEjez65/pMLuI+37r68D1/5EaImWVZfwJ/3Pu+RHbNdrnu8BzIIB8fAUKo9fptd+sGHUrpoeUD+ACX9mr9DmNGdgo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769723451; c=relaxed/simple; bh=ZmYulvijVpVN8u4fFLD119J4Lh4Pr/UOgKupgfDcl1k=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=QyTZrpZYB4Y2eICsUoa4nOWXu/Z0MDENzRuWgSF3LpJ5im0huFXrcoj3dxtbViuCrOWgtuaVnSNud5STzTIneA7mY7E5KKCxHQfKEhF3rNVxBiHFzeMc/qVzAH8Twu5FQA5J81oNlcPtXz8NXicVA0UZ2cFPSJbiskklyCW/Bdk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=XqasEmzc; arc=none smtp.client-ip=209.85.215.196 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="XqasEmzc" Received: by mail-pg1-f196.google.com with SMTP id 41be03b00d2f7-b4755f37c3eso842125a12.3 for ; Thu, 29 Jan 2026 13:50:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1769723446; x=1770328246; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=dVZ2yku2Ko/8tZ8Irc1VgCxoovL8Brno7qmr0cH0kl0=; b=XqasEmzc0XBj7oIaqQvePf5yZ7xLuRH+EjPs5FrRmwc7d/GqXvDP69ufAbd5JlhIWM wP067PZlREfSmgp2MP0NVN5laDipmYXHnvZy20hltCx4xucSPc4MI3r78Tp3iOhFlRMA 5s+sc2OENp+ZXh5HIOSQJCNlzQxi1XAQYeZjGWzZgp6MF30zATYQO1cieKrxbKuDhl9H eHw70F7vuUgWDqrmdtdxxhiWMOmFKs9MCIdmHYvsc4bzAODtq1Y6EuzkKWgOZLe9vAd6 M3xl2ltWesv0sTS07G1p/NVyu2tQWyDdB3J3fpjJCj8LvOVSitQchyywu0yIo1/D7Lf5 juTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1769723446; x=1770328246; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=dVZ2yku2Ko/8tZ8Irc1VgCxoovL8Brno7qmr0cH0kl0=; b=kld2w3sXe4wqpZ7o2jhbVBScfOcLbaPFjGnK3R9vV7TqtggWUnXV3/4dkTL8XmDLN/ 9OGgWSyFXHK1qf56FzT4apGHyb/H7XmR6L2P6drJbbxakzj8L0ozgdVhfNBQhdwaNV9s u+QMPC9TpdqgAYowF84SSi1VXySJ04U5hctVYIPrl0JE2n+z74UNdK+WZkYybBIWwHIF I/mw2oL0nKznFYD5HSMvab7mj/XJrBwkFEdXtSYPqa/NHUkxzIWBs22Uv1sKJjUIKFvc 0fLzZU+xuC/ES62fp2zwceGJBDnht/hbLELfFHZidXni878W7wrMOSyu7CKhmTuovj+d Nwug== X-Forwarded-Encrypted: i=1; AJvYcCVYQntT6pj+CxcXYJyovGCxt5z6lVYroVaM9BnNLXaNj7u5FEAMoYGwKWTr6nNBedZfKPE3NCFrRMhTZZs=@vger.kernel.org X-Gm-Message-State: AOJu0Yy6b04Rq+Sit9SMHamZI9I1GxD1WWfjdowzt1kSvMeOfWac1lEc JCdi5reG4xqb5omXZIPIZRRMdb7BKLd5kxnXim5bKCQO3na6GNwfsIb/ X-Gm-Gg: AZuq6aLBJHS4eHrB7J+K5C1CmVfOOEonw0+l5qE8PXuBz3liKoN0oVPOu4sEzqDHaej xCuyP4+7PSd24ooFDktmJXGpsZY5nCdQw4WvOYeHNGW/6KeW0MTE4+Vj90XeKNh+aCTlgiDLHPC w1ORPyb+O49am2Vwc76+3hA31r7SHEMmU/It/Mqwz6TN51REBzGG5g0yse/IaW/RUOzFtOEFqqm iVbolIVDxQjb+wmTY3D4W6oqlJeLH9obWxf63W/XY9HP4Ek0KepF/fHAlAKgp6KBZn/7SEyUUBj rq6VAWM+PXURLwGaI8kStIZ+flaUbpvUULiR+DhXO/6TkIU1O6iAeX3u96A9MV25UlISnjB4cFS sjaQeLuUxQUMTq9tJCBRC8K49SmjnmsrxzIrdLl1IMe7NNwlSZjFIx4/IEcRu7vLvYXI68Ykj20 /ebvYSgPcrIpstBYxW3uP2t+EECVxtrJ8DUFSM4HwQjEErkWnPDG8qxjuU4tFfgAjql0ZqIXt6V jz6cx0n3t0K81GKSWYP X-Received: by 2002:a05:6a21:32a3:b0:364:37d:cc63 with SMTP id adf61e73a8af0-392e0148311mr403708637.56.1769723446248; Thu, 29 Jan 2026 13:50:46 -0800 (PST) Received: from kator.shina-lab.internal ([133.11.33.33]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-c642afc045dsm5881498a12.35.2026.01.29.13.50.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Jan 2026 13:50:45 -0800 (PST) From: Lianjie Wang To: Herbert Xu Cc: Olivia Mackall , David Laight , Jonathan McDowell , linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, Lianjie Wang Subject: [PATCH v4] hwrng: core - use RCU and work_struct to fix race condition Date: Fri, 30 Jan 2026 06:50:16 +0900 Message-ID: <20260129215016.3042874-1-karin0.zst@gmail.com> X-Mailer: git-send-email 2.52.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Currently, 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. Additionally, if hwrng_unregister() is called immediately after hwrng_register(), the stopped thread may have never been executed. Thus, hwrng_fill remains dirty even after hwrng_unregister() returns. In this case, subsequent calls to hwrng_register() will fail to start new threads, and hwrng_unregister() will call kthread_stop() on the same freed task. In both cases, a use-after-free occurs: refcount_t: addition on 0; use-after-free. WARNING: ... at lib/refcount.c:25 refcount_warn_saturate+0xec/0x1c0 Call Trace: kthread_stop+0x181/0x360 hwrng_unregister+0x288/0x380 virtrng_remove+0xe3/0x200 This patch fixes the race by protecting the global hwrng_fill pointer inside the rng_mutex lock, so that hwrng_fillfn() thread is stopped only once, and calls to kthread_run() and kthread_stop() are serialized with the lock held. To avoid deadlock in hwrng_fillfn() while being stopped with the lock held, we convert current_rng to RCU, so that get_current_rng() can read current_rng without holding the lock. To remove the lock from put_rng(), we also delay the actual cleanup into a work_struct. Since get_current_rng() no longer returns ERR_PTR values, the IS_ERR() checks are removed from its callers. With hwrng_fill protected by the rng_mutex lock, hwrng_fillfn() can no longer clear hwrng_fill itself. Therefore, if hwrng_fillfn() returns directly after current_rng is dropped, kthread_stop() would be called on a freed task_struct later. To fix this, hwrng_fillfn() calls schedule() now to keep the task alive until being stopped. The kthread_stop() call is also moved from hwrng_unregister() to drop_current_rng(), ensuring kthread_stop() is called on all possible paths where current_rng becomes NULL, so that the thread would not wait forever. Fixes: be4000bc4644 ("hwrng: create filler thread") Suggested-by: Herbert Xu Signed-off-by: Lianjie Wang --- v4: - Include linux/workqueue_types.h in hw_random.h, rather than linux/workqueue.h. - Include linux/workqueue.h in core.c. v3: https://lore.kernel.org/linux-crypto/20260128221052.2141154-1-karin0.zs= t@gmail.com/ - Add work_struct to delay the cleanup and protect it with rng_mutex again to avoid races with hwrng_init(). - Change the waiting loop in hwrng_fillfn() to match the pattern in fs/ext4/mmp.c, and add comments for clarity. - Change kref_get_unless_zero() back to a plain kref_get() in get_current_rng(). - Move the NULL check back to put_rng(). v2: https://lore.kernel.org/linux-crypto/20260124195555.851117-1-karin0.zst= @gmail.com/ - Convert the lock for get_current_rng() to RCU to break the deadlock, as suggested by Herbert Xu. - Remove rng_mutex from put_rng() and move NULL check to rng_current_show(= ). - Move kthread_stop() to drop_current_rng() inside the lock to join the ta= sk on all paths, avoiding modifying hwrng_fill inside hwrng_fillfn(). - Revert changes to rng_fillbuf. v1: https://lore.kernel.org/linux-crypto/20251221122448.246531-1-karin0.zst= @gmail.com/ drivers/char/hw_random/core.c | 168 +++++++++++++++++++++------------- include/linux/hw_random.h | 2 + 2 files changed, 107 insertions(+), 63 deletions(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 96d7fe41b373..aba92d777f72 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -20,23 +20,25 @@ #include #include #include +#include #include #include #include #include #include +#include =20 #define RNG_MODULE_NAME "hw_random" =20 #define RNG_BUFFER_SIZE (SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES) =20 -static struct hwrng *current_rng; +static struct hwrng __rcu *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 */ static LIST_HEAD(rng_list); -/* Protects rng_list and current_rng */ +/* Protects rng_list, hwrng_fill and updating on current_rng */ static DEFINE_MUTEX(rng_mutex); /* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */ static DEFINE_MUTEX(reading_mutex); @@ -64,18 +66,39 @@ static size_t rng_buffer_size(void) return RNG_BUFFER_SIZE; } =20 -static inline void cleanup_rng(struct kref *kref) +static void cleanup_rng_work(struct work_struct *work) { - struct hwrng *rng =3D container_of(kref, struct hwrng, ref); + struct hwrng *rng =3D container_of(work, struct hwrng, cleanup_work); + + /* + * Hold rng_mutex here so we serialize in case they set_current_rng + * on rng again immediately. + */ + mutex_lock(&rng_mutex); + + /* Skip if rng has been reinitialized. */ + if (kref_read(&rng->ref)) { + mutex_unlock(&rng_mutex); + return; + } =20 if (rng->cleanup) rng->cleanup(rng); =20 complete(&rng->cleanup_done); + mutex_unlock(&rng_mutex); +} + +static inline void cleanup_rng(struct kref *kref) +{ + struct hwrng *rng =3D container_of(kref, struct hwrng, ref); + + schedule_work(&rng->cleanup_work); } =20 static int set_current_rng(struct hwrng *rng) { + struct hwrng *old_rng; int err; =20 BUG_ON(!mutex_is_locked(&rng_mutex)); @@ -84,8 +107,14 @@ static int set_current_rng(struct hwrng *rng) if (err) return err; =20 - drop_current_rng(); - current_rng =3D rng; + old_rng =3D rcu_dereference_protected(current_rng, + lockdep_is_held(&rng_mutex)); + rcu_assign_pointer(current_rng, rng); + + if (old_rng) { + synchronize_rcu(); + kref_put(&old_rng->ref, cleanup_rng); + } =20 /* if necessary, start hwrng thread */ if (!hwrng_fill) { @@ -101,47 +130,56 @@ static int set_current_rng(struct hwrng *rng) =20 static void drop_current_rng(void) { - BUG_ON(!mutex_is_locked(&rng_mutex)); - if (!current_rng) + struct hwrng *rng; + + rng =3D rcu_dereference_protected(current_rng, + lockdep_is_held(&rng_mutex)); + if (!rng) return; =20 + RCU_INIT_POINTER(current_rng, NULL); + synchronize_rcu(); + + if (hwrng_fill) { + kthread_stop(hwrng_fill); + hwrng_fill =3D NULL; + } + /* decrease last reference for triggering the cleanup */ - kref_put(¤t_rng->ref, cleanup_rng); - current_rng =3D NULL; + kref_put(&rng->ref, cleanup_rng); } =20 -/* Returns ERR_PTR(), NULL or refcounted hwrng */ +/* Returns NULL or refcounted hwrng */ static struct hwrng *get_current_rng_nolock(void) { - if (current_rng) - kref_get(¤t_rng->ref); + struct hwrng *rng; + + rng =3D rcu_dereference_protected(current_rng, + lockdep_is_held(&rng_mutex)); + if (rng) + kref_get(&rng->ref); =20 - return current_rng; + return rng; } =20 static struct hwrng *get_current_rng(void) { struct hwrng *rng; =20 - if (mutex_lock_interruptible(&rng_mutex)) - return ERR_PTR(-ERESTARTSYS); + rcu_read_lock(); + rng =3D rcu_dereference(current_rng); + if (rng) + kref_get(&rng->ref); =20 - rng =3D get_current_rng_nolock(); + rcu_read_unlock(); =20 - mutex_unlock(&rng_mutex); return rng; } =20 static void put_rng(struct hwrng *rng) { - /* - * Hold rng_mutex here so we serialize in case they set_current_rng - * on rng again immediately. - */ - mutex_lock(&rng_mutex); if (rng) kref_put(&rng->ref, cleanup_rng); - mutex_unlock(&rng_mutex); } =20 static int hwrng_init(struct hwrng *rng) @@ -213,10 +251,6 @@ static ssize_t rng_dev_read(struct file *filp, char __= user *buf, =20 while (size) { rng =3D get_current_rng(); - if (IS_ERR(rng)) { - err =3D PTR_ERR(rng); - goto out; - } if (!rng) { err =3D -ENODEV; goto out; @@ -303,7 +337,7 @@ static struct miscdevice rng_miscdev =3D { =20 static int enable_best_rng(void) { - struct hwrng *rng, *new_rng =3D NULL; + struct hwrng *rng, *cur_rng, *new_rng =3D NULL; int ret =3D -ENODEV; =20 BUG_ON(!mutex_is_locked(&rng_mutex)); @@ -321,7 +355,9 @@ static int enable_best_rng(void) new_rng =3D rng; } =20 - ret =3D ((new_rng =3D=3D current_rng) ? 0 : set_current_rng(new_rng)); + cur_rng =3D rcu_dereference_protected(current_rng, + lockdep_is_held(&rng_mutex)); + ret =3D ((new_rng =3D=3D cur_rng) ? 0 : set_current_rng(new_rng)); if (!ret) cur_rng_set_by_user =3D 0; =20 @@ -371,8 +407,6 @@ static ssize_t rng_current_show(struct device *dev, struct hwrng *rng; =20 rng =3D get_current_rng(); - if (IS_ERR(rng)) - return PTR_ERR(rng); =20 ret =3D sysfs_emit(buf, "%s\n", rng ? rng->name : "none"); put_rng(rng); @@ -416,8 +450,6 @@ static ssize_t rng_quality_show(struct device *dev, struct hwrng *rng; =20 rng =3D get_current_rng(); - if (IS_ERR(rng)) - return PTR_ERR(rng); =20 if (!rng) /* no need to put_rng */ return -ENODEV; @@ -432,6 +464,7 @@ static ssize_t rng_quality_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t len) { + struct hwrng *rng; u16 quality; int ret =3D -EINVAL; =20 @@ -448,12 +481,13 @@ static ssize_t rng_quality_store(struct device *dev, goto out; } =20 - if (!current_rng) { + rng =3D rcu_dereference_protected(current_rng, lockdep_is_held(&rng_mutex= )); + if (!rng) { ret =3D -ENODEV; goto out; } =20 - current_rng->quality =3D quality; + rng->quality =3D quality; current_quality =3D quality; /* obsolete */ =20 /* the best available RNG may have changed */ @@ -489,8 +523,20 @@ static int hwrng_fillfn(void *unused) struct hwrng *rng; =20 rng =3D get_current_rng(); - if (IS_ERR(rng) || !rng) + if (!rng) { + /* + * Keep the task_struct alive until kthread_stop() + * is called to avoid UAF in drop_current_rng(). + */ + while (!kthread_should_stop()) { + set_current_state(TASK_INTERRUPTIBLE); + if (!kthread_should_stop()) + schedule(); + } + set_current_state(TASK_RUNNING); break; + } + mutex_lock(&reading_mutex); rc =3D rng_get_data(rng, rng_fillbuf, rng_buffer_size(), 1); @@ -518,14 +564,13 @@ static int hwrng_fillfn(void *unused) add_hwgenerator_randomness((void *)rng_fillbuf, rc, entropy >> 10, true); } - hwrng_fill =3D NULL; return 0; } =20 int hwrng_register(struct hwrng *rng) { int err =3D -EINVAL; - struct hwrng *tmp; + struct hwrng *cur_rng, *tmp; =20 if (!rng->name || (!rng->data_read && !rng->read)) goto out; @@ -540,6 +585,7 @@ int hwrng_register(struct hwrng *rng) } list_add_tail(&rng->list, &rng_list); =20 + INIT_WORK(&rng->cleanup_work, cleanup_rng_work); init_completion(&rng->cleanup_done); complete(&rng->cleanup_done); init_completion(&rng->dying); @@ -547,16 +593,19 @@ int hwrng_register(struct hwrng *rng) /* Adjust quality field to always have a proper value */ rng->quality =3D min3(default_quality, 1024, rng->quality ?: 1024); =20 - if (!cur_rng_set_by_user && - (!current_rng || rng->quality > current_rng->quality)) { - /* - * Set new rng as current as the new rng source - * provides better entropy quality and was not - * chosen by userspace. - */ - err =3D set_current_rng(rng); - if (err) - goto out_unlock; + if (!cur_rng_set_by_user) { + cur_rng =3D rcu_dereference_protected(current_rng, + lockdep_is_held(&rng_mutex)); + if (!cur_rng || rng->quality > cur_rng->quality) { + /* + * Set new rng as current as the new rng source + * provides better entropy quality and was not + * chosen by userspace. + */ + err =3D set_current_rng(rng); + if (err) + goto out_unlock; + } } mutex_unlock(&rng_mutex); return 0; @@ -569,14 +618,17 @@ EXPORT_SYMBOL_GPL(hwrng_register); =20 void hwrng_unregister(struct hwrng *rng) { - struct hwrng *new_rng; + struct hwrng *cur_rng; int err; =20 mutex_lock(&rng_mutex); =20 list_del(&rng->list); complete_all(&rng->dying); - if (current_rng =3D=3D rng) { + + cur_rng =3D rcu_dereference_protected(current_rng, + lockdep_is_held(&rng_mutex)); + if (cur_rng =3D=3D rng) { err =3D enable_best_rng(); if (err) { drop_current_rng(); @@ -584,17 +636,7 @@ void hwrng_unregister(struct hwrng *rng) } } =20 - new_rng =3D get_current_rng_nolock(); - if (list_empty(&rng_list)) { - mutex_unlock(&rng_mutex); - if (hwrng_fill) - kthread_stop(hwrng_fill); - } else - mutex_unlock(&rng_mutex); - - if (new_rng) - put_rng(new_rng); - + mutex_unlock(&rng_mutex); wait_for_completion(&rng->cleanup_done); } EXPORT_SYMBOL_GPL(hwrng_unregister); @@ -682,7 +724,7 @@ static int __init hwrng_modinit(void) static void __exit hwrng_modexit(void) { mutex_lock(&rng_mutex); - BUG_ON(current_rng); + WARN_ON(rcu_access_pointer(current_rng)); kfree(rng_buffer); kfree(rng_fillbuf); mutex_unlock(&rng_mutex); diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h index b424555753b1..b77bc55a4cf3 100644 --- a/include/linux/hw_random.h +++ b/include/linux/hw_random.h @@ -15,6 +15,7 @@ #include #include #include +#include =20 /** * struct hwrng - Hardware Random Number Generator driver @@ -48,6 +49,7 @@ struct hwrng { /* internal. */ struct list_head list; struct kref ref; + struct work_struct cleanup_work; struct completion cleanup_done; struct completion dying; }; --=20 2.52.0