2020-01-09 08:41:11

by Stephan Müller

[permalink] [raw]
Subject: [PATCH v27 04/12] LRNG - add switchable DRNG support

The DRNG switch support allows replacing the DRNG mechanism of the
LRNG. The switching support rests on the interface definition of
include/linux/lrng.h. A new DRNG is implemented by filling in the
interface defined in this header file.

In addition to the DRNG, the extension also has to provide a hash
implementation that is used to hash the entropy pool for random number
extraction.

Note: It is permissible to implement a DRNG whose operations may sleep.
However, the hash function must not sleep.

The switchable DRNG support allows replacing the DRNG at runtime.
However, only one DRNG extension is allowed to be loaded at any given
time. Before replacing it with another DRNG implementation, the possibly
existing DRNG extension must be unloaded.

The switchable DRNG extension activates the new DRNG during load time.
It is expected, however, that such a DRNG switch would be done only once
by an administrator to load the intended DRNG implementation.

It is permissible to compile DRNG extensions either as kernel modules or
statically. The initialization of the DRNG extension should be performed
with a late_initcall to ensure the extension is available when user
space starts but after all other initialization completed.
The initialization is performed by registering the function call data
structure with the lrng_set_drng_cb function. In order to unload the
DRNG extension, lrng_set_drng_cb must be invoked with the NULL
parameter.

The DRNG extension should always provide a security strength that is at
least as strong as LRNG_DRNG_SECURITY_STRENGTH_BITS.

CC: "Eric W. Biederman" <[email protected]>
CC: "Alexander E. Patrakov" <[email protected]>
CC: "Ahmed S. Darwish" <[email protected]>
CC: "Theodore Y. Ts'o" <[email protected]>
CC: Willy Tarreau <[email protected]>
CC: Matthew Garrett <[email protected]>
CC: Vito Caputo <[email protected]>
CC: Andreas Dilger <[email protected]>
CC: Jan Kara <[email protected]>
CC: Ray Strode <[email protected]>
CC: William Jon McCann <[email protected]>
CC: zhangjs <[email protected]>
CC: Andy Lutomirski <[email protected]>
CC: Florian Weimer <[email protected]>
CC: Lennart Poettering <[email protected]>
CC: Nicolai Stange <[email protected]>
Reviewed-by: Marcelo Henrique Cerri <[email protected]>
Reviewed-by: Roman Drahtmueller <[email protected]>
Tested-by: Roman Drahtm?ller <[email protected]>
Tested-by: Marcelo Henrique Cerri <[email protected]>
Tested-by: Neil Horman <[email protected]>
Signed-off-by: Stephan Mueller <[email protected]>
---
drivers/char/lrng/Kconfig | 7 ++
drivers/char/lrng/Makefile | 1 +
drivers/char/lrng/lrng_switch.c | 179 ++++++++++++++++++++++++++++++++
3 files changed, 187 insertions(+)
create mode 100644 drivers/char/lrng/lrng_switch.c

diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
index 56f13efd3592..cb701bb0b8b6 100644
--- a/drivers/char/lrng/Kconfig
+++ b/drivers/char/lrng/Kconfig
@@ -64,4 +64,11 @@ config LRNG_POOL_SIZE
default 7 if LRNG_POOL_SIZE_65536
default 8 if LRNG_POOL_SIZE_131072

+menuconfig LRNG_DRNG_SWITCH
+ bool "Support DRNG runtime switching"
+ help
+ The Linux RNG per default uses a ChaCha20 DRNG that is
+ accessible via the external interfaces. With this configuration
+ option other DRNGs can be selected and loaded at runtime.
+
endif # LRNG
diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
index e69c176f0161..31cfe87c999e 100644
--- a/drivers/char/lrng/Makefile
+++ b/drivers/char/lrng/Makefile
@@ -10,3 +10,4 @@ obj-y += lrng_pool.o lrng_aux.o \

