From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 07020C432C3 for ; Wed, 20 Nov 2019 20:08:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C538720721 for ; Wed, 20 Nov 2019 20:08:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=chronox.de header.i=@chronox.de header.b="OV1iKzLt" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727240AbfKTUIX (ORCPT ); Wed, 20 Nov 2019 15:08:23 -0500 Received: from mo4-p02-ob.smtp.rzone.de ([85.215.255.83]:12399 "EHLO mo4-p02-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726270AbfKTUIX (ORCPT ); Wed, 20 Nov 2019 15:08:23 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1574280499; s=strato-dkim-0002; d=chronox.de; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=A/JJtjiw5CgSnKOSzJJcy5f+8wH4gqBQ2bA1uDiXBQ4=; b=OV1iKzLtFiKtQXzQUuoKZu5b9ufiVcmxOLYGisEImwrUfnNLbEbaws53J8+R+Qih1I JaXlPyizXTHxZUsXsbUE3FN9Fa27VGg1+1TfVJCcjOIRv3U0WDYte+wfDy/OSYlhhGHT VpYNWF8YPEFfzYA329+GW7XRrPi5q6X2AAryu+4JFyMGOWTX+MLN2XU2vbuFCeZzSs0w soFN+mRxLpxaEvTyd+aLHMJ12UX7GBvZLqCnYWQfLf2c2Cyawz8M3PEOw7X+UlEWhuXQ kCCj914KIHpJU4imHJyQFQPFM6/tCciGkG99ZmthVbAr9pv3tFkK3Glki36+8vN3bYHR Utfw== X-RZG-AUTH: ":P2ERcEykfu11Y98lp/T7+hdri+uKZK8TKWEqNyiHySGSa9k9xmwdNnzHHXDbL/ScbtM=" X-RZG-CLASS-ID: mo00 Received: from positron.chronox.de by smtp.strato.de (RZmta 44.29.0 DYNA|AUTH) with ESMTPSA id N09a57vAKK7EmzS (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Wed, 20 Nov 2019 21:07:14 +0100 (CET) From: Stephan =?ISO-8859-1?Q?M=FCller?= To: Neil Horman Cc: Arnd Bergmann , Greg Kroah-Hartman , linux-crypto@vger.kernel.org, LKML , linux-api@vger.kernel.org, "Eric W. Biederman" , "Alexander E. Patrakov" , "Ahmed S. Darwish" , "Theodore Y. Ts'o" , Willy Tarreau , Matthew Garrett , Vito Caputo , Andreas Dilger , Jan Kara , Ray Strode , William Jon McCann , zhangjs , Andy Lutomirski , Florian Weimer , Lennart Poettering , Nicolai Stange , "Peter, Matthias" , Marcelo Henrique Cerri , Roman Drahtmueller Subject: Re: [PATCH v25 09/12] LRNG - add Jitter RNG fast noise source Date: Wed, 20 Nov 2019 21:07:13 +0100 Message-ID: <1844272.AK0ElEJLVa@positron.chronox.de> In-Reply-To: <20191120133303.GA28341@hmswarspite.think-freely.org> References: <6157374.ptSnyUpaCn@positron.chronox.de> <2377947.mlgTlHak1g@positron.chronox.de> <20191120133303.GA28341@hmswarspite.think-freely.org> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org Am Mittwoch, 20. November 2019, 14:33:03 CET schrieb Neil Horman: Hi Neil, > On Sat, Nov 16, 2019 at 10:36:52AM +0100, Stephan M=FCller wrote: > > The Jitter RNG fast noise source implemented as part of the kernel > > crypto API is queried for 256 bits of entropy at the time the seed > > buffer managed by the LRNG is about to be filled. > >=20 > > CC: "Eric W. Biederman" > > CC: "Alexander E. Patrakov" > > CC: "Ahmed S. Darwish" > > CC: "Theodore Y. Ts'o" > > CC: Willy Tarreau > > CC: Matthew Garrett > > CC: Vito Caputo > > CC: Andreas Dilger > > CC: Jan Kara > > CC: Ray Strode > > CC: William Jon McCann > > CC: zhangjs > > CC: Andy Lutomirski > > CC: Florian Weimer > > CC: Lennart Poettering > > CC: Nicolai Stange > > Reviewed-by: Marcelo Henrique Cerri > > Reviewed-by: Roman Drahtmueller > > Tested-by: Roman Drahtm=FCller > > Tested-by: Marcelo Henrique Cerri > > Tested-by: Neil Horman > > Signed-off-by: Stephan Mueller > > --- > >=20 > > drivers/char/lrng/Kconfig | 11 +++++ > > drivers/char/lrng/Makefile | 1 + > > drivers/char/lrng/lrng_jent.c | 88 +++++++++++++++++++++++++++++++++++ > > 3 files changed, 100 insertions(+) > > create mode 100644 drivers/char/lrng/lrng_jent.c > >=20 > > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig > > index 03e6e2ec356b..80fc723c67d2 100644 > > --- a/drivers/char/lrng/Kconfig > > +++ b/drivers/char/lrng/Kconfig > > @@ -80,4 +80,15 @@ config LRNG_KCAPI > >=20 > > provided by the selected kernel crypto API RNG. > > =20 > > endif # LRNG_DRNG_SWITCH > >=20 > > +config LRNG_JENT > > + bool "Enable Jitter RNG as LRNG Seed Source" > > + select CRYPTO_JITTERENTROPY > > + help > > + The Linux RNG may use the Jitter RNG as noise source. Enabling > > + this option enables the use of the Jitter RNG. Its default > > + entropy level is 16 bits of entropy per 256 data bits delivered > > + by the Jitter RNG. This entropy level can be changed at boot > > + time or at runtime with the lrng_base.jitterrng configuration > > + variable. > > + > >=20 > > endif # LRNG > >=20 > > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile > > index 027b6ea51c20..a87d800c9aae 100644 > > --- a/drivers/char/lrng/Makefile > > +++ b/drivers/char/lrng/Makefile > > @@ -13,3 +13,4 @@ obj-$(CONFIG_SYSCTL) +=3D lrng_proc.o > >=20 > > obj-$(CONFIG_LRNG_DRNG_SWITCH) +=3D lrng_switch.o > > obj-$(CONFIG_LRNG_DRBG) +=3D lrng_drbg.o > > obj-$(CONFIG_LRNG_KCAPI) +=3D lrng_kcapi.o > >=20 > > +obj-$(CONFIG_LRNG_JENT) +=3D lrng_jent.o > > diff --git a/drivers/char/lrng/lrng_jent.c b/drivers/char/lrng/lrng_jen= t.c > > new file mode 100644 > > index 000000000000..43114a44b8f5 > > --- /dev/null > > +++ b/drivers/char/lrng/lrng_jent.c > > @@ -0,0 +1,88 @@ > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause > > +/* > > + * LRNG Fast Noise Source: Jitter RNG > > + * > > + * Copyright (C) 2016 - 2019, Stephan Mueller > > + */ > > + > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > + > > +#include "lrng_internal.h" > > + > > +/* > > + * Estimated entropy of data is a 16th of > > LRNG_DRNG_SECURITY_STRENGTH_BITS. + * Albeit a full entropy assessment = is > > provided for the noise source indicating + * that it provides high > > entropy rates and considering that it deactivates + * when it detects > > insufficient hardware, the chosen under estimation of + * entropy is > > considered to be acceptable to all reviewers. > > + */ > > +static u32 jitterrng =3D LRNG_DRNG_SECURITY_STRENGTH_BITS>>4; > > +module_param(jitterrng, uint, 0644); > > +MODULE_PARM_DESC(jitterrng, "Entropy in bits of 256 data bits from Jit= ter > > " + "RNG noise source"); > > + > > +/** > > + * Get Jitter RNG entropy > > + * > > + * @outbuf buffer to store entropy > > + * @outbuflen length of buffer > > + * @return > 0 on success where value provides the added entropy in bi= ts > > + * 0 if no fast source was available > > + */ > > +struct rand_data; > > +struct rand_data *jent_lrng_entropy_collector(void); > > +int jent_read_entropy(struct rand_data *ec, unsigned char *data, > > + unsigned int len); > > +static struct rand_data *lrng_jent_state; > > + > > +u32 lrng_get_jent(u8 *outbuf, unsigned int outbuflen) > > +{ > > + int ret; > > + u32 ent_bits =3D jitterrng; > > + unsigned long flags; > > + static DEFINE_SPINLOCK(lrng_jent_lock); > > + static int lrng_jent_initialized =3D 0; > > + > > + spin_lock_irqsave(&lrng_jent_lock, flags); > > + > > + if (!ent_bits || (lrng_jent_initialized =3D=3D -1)) { > > + spin_unlock_irqrestore(&lrng_jent_lock, flags); > > + return 0; > > + } > > + >=20 > this works, but I think you can avoid the use of the spin lock on the read > calls here. If you assign a global pointer to the value of > &lrng_jent_state on init, you can just take the spinlock on assignment, a= nd > assume its stable after that (which it should be given that its only ever > going to point to a static data structure). It is correct that the lock protects the assignment of the data structure. But the Jitter RNG itself is not multi-threaded. So, a form of serializatio= n=20 is needed to also "read" data from the Jitter RNG using one and the same=20 state. Granted, there is a serialization in the current code as the=20 lrng_pool_trylock() is taken before the Jitter RNG is called by=20 lrng_fill_seed_buffer which effectively serializes all requests to also the= =20 Jitter RNG. But this is coincidence in this case. I would think, however, t= hat=20 this coincidence could easily lead to programming errors further down the r= oad=20 when the spinlock is not present and that trylock() is moved to some place= =20 else considering that this trylock() is meant to protect reading the entrop= y=20 pool and not the Jitter RNG. As the reading of the Jitter RNG is always performed in process context, I= =20 think having this additional spin lock against possible programming errors= =20 should not lead to performance regressions. What do you think? Thank you for your review! Ciao Stephan