From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 B5D2B13D529; Wed, 22 May 2024 11:51:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716378701; cv=none; b=L5sSAc0WRhzGMCMfdE3Kj5uQPpjazH/VbHoAy4C4DspLXaXtLJr+IKJPX6imWbHyLGlX+OWyoWHMmFtEZ1OlLePBlvs7vswCJRnFTd7065qcwdLZZQlsiyk23QpFqDOZKI74V+z5BpWYlmjXnqyv/tiyVJWFd2NuE/A9PSFPrCE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716378701; c=relaxed/simple; bh=RftWGtYZutda+ggc/D573POE4wIZ/qWSGIv5UU3kk7M=; h=Mime-Version:Content-Type:Date:Message-Id:From:To:Cc:Subject: References:In-Reply-To; b=FqlmJCcbh6S344t95BxBCKTLsZCosc6RDAEuV0x2bIGUimyXgTfAj1MQDQgJG8U4ugvjodcVH41fY65F57UYfLyifEZEiUQgqEYHY6OTQ/QP/2nlhkUfhKK1lM6x0huCSeuigFdwqPKejRKyyDWdt0Wjg3HAuTwuKvS4IMVM+7Y= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=JSbf7IZj; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="JSbf7IZj" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 66520C32789; Wed, 22 May 2024 11:51:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1716378701; bh=RftWGtYZutda+ggc/D573POE4wIZ/qWSGIv5UU3kk7M=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JSbf7IZjd/vyIR9PbvZkzywbOC/7JgPk4Ft1uOCeUsg8qlVF2xTbFge+xHPoqXLfn 3BvURvg+Pmdq4Q5d8d2RYO6er39h39lw82zROGTPAOm7JTuDegGdAi88AFN0ykv3nq 22V5tPgjyqb/w76I1qEGdN9gtq3o1ZmqzKnGCIPDN0Vzy4nZsf3DgfyGnBNYmTEBxN F/NrKySOcElaoKFtM1/KSf+6TS1RPQxbmSIgTa4yRUGRRwfna+zYEgjogMpeU97YF0 LH3TeVZEj+HBOe1MLA1gmidl1+n9SFvX4w+YpNy08eO76YbPf29uc2AnSDNPorrb7V v2ibhomIu/T6w== Precedence: bulk X-Mailing-List: linux-integrity@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 Date: Wed, 22 May 2024 14:51:36 +0300 Message-Id: From: "Jarkko Sakkinen" To: "Herbert Xu" , =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= Cc: "Eric Biggers" , "James Bottomley" , "Ard Biesheuvel" , "Linux Crypto Mailing List" , , , , , "Linus Torvalds" , "Tejun Heo" , "Linux Kernel Mailing List" , "Kees Cook" Subject: Re: [v3 PATCH] hwrng: core - Remove add_early_randomness X-Mailer: aerc 0.17.0 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> In-Reply-To: On Wed May 22, 2024 at 8:37 AM EEST, Herbert Xu wrote: > On Tue, May 21, 2024 at 03:37:16PM -0400, N=C3=ADcolas F. R. A. Prado wro= te: > > > > FWIW this patch fixes the warning. So feel free to add > >=20 > > Tested-by: N=C3=ADcolas 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. "vestigial" did not know that word before ;-) Something learned. What is the kthread doing this currently? > > Reported-by: N=C3=ADcolas 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@nota= piano/ > Signed-off-by: Herbert Xu > > 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; > } > =20 > -static void add_early_randomness(struct hwrng *rng) > -{ > - int bytes_read; > - > - mutex_lock(&reading_mutex); > - bytes_read =3D rng_get_data(rng, rng_fillbuf, 32, 0); > - mutex_unlock(&reading_mutex); > - if (bytes_read > 0) { > - size_t entropy =3D 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 =3D 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; > =20 > err =3D mutex_lock_interruptible(&rng_mutex); > if (err) > return -ERESTARTSYS; > =20 > - old_rng =3D current_rng; > if (sysfs_streq(buf, "")) { > err =3D enable_best_rng(); > } else { > @@ -362,11 +348,8 @@ static ssize_t rng_current_store(struct device *dev, > new_rng =3D get_current_rng_nolock(); > mutex_unlock(&rng_mutex); > =20 > - if (new_rng) { > - if (new_rng !=3D old_rng) > - add_early_randomness(new_rng); > + if (new_rng) > put_rng(new_rng); > - } > =20 > return err ? : len; > } > @@ -544,7 +527,6 @@ int hwrng_register(struct hwrng *rng) > { > int err =3D -EINVAL; > struct hwrng *tmp; > - bool is_new_current =3D false; > =20 > if (!rng->name || (!rng->data_read && !rng->read)) > goto out; > @@ -573,25 +555,8 @@ int hwrng_register(struct hwrng *rng) > err =3D 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 =3D 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); > =20 > void hwrng_unregister(struct hwrng *rng) > { > - struct hwrng *old_rng, *new_rng; > + struct hwrng *new_rng; > int err; > =20 > mutex_lock(&rng_mutex); > =20 > - old_rng =3D current_rng; > list_del(&rng->list); > complete_all(&rng->dying); > if (current_rng =3D=3D rng) { > @@ -626,11 +590,8 @@ void hwrng_unregister(struct hwrng *rng) > } else > mutex_unlock(&rng_mutex); > =20 > - if (new_rng) { > - if (old_rng !=3D new_rng) > - add_early_randomness(new_rng); > + if (new_rng) > put_rng(new_rng); > - } > =20 > wait_for_completion(&rng->cleanup_done); > } I have no doubts that such thread would not exist, so: Reviewed-by: Jarkko Sakkinen BR, Jarkko