obj-$(CONFIG_NUMA) += lrng_numa.o
obj-$(CONFIG_SYSCTL) += lrng_proc.o
+obj-$(CONFIG_LRNG_DRNG_SWITCH) += lrng_switch.o
diff --git a/drivers/char/lrng/lrng_switch.c b/drivers/char/lrng/lrng_switch.c
new file mode 100644
index 000000000000..2c7468d8de09
--- /dev/null
+++ b/drivers/char/lrng/lrng_switch.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * LRNG DRNG switching support
+ *
+ * Copyright (C) 2016 - 2020, Stephan Mueller <[email protected]>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/lrng.h>
+
+#include "lrng_internal.h"
+
+static int lrng_drng_switch(struct lrng_drng *drng_store,
+ const struct lrng_crypto_cb *cb, int node)
+{
+ const struct lrng_crypto_cb *old_cb;
+ unsigned long flags = 0;
+ int ret;
+ u8 seed[LRNG_DRNG_SECURITY_STRENGTH_BYTES];
+ void *new_drng = cb->lrng_drng_alloc(LRNG_DRNG_SECURITY_STRENGTH_BYTES);
+ void *old_drng, *new_hash, *old_hash;
+ bool sl = false, reset_drng = !lrng_get_available();
+
+ if (IS_ERR(new_drng)) {
+ pr_warn("could not allocate new DRNG for NUMA node %d (%ld)\n",
+ node, PTR_ERR(new_drng));
+ return PTR_ERR(new_drng);
+ }
+
+ new_hash = cb->lrng_hash_alloc(seed, sizeof(seed));
+ if (IS_ERR(new_hash)) {
+ pr_warn("could not allocate new LRNG pool hash (%ld)\n",
+ PTR_ERR(new_hash));
+ cb->lrng_drng_dealloc(new_drng);
+ return PTR_ERR(new_hash);
+ }
+
+ lrng_drng_lock(drng_store, &flags);
+
+ /*
+ * Pull from existing DRNG to seed new DRNG regardless of seed status
+ * of old DRNG -- the entropy state for the DRNG is left unchanged which
+ * implies that als the new DRNG is reseeded when deemed necessary. This
+ * seeding of the new DRNG shall only ensure that the new DRNG has the
+ * same entropy as the old DRNG.
+ */
+ ret = drng_store->crypto_cb->lrng_drng_generate_helper(
+ drng_store->drng, seed, sizeof(seed));
+ lrng_drng_unlock(drng_store, &flags);
+
+ if (ret < 0) {
+ reset_drng = true;
+ pr_warn("getting random data from DRNG failed for NUMA node %d "
+ "(%d)\n", node, ret);
+ } else {
+ /* seed new DRNG with data */
+ ret = cb->lrng_drng_seed_helper(new_drng, seed, ret);
+ if (ret < 0) {
+ reset_drng = true;
+ pr_warn("seeding of new DRNG failed for NUMA node %d "
+ "(%d)\n", node, ret);
+ } else {
+ pr_debug("seeded new DRNG of NUMA node %d instance "
+ "from old DRNG instance\n", node);
+ }
+ }
+
+ mutex_lock(&drng_store->lock);
+ /*
+ * If we switch the DRNG from the initial ChaCha20 DRNG to something
+ * else, there is a lock transition from spin lock to mutex (see
+ * lrng_drng_is_atomic and how the lock is taken in lrng_drng_lock).
+ * Thus, we need to take both locks during the transition phase.
+ */
+ if (lrng_drng_is_atomic(drng_store)) {
+ spin_lock_irqsave(&drng_store->spin_lock, flags);
+ sl = true;
+ }
+
+ if (reset_drng)
+ lrng_drng_reset(drng_store);
+
+ old_drng = drng_store->drng;
+ old_cb = drng_store->crypto_cb;
+ drng_store->drng = new_drng;
+ drng_store->crypto_cb = cb;
+
+ old_hash = drng_store->hash;
+ drng_store->hash = new_hash;
+ pr_info("Entropy pool read-hash allocated for DRNG for NUMA node %d\n",
+ node);
+
+ if (sl)
+ spin_unlock_irqrestore(&drng_store->spin_lock, flags);
+ mutex_unlock(&drng_store->lock);
+
+ /* ChaCha20 serves as atomic instance left untouched. */
+ if (old_drng != &chacha20) {
+ old_cb->lrng_drng_dealloc(old_drng);
+ old_cb->lrng_hash_dealloc(old_hash);
+ }
+
+ pr_info("DRNG of NUMA node %d switched\n", node);
+
+ return 0;
+}
+
+/**
+ * Switch the existing DRNG instances with new using the new crypto callbacks.
+ * The caller must hold the lrng_crypto_cb_update lock.
+ */
+static int lrng_drngs_switch(const struct lrng_crypto_cb *cb)
+{
+ struct lrng_drng **lrng_drng = lrng_drng_instances();
+ struct lrng_drng *lrng_drng_init = lrng_drng_init_instance();
+ int ret = 0;
+
+ /* Update DRNG */
+ if (lrng_drng) {
+ u32 node;
+
+ for_each_online_node(node) {
+ if (lrng_drng[node])
+ ret = lrng_drng_switch(lrng_drng[node], cb,
+ node);
+ }
+ } else {
+ ret = lrng_drng_switch(lrng_drng_init, cb, 0);
+ }
+
+ if (!ret)
+ lrng_set_available();
+
+ return 0;
+}
+
+/**
+ * lrng_set_drng_cb - Register new cryptographic callback functions for DRNG
+ * The registering implies that all old DRNG states are replaced with new
+ * DRNG states.
+ * @cb: Callback functions to be registered -- if NULL, use the default
+ * callbacks pointing to the ChaCha20 DRNG.
+ * @return: 0 on success, < 0 on error
+ */
+int lrng_set_drng_cb(const struct lrng_crypto_cb *cb)
+{
+ struct lrng_drng *lrng_drng_init = lrng_drng_init_instance();
+ int ret;
+
+ if (!cb)
+ cb = &lrng_cc20_crypto_cb;
+
+ mutex_lock(&lrng_crypto_cb_update);
+
+ /*
+ * If a callback other than the default is set, allow it only to be
+ * set back to the default callback. This ensures that multiple
+ * different callbacks can be registered at the same time. If a
+ * callback different from the current callback and the default
+ * callback shall be set, the current callback must be deregistered
+ * (e.g. the kernel module providing it must be unloaded) and the new
+ * implementation can be registered.
+ */
+ if ((cb != &lrng_cc20_crypto_cb) &&
+ (lrng_drng_init->crypto_cb != &lrng_cc20_crypto_cb)) {
+ pr_warn("disallow setting new cipher callbacks, unload the old "
+ "callbacks first!\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = lrng_drngs_switch(cb);
+
+out:
+ mutex_unlock(&lrng_crypto_cb_update);
+ return ret;
+}
+EXPORT_SYMBOL(lrng_set_drng_cb);
--
2.24.1





2020-01-11 07:11:22

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v27 04/12] LRNG - add switchable DRNG support

Hi "Stephan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on cryptodev/master crypto/master v5.5-rc5 next-20200110]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Stephan-M-ller/dev-random-a-new-approach-with-full-SP800-90B/20200110-084934
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 68faa679b8be1a74e6663c21c3a9d25d32f1c079
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-129-g341daf20-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>


sparse warnings: (new ones prefixed by >>)

>> drivers/char/lrng/lrng_internal.h:239:39: sparse: sparse: context imbalance in 'lrng_drng_switch' - unexpected unlock

vim +/lrng_drng_switch +239 drivers/char/lrng/lrng_internal.h

58c283819a1e87 Stephan M?ller 2020-01-09 233
58c283819a1e87 Stephan M?ller 2020-01-09 234 /* Unlock the DRNG */
58c283819a1e87 Stephan M?ller 2020-01-09 235 static __always_inline void lrng_drng_unlock(struct lrng_drng *drng,
58c283819a1e87 Stephan M?ller 2020-01-09 236 unsigned long *flags)
58c283819a1e87 Stephan M?ller 2020-01-09 237 {
58c283819a1e87 Stephan M?ller 2020-01-09 238 if (lrng_drng_is_atomic(drng))
58c283819a1e87 Stephan M?ller 2020-01-09 @239 spin_unlock_irqrestore(&drng->spin_lock, *flags);
58c283819a1e87 Stephan M?ller 2020-01-09 240 else
58c283819a1e87 Stephan M?ller 2020-01-09 241 mutex_unlock(&drng->lock);
58c283819a1e87 Stephan M?ller 2020-01-09 242 }
58c283819a1e87 Stephan M?ller 2020-01-09 243

:::::: The code at line 239 was first introduced by commit
:::::: 58c283819a1e879bc2e30d05720285f9709f7f6d Linux Random Number Generator

:::::: TO: Stephan M?ller <[email protected]>
:::::: CC: 0day robot <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

2020-01-12 10:21:43

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH v27 04/12] LRNG - add switchable DRNG support

