The khwrng can terminate also if the rng is removed, and in this case it doesn't synchronize with kthread_should_stop(), but it directly sets hwrng_fill to NULL. If this happens after the NULL check but before kthread_stop() is called, we'll have a NULL pointer dereference. Keep the hwrng_fill thread around and only park it when unused, i.e. when there is no RNG available. Unpark it when RNG becomes available. This way, we are sure to never trigger the NULL pointer dereference. Signed-off-by: Marek Vasut <marex@xxxxxxx> --- This is a follow-up on first part of V2 of work by Luca Dariz: https://lore.kernel.org/all/AM6PR06MB5400DAFE0551F1D468B728FBAB889@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ --- Cc: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx> Cc: Harald Freudenberger <freude@xxxxxxxxxxxxx> Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> Cc: Li Zhijian <lizhijian@xxxxxxxxxxx> Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> Cc: Olivia Mackall <olivia@xxxxxxxxxxx> Cc: linux-crypto@xxxxxxxxxxxxxxx --- drivers/char/hw_random/core.c | 71 ++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 018316f546215..5be26f4e9d975 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -87,15 +87,6 @@ static int set_current_rng(struct hwrng *rng) drop_current_rng(); current_rng = rng; - /* if necessary, start hwrng thread */ - if (!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; - } - } - return 0; } @@ -484,10 +475,17 @@ static int hwrng_fillfn(void *unused) while (!kthread_should_stop()) { unsigned short quality; struct hwrng *rng; + if (kthread_should_park()) { + kthread_parkme(); + continue; + } rng = get_current_rng(); - if (IS_ERR(rng) || !rng) - break; + if (IS_ERR(rng) || !rng) { + schedule(); + continue; + } + mutex_lock(&reading_mutex); rc = rng_get_data(rng, rng_fillbuf, rng_buffer_size(), 1); @@ -515,12 +513,12 @@ static int hwrng_fillfn(void *unused) add_hwgenerator_randomness((void *)rng_fillbuf, rc, entropy >> 10, true); } - hwrng_fill = NULL; return 0; } int hwrng_register(struct hwrng *rng) { + struct hwrng *old_rng; int err = -EINVAL; struct hwrng *tmp; @@ -544,6 +542,7 @@ int hwrng_register(struct hwrng *rng) /* Adjust quality field to always have a proper value */ rng->quality = min_t(u16, min_t(u16, default_quality, 1024), rng->quality ?: 1024); + old_rng = current_rng; if (!current_rng || (!cur_rng_set_by_user && rng->quality > current_rng->quality)) { /* @@ -556,6 +555,10 @@ int hwrng_register(struct hwrng *rng) goto out_unlock; } mutex_unlock(&rng_mutex); + + if (!old_rng && current_rng) + kthread_unpark(hwrng_fill); + return 0; out_unlock: mutex_unlock(&rng_mutex); @@ -582,15 +585,12 @@ void hwrng_unregister(struct hwrng *rng) } new_rng = 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); + mutex_unlock(&rng_mutex); if (new_rng) put_rng(new_rng); + else + kthread_park(hwrng_fill); wait_for_completion(&rng->cleanup_done); } @@ -654,7 +654,7 @@ EXPORT_SYMBOL_GPL(hwrng_yield); static int __init hwrng_modinit(void) { - int ret; + int ret = -ENOMEM; /* kmalloc makes this safe for virt_to_page() in virtio_rng.c */ rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL); @@ -662,22 +662,41 @@ static int __init hwrng_modinit(void) return -ENOMEM; rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL); - if (!rng_fillbuf) { - kfree(rng_buffer); - return -ENOMEM; - } + if (!rng_fillbuf) + goto err_fillbuf; ret = misc_register(&rng_miscdev); - if (ret) { - kfree(rng_fillbuf); - kfree(rng_buffer); + if (ret) + goto err_miscdev; + + hwrng_fill = kthread_create(hwrng_fillfn, NULL, "hwrng"); + if (IS_ERR(hwrng_fill)) { + ret = PTR_ERR(hwrng_fill); + pr_err("hwrng_fill thread creation failed (%d)\n", ret); + goto err_kthread; } + ret = kthread_park(hwrng_fill); + if (ret) + goto err_park; + + return ret; + +err_park: + kthread_stop(hwrng_fill); +err_kthread: + misc_deregister(&rng_miscdev); +err_miscdev: + kfree(rng_buffer); +err_fillbuf: + kfree(rng_buffer); return ret; } static void __exit hwrng_modexit(void) { + kthread_stop(hwrng_fill); + mutex_lock(&rng_mutex); BUG_ON(current_rng); kfree(rng_buffer); -- 2.45.2
