From patchwork Thu Oct 24 16:30:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 838839 Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (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 78F9A1C728E for ; Thu, 24 Oct 2024 16:31:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=85.214.62.61 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729787510; cv=none; b=jOk7V8ncM5mW26e7lc9L65f95YXpo3x6kgSeOziECVv1eEVXgaTig2u3OmBp8sdc8D1AJkGm94UvHqVZ2ZWQTVrzcYk3VPM3pFGuYCP5wvdDFGWCMA6FODko0s74bDTwvihPebX0YCgLP8dwGOZOd4W011duhiOdHx7Gcj5dqdk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729787510; c=relaxed/simple; bh=ZO9k30mb+ULzxzsxBTttL7WgHHyAz6SAieYZ59l0uqo=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=BACcgBgwx8y7dkX/HlmxcmLRrx/mn19l1th9IoYjwBctj8HmgDsprG8jiMi9TBFJSDhHIl8v6wPdd84Gbue/ufmcadbxujFjzHWkmxbN1GMmjfnGzj+kO7pAXfJ2KJo2a8zXx/PjGQHtcVV+TqzSY5QbKaka3XbFbX41nig01qY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=denx.de; spf=pass smtp.mailfrom=denx.de; dkim=pass (2048-bit key) header.d=denx.de header.i=@denx.de header.b=ztEQ7hdG; arc=none smtp.client-ip=85.214.62.61 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=denx.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=denx.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=denx.de header.i=@denx.de header.b="ztEQ7hdG" Received: from tr.lan (ip-86-49-120-218.bb.vodafone.cz [86.49.120.218]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id AF60488F55; Thu, 24 Oct 2024 18:31:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1729787500; bh=uT3dmEunMhyO4RLAkq3R/O0Yfgr5p6PgNIFHGQuE2KI=; h=From:To:Cc:Subject:Date:From; b=ztEQ7hdGOSnRJLQ/axPyhXKTmHaCguHEM1lw4QON/dLqtuRKNoGgvS6k+Z2DEna6V GA+FY48xWim3TUtbmP8d4QOATWDyQvxHoVikNM0IuApxqpluNTBhYK2NptotxYX1oj s8Zk+ZUNCt+FITHzLCKXAvidP7/MKXUcInj7+kMslY776OuakiLxndSJvgDrc0B8nR TrJqq6qgIlEfd5cfuW8cnrbXle2DL8P5NqMyUU3HZUYZtYVct9WoIcQYkn5RxwTmm3 jNWqnVP6ea9H/QBNiiMNxbW1uFM1GagaHlgQYK9P9a96PMINIBhuq3vk0YWK+5Fpks PAFZboWFTp1Vg== From: Marek Vasut To: linux-crypto@vger.kernel.org Cc: Marek Vasut , Dominik Brodowski , Harald Freudenberger , Herbert Xu , Li Zhijian , Masahiro Yamada , Olivia Mackall Subject: [PATCH 1/2] [RFC] hwrng: fix khwrng lifecycle Date: Thu, 24 Oct 2024 18:30:15 +0200 Message-ID: <20241024163121.246420-1-marex@denx.de> X-Mailer: git-send-email 2.45.2 Precedence: bulk X-Mailing-List: linux-crypto@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean 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 --- This is a follow-up on first part of V2 of work by Luca Dariz: https://lore.kernel.org/all/AM6PR06MB5400DAFE0551F1D468B728FBAB889@AM6PR06MB5400.eurprd06.prod.outlook.com/ --- Cc: Dominik Brodowski Cc: Harald Freudenberger Cc: Herbert Xu Cc: Li Zhijian Cc: Masahiro Yamada Cc: Olivia Mackall Cc: linux-crypto@vger.kernel.org --- 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); From patchwork Thu Oct 24 16:30:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 838227 Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (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 873D21E32AE for ; Thu, 24 Oct 2024 16:31:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=85.214.62.61 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729787510; cv=none; b=t80YF0PiWQq0Geot+C6EIbIAYGHDzVqv4gcCaJezNePlVqhSijt85HxPwW/E/NDJoXDpsElYrKcjsshhdlptRUl2N9VFncMf4oTGA1a3HUkDVFmDBCzKmkTTWs5Ml0p5U/vH2pa8h728O0NoXXm2AGzadiajh04pdXKtrcgLhEs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729787510; c=relaxed/simple; bh=nO3ERuTEU0118RKUrpTGZ8JICg14sxlUIj9uF/rDHwI=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=m09g7Kvd8KZCral47j5am2mmuae+Q7GCZ/bfEzVjeyOk/EdOHbuDyEeKg330s5PkTvK4NduBxMky1jGFW6QwiK8ErnN8OHDlRX0g1W1f6oZKnyu37WgDRAOIkKlOOMu86mDw2USWYgIPhyBbwiBgsTuSpHwWsMV5Jtkd8JPX6gA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=denx.de; spf=pass smtp.mailfrom=denx.de; dkim=pass (2048-bit key) header.d=denx.de header.i=@denx.de header.b=REy65Q/F; arc=none smtp.client-ip=85.214.62.61 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=denx.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=denx.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=denx.de header.i=@denx.de header.b="REy65Q/F" Received: from tr.lan (ip-86-49-120-218.bb.vodafone.cz [86.49.120.218]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 778F788F83; Thu, 24 Oct 2024 18:31:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1729787500; bh=Rsh4v6/PeIHtySykJlhUm80gJ6K4ieESJR6idPtAPCU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=REy65Q/F1gvwiJpjztNedWHIiC8K+PJKQshLGDT6deiy62R/3kOE/x7q3ekYjzeSr h7vUBKIwdjSad4stIiqnCG9PsXdKFV67OlddeeSH6w0LB0YSOYPe+bMELr+HDoDXwB eJbBkT8zHCaxl2y2wh3T6tNZeXMHGPPw5KrLXr1Ygy0N6v38a9SuC9z0WWxSgYyX5T ql5sE7DK11Aezu7IU2bXJL/AiYkMU3m5NoHu48GFauPV83W0jk00LboGnDglvEpDDn SxyX4qE8q9zrSsCfE109Ir5CZoPA/nLReIPRd/qWy6gFC+Mr4+bGIvDoHFlwTjPXh1 qksjl/iyWrkfQ== From: Marek Vasut To: linux-crypto@vger.kernel.org Cc: Marek Vasut , Dominik Brodowski , Harald Freudenberger , Herbert Xu , Li Zhijian , Masahiro Yamada , Olivia Mackall Subject: [PATCH 2/2] [RFC] hwrng: fix khwrng registration Date: Thu, 24 Oct 2024 18:30:16 +0200 Message-ID: <20241024163121.246420-2-marex@denx.de> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241024163121.246420-1-marex@denx.de> References: <20241024163121.246420-1-marex@denx.de> Precedence: bulk X-Mailing-List: linux-crypto@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean If we have a register/unregister too fast, it can happen that the kthread is not yet started when kthread_stop is called, and this seems to leave a corrupted or uninitialized kthread struct. This is detected by the WARN_ON at kernel/kthread.c:75 and later causes a page domain fault. Wait for the kthread to start the same way as drivers/base/devtmpfs.c does wait for kdevtmpfs thread to start using setup_done completion. Signed-off-by: Marek Vasut --- This is a follow-up on second part of V2 of work by Luca Dariz: https://lore.kernel.org/all/AM6PR06MB5400DAFE0551F1D468B728FBAB889@AM6PR06MB5400.eurprd06.prod.outlook.com/ --- Cc: Dominik Brodowski Cc: Harald Freudenberger Cc: Herbert Xu Cc: Li Zhijian Cc: Masahiro Yamada Cc: Olivia Mackall Cc: linux-crypto@vger.kernel.org --- drivers/char/hw_random/core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 5be26f4e9d975..bb1f4ba602b1d 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -55,6 +55,7 @@ MODULE_PARM_DESC(default_quality, static void drop_current_rng(void); static int hwrng_init(struct hwrng *rng); static int hwrng_fillfn(void *unused); +static DECLARE_COMPLETION(setup_done); static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size, int wait); @@ -472,6 +473,7 @@ static int hwrng_fillfn(void *unused) size_t entropy, entropy_credit = 0; /* in 1/1024 of a bit */ long rc; + complete(&setup_done); while (!kthread_should_stop()) { unsigned short quality; struct hwrng *rng; @@ -669,13 +671,15 @@ static int __init hwrng_modinit(void) if (ret) goto err_miscdev; - hwrng_fill = kthread_create(hwrng_fillfn, NULL, "hwrng"); + hwrng_fill = kthread_run(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; } + wait_for_completion(&setup_done); + ret = kthread_park(hwrng_fill); if (ret) goto err_park;