Am Samstag, 11. Januar 2020, 08:09:50 CET schrieb kbuild test robot:

Hi,

> Hi "Stephan,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on char-misc/char-misc-testing]
> [also build test WARNING on cryptodev/master crypto/master v5.5-rc5
> next-20200110] [if your patch is applied to the wrong git tree, please drop
> us a note to help improve the system. BTW, we also suggest to use '--base'
> option to specify the base tree in git format-patch, please see
> https://stackoverflow.com/a/37406982]
>
> url:
> https://github.com/0day-ci/linux/commits/Stephan-M-ller/dev-random-a-new-ap
> proach-with-full-SP800-90B/20200110-084934 base:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> 68faa679b8be1a74e6663c21c3a9d25d32f1c079 reproduce:
> # apt-get install sparse
> # sparse version: v0.6.1-129-g341daf20-dirty
> make ARCH=x86_64 allmodconfig
> make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/char/lrng/lrng_internal.h:239:39: sparse: sparse: context
> >> imbalance in 'lrng_drng_switch' - unexpected unlock
> vim +/lrng_drng_switch +239 drivers/char/lrng/lrng_internal.h
>
> 58c283819a1e87 Stephan M?ller 2020-01-09 233
> 58c283819a1e87 Stephan M?ller 2020-01-09 234 /* Unlock the DRNG */
> 58c283819a1e87 Stephan M?ller 2020-01-09 235 static __always_inline void
> lrng_drng_unlock(struct lrng_drng *drng, 58c283819a1e87 Stephan M?ller
> 2020-01-09 236 unsigned long *flags)
58c283819a1e87 Stephan
> M?ller 2020-01-09 237 {
> 58c283819a1e87 Stephan M?ller 2020-01-09 238 if
> (lrng_drng_is_atomic(drng)) 58c283819a1e87 Stephan M?ller 2020-01-09 @239
> spin_unlock_irqrestore(&drng->spin_lock, *flags); 58c283819a1e87
Stephan
> M?ller 2020-01-09 240 else
> 58c283819a1e87 Stephan M?ller 2020-01-09 241 mutex_unlock(&drng-
>lock);
> 58c283819a1e87 Stephan M?ller 2020-01-09 242 }
> 58c283819a1e87 Stephan M?ller 2020-01-09 243
>
> :::::: The code at line 239 was first introduced by commit
> :::::: 58c283819a1e879bc2e30d05720285f9709f7f6d Linux Random Number
> :::::: Generator
> ::::::
> :::::: TO: Stephan M?ller <[email protected]>
> :::::: CC: 0day robot <[email protected]>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology
> Center https://lists.01.org/hyperkitty/list/[email protected] Intel
> Corporation

After analyzing the issue a bit more, it seems that I have to remove
"unlikely" from lrng_drng_lock which seems to cause additional grief
with sparse. Note, sparse will still report a lock context imbalance as it
used to since we indeed have two lock context as documented in
lrng_drng_switch.

Ciao
Stephan