From patchwork Wed May 22 05:37:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 13670429 Received: from abb.hmeau.com (abb.hmeau.com [144.6.53.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 96736770E6; Wed, 22 May 2024 05:37:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=144.6.53.87 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716356284; cv=none; b=HjEtWMNXnW1NSIrZWE2XezxCu+nMqzcWUtBuE7vk7t2cCi0vmbI9boztYvU+hyQCv9G5ul3n/E9DqtyUwuk/3OIo6zjapHE3pposU3hmY+4Y08NHTBXov3ftCiNrwW6vDa+NuEbaEyKg7JXaVkM7ctPdVeE/yS/9z2Ysw8Vs7vA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716356284; c=relaxed/simple; bh=/95XhBabHkx9klpzhI23q0R9Gqb7kl8/396FtFx7jxE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KRrXkNqquK+bYzaoR15hb4l9yjlbdV0+yev8qwmh0//b+rStkp6U20+s51OI99SbI+yqmtEvPJyn/FKN/O5+Qt5i3m+lJZTToc/333a61NqJi/1Pzpzi0JWKq7uxVUI4OesLHyGcqXCMAFLYJTnxIpufJ3YsgPL3b2ODaAAy8xk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gondor.apana.org.au; spf=pass smtp.mailfrom=gondor.apana.org.au; arc=none smtp.client-ip=144.6.53.87 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gondor.apana.org.au Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gondor.apana.org.au Received: from loth.rohan.me.apana.org.au ([192.168.167.2]) by formenos.hmeau.com with smtp (Exim 4.96 #2 (Debian)) id 1s9efh-000nse-0t; Wed, 22 May 2024 13:37:54 +0800 Received: by loth.rohan.me.apana.org.au (sSMTP sendmail emulation); Wed, 22 May 2024 13:37:54 +0800 Date: Wed, 22 May 2024 13:37:54 +0800 From: Herbert Xu To: =?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado Cc: Eric Biggers , Jarkko Sakkinen , James Bottomley , Ard Biesheuvel , Linux Crypto Mailing List , linux-integrity@vger.kernel.org, keyrings@vger.kernel.org, regressions@lists.linux.dev, kernel@collabora.com, Linus Torvalds , Tejun Heo , Linux Kernel Mailing List , Kees Cook Subject: [v3 PATCH] hwrng: core - Remove add_early_randomness Message-ID: References: <0d260c2f7a9f67ec8bd2305919636678d06000d1.camel@HansenPartnership.com> <66ec985f3ee229135bf748f1b0874d5367a74d7f.camel@HansenPartnership.com> <20240518043115.GA53815@sol.localdomain> <00bcfa65-384d-46ae-ab8b-30f12487928b@notapiano> <07512097-8198-4a84-b166-ef9809c2913b@notapiano> Precedence: bulk X-Mailing-List: linux-integrity@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <07512097-8198-4a84-b166-ef9809c2913b@notapiano> On Tue, May 21, 2024 at 03:37:16PM -0400, Nícolas F. R. A. Prado wrote: > > FWIW this patch fixes the warning. So feel free to add > > Tested-by: Nícolas F. R. A. Prado Could you please test this patch instead? ---8<--- A potential deadlock was reported with the config file at https://web.archive.org/web/20240522052129/https://0x0.st/XPN_.txt In this particular configuration, the deadlock doesn't exist because the warning triggered at a point before modules were even available. However, the deadlock can be real because any module loaded would invoke async_synchronize_full. The issue is spurious for software crypto algorithms which aren't themselves involved in async probing. However, it would be hard to avoid for a PCI crypto driver using async probing. In this particular call trace, the problem is easily avoided because the only reason the module is being requested during probing is the add_early_randomness call in the hwrng core. This feature is vestigial since there is now a kernel thread dedicated to doing exactly this. So remove add_early_randomness as it is no longer needed. Reported-by: Nícolas F. R. A. Prado Reported-by: Eric Biggers Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()") Link: https://lore.kernel.org/r/119dc5ed-f159-41be-9dda-1a056f29888d@notapiano/ Signed-off-by: Herbert Xu Reviewed-by: Jarkko Sakkinen Tested-by: Nícolas F. R. A. Prado diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index f5c71a617a99..4084df65c9fa 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -64,19 +64,6 @@ static size_t rng_buffer_size(void) return RNG_BUFFER_SIZE; } -static void add_early_randomness(struct hwrng *rng) -{ - int bytes_read; - - mutex_lock(&reading_mutex); - bytes_read = rng_get_data(rng, rng_fillbuf, 32, 0); - mutex_unlock(&reading_mutex); - if (bytes_read > 0) { - size_t entropy = bytes_read * 8 * rng->quality / 1024; - add_hwgenerator_randomness(rng_fillbuf, bytes_read, entropy, false); - } -} - static inline void cleanup_rng(struct kref *kref) { struct hwrng *rng = container_of(kref, struct hwrng, ref); @@ -340,13 +327,12 @@ static ssize_t rng_current_store(struct device *dev, const char *buf, size_t len) { int err; - struct hwrng *rng, *old_rng, *new_rng; + struct hwrng *rng, *new_rng; err = mutex_lock_interruptible(&rng_mutex); if (err) return -ERESTARTSYS; - old_rng = current_rng; if (sysfs_streq(buf, "")) { err = enable_best_rng(); } else { @@ -362,11 +348,8 @@ static ssize_t rng_current_store(struct device *dev, new_rng = get_current_rng_nolock(); mutex_unlock(&rng_mutex); - if (new_rng) { - if (new_rng != old_rng) - add_early_randomness(new_rng); + if (new_rng) put_rng(new_rng); - } return err ? : len; } @@ -544,7 +527,6 @@ int hwrng_register(struct hwrng *rng) { int err = -EINVAL; struct hwrng *tmp; - bool is_new_current = false; if (!rng->name || (!rng->data_read && !rng->read)) goto out; @@ -573,25 +555,8 @@ int hwrng_register(struct hwrng *rng) err = set_current_rng(rng); if (err) goto out_unlock; - /* to use current_rng in add_early_randomness() we need - * to take a ref - */ - is_new_current = true; - kref_get(&rng->ref); } mutex_unlock(&rng_mutex); - if (is_new_current || !rng->init) { - /* - * Use a new device's input to add some randomness to - * the system. If this rng device isn't going to be - * used right away, its init function hasn't been - * called yet by set_current_rng(); so only use the - * randomness from devices that don't need an init callback - */ - add_early_randomness(rng); - } - if (is_new_current) - put_rng(rng); return 0; out_unlock: mutex_unlock(&rng_mutex); @@ -602,12 +567,11 @@ EXPORT_SYMBOL_GPL(hwrng_register); void hwrng_unregister(struct hwrng *rng) { - struct hwrng *old_rng, *new_rng; + struct hwrng *new_rng; int err; mutex_lock(&rng_mutex); - old_rng = current_rng; list_del(&rng->list); complete_all(&rng->dying); if (current_rng == rng) { @@ -626,11 +590,8 @@ void hwrng_unregister(struct hwrng *rng) } else mutex_unlock(&rng_mutex); - if (new_rng) { - if (old_rng != new_rng) - add_early_randomness(new_rng); + if (new_rng) put_rng(new_rng); - } wait_for_completion(&rng->cleanup_done); }