* Re: [PATCH v25 03/12] LRNG - /proc interface
From: Stephan Müller @ 2019-11-17 11:37 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
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
In-Reply-To: <87k17z4cna.fsf@x220.int.ebiederm.org>
Am Sonntag, 17. November 2019, 00:36:25 CET schrieb Eric W. Biederman:
Hi Eric,
> Stephan Müller <smueller@chronox.de> writes:
> > The LRNG /proc interface provides the same files as the legacy
> > /dev/random. These files behave identically. Yet, all files are
> > documented at [1].
>
> For someone who works in this area a lot this description is confusing.
>
> You are talking about sysctls not ordinary proc files.
Agreed and I will change the description accordingly.
>
> You don't have a call register_sysctl. If you want your own
> implementation of these sysctls that would seem to be the way to get
> them. Teach each implementation to register their own set of sysctls
> if they are enabled.
Agreed, I will do that.
>
> The entire structure of the code you are adding I have quite confusing,
> and a bit messing.
>
> Why add a declaration of random_table in patch 1 and then delete that
> declaration in patch 3? Nothing uses that declaration until this point.
This is only to ensure that patch 1 compiles. Without it, there would be a
dangling reference that is required by static struct ctl_table kern_table[].
As I was always under the impression that each patch should compile by itself
to support bisect, I added that empty declaration. Yet, patch 1 is never
intended to live without patch 3. I only split patch 3 up is to aid code
review by having as many individual patches as possible while still allowing
them to compile.
>
> What is the point of adding an extern declaration just before you
> declare the table itself? As I understand the C language that achieves
> nothing. I understand that is what the existing code in
> drivers/char/random.c does but that is equally buggy there.
I totally agree and I was wondering here as well. But I simply was taking it
as is.
I am happy to clean this one up.
>
> I also don't understand why you don't modify the existing random
> generator code into the form you want? What is the point of a
> side-by-side replacement? Especially since only one of them can
> be compiled into the kernel at the same time?
I was developing small patches for random.c since 2013, mostly cleanup
patches. Unfortunately most of them were silently ignored. Some others were
silently taken but appeared two or three kernel releases later.
Getting more architecture-invasive patches into the existing random.c code is
considered to be quite a problem considering this experience.
Besides, the LRNG has quite a different architecture compared to the random.c.
As the RNG is an important aspect of the kernel, I did not feel bold enough to
simply replace the existing code which implies that there is no fallback. By
allowing a side-by-side code base which is even deactivated by default, it
allows other researchers to analyze the mathematical aspects beyond the pure
code while still having an implementation that provides a known and analyzed
entropy source.
Also, considering other kernel components like memory allocators, I/O
schedulers or even file systems, providing different architectures covering
similar problems (memory allocation, accessing a disk) with an entirely
different architecture and thus implementation seems to be an appropriate
solution.
Finally, I tried to keep code that has a similar functionality to the existing
random.c similar to allow a merge at a later stage. For example, the sysctls
are identical, but internally use different variables.
>
> This build a replacement and then switch over seems like a recipe for
> loosing the lessons of history because you are not making incremental
> changes that can be clearly understood, reviewed and bisected.
>
> As I read your patchset until this change your code will fail to compile
> in an ordinary configuration with proc enabled. Have you even tested
> compiling your patchset one patch at a time?
Yes, it does compile with proc enabled with the warning that random_table is
considered to contain one element.
>
> For me a great reorganization to impelment a better structure that fails
> to have a good structure on the usual merits makes me dubious about the
> entire thing. As it can be a sign the author was pushing so hard to
> make things work he stopped looking at problematic details.
>
> Dubious-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> Eric
>
> > +
> > +extern struct ctl_table random_table[];
> > +struct ctl_table random_table[] = {
> > + {
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 03/12] LRNG - /proc interface
From: Stephan Müller @ 2019-11-17 12:16 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
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
In-Reply-To: <4EB89769-7A2C-4A03-A832-9A0539DD3336@amacapital.net>
Am Samstag, 16. November 2019, 17:39:40 CET schrieb Andy Lutomirski:
Hi Andy,
> > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de> wrote:
> >
> > The LRNG /proc interface provides the same files as the legacy
> > /dev/random. These files behave identically. Yet, all files are
> > documented at [1].
>
> Why?
I am not sure here: are you referring to the documentation? Or the one
additional file?
If it is the documentation, do you want me to add it to the patch description?
I initially did not add it as these files were present and seemingly known
what they provide. But I would add that documentation to the patch description
if this is desired.
If it is the additional file, should I move it to another place like a sysfs
interface?
Thank you.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 1/3] clone.2: Fix typos
From: Michael Kerrisk (man-pages) @ 2019-11-17 18:00 UTC (permalink / raw)
To: Christian Brauner
Cc: mtk.manpages, adrian, akpm, arnd, avagin, christian.brauner,
dhowells, fweimer, jannh, keescook, linux-api, linux-kernel,
linux-man, mingo, oleg, xemul
In-Reply-To: <20191116114114.7066-1-christian@brauner.io>
On 11/16/19 12:41 PM, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
Thanks, Christian,
Patch applied.
Cheers,
Michael
> Fix two spelling mistakes in manpage describing the clone{2,3}()
> syscalls/syscall wrappers.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> man2/clone.2 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man2/clone.2 b/man2/clone.2
> index f67a60067..57a9eaba7 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -70,12 +70,12 @@ create a new ("child") process, in a manner similar to
> .PP
> By contrast with
> .BR fork (2),
> -these system cals provide more precise control over what pieces of execution
> +these system calls provide more precise control over what pieces of execution
> context are shared between the calling process and the child process.
> For example, using these system calls, the caller can control whether
> or not the two processes share the virtual address space,
> the table of file descriptors, and the table of signal handlers.
> -These system calls also allow the new child process to placed
> +These system calls also allow the new child process to be placed
> in separate
> .BR namespaces (7).
> .PP
>
> base-commit: 91243dad42a7a62df73e3b1dfbb810adc1b8b654
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH 2/3] clone.2: Check for MAP_FAILED not NULL on mmap()
From: Michael Kerrisk (man-pages) @ 2019-11-17 18:01 UTC (permalink / raw)
To: Christian Brauner
Cc: mtk.manpages, adrian, akpm, arnd, avagin, christian.brauner,
dhowells, fweimer, jannh, keescook, linux-api, linux-kernel,
linux-man, mingo, oleg, xemul
In-Reply-To: <20191116114114.7066-2-christian@brauner.io>
On 11/16/19 12:41 PM, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> If mmap() fails it will return MAP_FAILED which according to the manpage
> is (void *)-1 not NULL.
Sigh! Bad editing on my part.
Thanks, Christian.
Patch applied.
Cheers,
Michael
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> man2/clone.2 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man2/clone.2 b/man2/clone.2
> index 57a9eaba7..faff2ada6 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -1628,7 +1628,7 @@ main(int argc, char *argv[])
>
> stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, \-1, 0);
> - if (stack == NULL)
> + if (stack == MAP_FAILED)
> errExit("mmap");
>
> stackTop = stack + STACK_SIZE; /* Assume stack grows downward */
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH 3/3] clone.2: Use pid_t for clone3() {child,parent}_tid
From: Michael Kerrisk (man-pages) @ 2019-11-17 18:02 UTC (permalink / raw)
To: Christian Brauner
Cc: mtk.manpages, adrian, akpm, arnd, avagin, christian.brauner,
dhowells, fweimer, jannh, keescook, linux-api, linux-kernel,
linux-man, mingo, oleg, xemul
In-Reply-To: <20191116114114.7066-3-christian@brauner.io>
On 11/16/19 12:41 PM, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Advertise to userspace that they should use proper pid_t types for
> arguments returning a pid.
> The kernel-internal struct kernel_clone_args currently uses int as type
> and since POSIX mandates that pid_t is a signed integer type and glibc
> and friends use int this is not an issue. After the merge window for
> v5.5 closes we can switch struct kernel_clone_args over to using pid_t
> as well without any danger in regressing current userspace.
> Also note, that the new set tid feature which will be merged for v5.5
> uses pid_t types as well.
Thanks taking a further pass throuh this page, Christian,
Patch applied.
Cheers,
Michael
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> man2/clone.2 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man2/clone.2 b/man2/clone.2
> index faff2ada6..bf2d7c731 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -183,9 +183,9 @@ is a structure of the following form:
> struct clone_args {
> u64 flags; /* Flags bit mask */
> u64 pidfd; /* Where to store PID file descriptor
> - (\fIint *\fP) */
> + (\fIpid_t *\fP) */
> u64 child_tid; /* Where to store child TID,
> - in child's memory (\fIint *\fP) */
> + in child's memory (\fIpid_t *\fP) */
> u64 parent_tid; /* Where to store child TID,
> in parent's memory (\fIint *\fP) */
> u64 exit_signal; /* Signal to deliver to parent on
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH v25 12/12] LRNG - add interface for gathering of raw entropy
From: Stephan Müller @ 2019-11-17 22:55 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
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
In-Reply-To: <6950B235-6231-4DFF-A375-54A70C548B2E@amacapital.net>
Am Samstag, 16. November 2019, 17:51:24 CET schrieb Andy Lutomirski:
Hi Andy,
> > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de> wrote:
> >
> > The test interface allows a privileged process to capture the raw
> > unconditioned noise that is collected by the LRNG for statistical
> > analysis. Extracted noise data is not used to seed the LRNG. This
> > is a test interface and not appropriate for production systems.
> > Yet, the interface is considered to be sufficiently secured for
> > production systems.
> >
> > Access to the data is given through the lrng_raw debugfs file. The
> > data buffer should be multiples of sizeof(u32) to fill the entire
> > buffer. Using the option lrng_testing.boot_test=1 the raw noise of
> > the first 1000 entropy events since boot can be sampled.
> >
> > This test interface allows generating the data required for
> > analysis whether the LRNG is in compliance with SP800-90B
> > sections 3.1.3 and 3.1.4.
> >
> > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > CC: Willy Tarreau <w@1wt.eu>
> > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > CC: Vito Caputo <vcaputo@pengaru.com>
> > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > CC: Jan Kara <jack@suse.cz>
> > CC: Ray Strode <rstrode@redhat.com>
> > CC: William Jon McCann <mccann@jhu.edu>
> > CC: zhangjs <zachary@baishancloud.com>
> > CC: Andy Lutomirski <luto@kernel.org>
> > CC: Florian Weimer <fweimer@redhat.com>
> > CC: Lennart Poettering <mzxreary@0pointer.de>
> > CC: Nicolai Stange <nstange@suse.de>
> > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > Tested-by: Neil Horman <nhorman@redhat.com>
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > drivers/char/lrng/Kconfig | 16 ++
> > drivers/char/lrng/Makefile | 1 +
> > drivers/char/lrng/lrng_testing.c | 324 +++++++++++++++++++++++++++++++
> > 3 files changed, 341 insertions(+)
> > create mode 100644 drivers/char/lrng/lrng_testing.c
> >
> > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > index e6ca3acc1e48..4ccc710832ef 100644
> > --- a/drivers/char/lrng/Kconfig
> > +++ b/drivers/char/lrng/Kconfig
> > @@ -169,4 +169,20 @@ config LRNG_APT_CUTOFF
> >
> > default 325 if !LRNG_APT_BROKEN
> > default 32 if LRNG_APT_BROKEN
> >
> > +config LRNG_TESTING
> > + bool "Enable entropy test interface to LRNG noise source"
> > + select CONFIG_DEBUG_FS
> > + help
> > + The test interface allows a privileged process to capture
> > + the raw unconditioned noise that is collected by the LRNG
> > + for statistical analysis. Extracted noise data is not used
> > + to seed the LRNG.
> > +
> > + The raw noise data can be obtained using the lrng_raw
> > + debugfs file. Using the option lrng_testing.boot_test=1
> > + the raw noise of the first 1000 entropy events since boot
> > + can be sampled.
> > +
> > + If unsure, say N.
> > +
> > endif # LRNG
> > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
> > index 0713e9c0aa6e..c0b6cc4301fe 100644
> > --- a/drivers/char/lrng/Makefile
> > +++ b/drivers/char/lrng/Makefile
> > @@ -16,3 +16,4 @@ obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
> > obj-$(CONFIG_LRNG_JENT) += lrng_jent.o
> > obj-$(CONFIG_LRNG_TRNG_SUPPORT) += lrng_trng.o
> > obj-$(CONFIG_LRNG_HEALTH_TESTS) += lrng_health.o
> > +obj-$(CONFIG_LRNG_TESTING) += lrng_testing.o
> > diff --git a/drivers/char/lrng/lrng_testing.c
> > b/drivers/char/lrng/lrng_testing.c new file mode 100644
> > index 000000000000..5c33d3bd2172
> > --- /dev/null
> > +++ b/drivers/char/lrng/lrng_testing.c
> > @@ -0,0 +1,324 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > +/*
> > + * Linux Random Number Generator (LRNG) Raw entropy collection tool
> > + *
> > + * Copyright (C) 2019, Stephan Mueller <smueller@chronox.de>
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/atomic.h>
> > +#include <linux/bug.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/sched/signal.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/workqueue.h>
> > +#include <asm/errno.h>
> > +
> > +#include "lrng_internal.h"
> > +
> > +#define LRNG_TESTING_RINGBUFFER_SIZE 1024
> > +#define LRNG_TESTING_RINGBUFFER_MASK (LRNG_TESTING_RINGBUFFER_SIZE -
> > 1)
> > +
> > +static u32 lrng_testing_rb[LRNG_TESTING_RINGBUFFER_SIZE];
> > +static atomic_t lrng_rb_reader = ATOMIC_INIT(0);
> > +static atomic_t lrng_rb_writer = ATOMIC_INIT(0);
> > +static atomic_t lrng_rb_first_in = ATOMIC_INIT(0);
> > +static atomic_t lrng_testing_enabled = ATOMIC_INIT(0);
> > +
> > +static DECLARE_WAIT_QUEUE_HEAD(lrng_raw_read_wait);
> > +
> > +static u32 boot_test = 0;
> > +module_param(boot_test, uint, 0644);
> > +MODULE_PARM_DESC(boot_test, "Enable gathering boot time entropy of the
> > first" + " entropy events");
> > +
> > +static inline void lrng_raw_entropy_reset(void)
> > +{
> > + atomic_set(&lrng_rb_reader, 0);
> > + atomic_set(&lrng_rb_writer, 0);
> > + atomic_set(&lrng_rb_first_in, 0);
> > +}
> > +
> > +static void lrng_raw_entropy_init(void)
> > +{
> > + /*
> > + * The boot time testing implies we have a running test. If the
> > + * caller wants to clear it, he has to unset the boot_test flag
> > + * at runtime via sysfs to enable regular runtime testing
> > + */
> > + if (boot_test)
> > + return;
> > +
> > + lrng_raw_entropy_reset();
> > + atomic_set(&lrng_testing_enabled, 1);
> > + pr_warn("Enabling raw entropy collection\n");
> > +}
> > +
> > +static void lrng_raw_entropy_fini(void)
> > +{
> > + if (boot_test)
> > + return;
> > +
> > + lrng_raw_entropy_reset();
> > + atomic_set(&lrng_testing_enabled, 0);
> > + pr_warn("Disabling raw entropy collection\n");
> > +}
> > +
> > +bool lrng_raw_entropy_store(u32 value)
> > +{
> > + unsigned int write_ptr;
> > + unsigned int read_ptr;
> > +
> > + if (!atomic_read(&lrng_testing_enabled) && !boot_test)
> > + return false;
> > +
> > + write_ptr = (unsigned int)atomic_add_return_relaxed(1,
> > &lrng_rb_writer); + read_ptr = (unsigned
> > int)atomic_read(&lrng_rb_reader);
Before answering your comments, please allow me to clarify the following:
This entire code is intended to obtain take raw unconditioned noise data that
needs to be extracted from the kernel to user space to allow it to be further
analyzed. This is also why it is mentioned in the Kconfig selection that in
doubt one should select N and that this code is not intended for production
kernels although this code should be secure enough to be present in production
kernels.
For example, raw unconditioned noise data needs to be processed by the
complicated math outlined in chapter 6 of [1]. For that, there is a tool
available, see [2]. For that tool, data is needed that is obtained with the
getrawentropy tool available with [3] where this tool obtains the data from
the SysFS file that is implemented with this C file.
In addition, [1] even needs the data from the very first 1000 interrupts after
boot. Hence, the LRNG needs to be able to store that data until user space can
pick it up (see the boot_test variable).
The assessment resulting from this can be reviewed at [4] section 3.2 In
particular, the numbers provided at the end of sections 3.2.3 and 3.2.4 are
obtained with this interface.
Other examples where such raw unconditioned noise data is needed for further
analysis is [5], especially chapter 6.
This testing has nothing to do with the runtime testing provided with the
patch set 11. All data that ends up here is not available to the LRNG and will
not contribute to any entropy collection.
See the following:
static inline void lrng_time_process(void)
{
...
if (lrng_raw_entropy_store(now_time))
return;
bool lrng_raw_entropy_store(u32 value)
{
...
if (!atomic_read(&lrng_testing_enabled) && !boot_test)
return false;
...
return true;
>
> Am I correct in assuming that this function can be called concurrently in
> different threads or CPUs?
Yes, because it is called indirectly by add_interrupt_randomness.
> > +
> > + /*
> > + * Disable entropy testing for boot time testing after ring buffer
> > + * is filled.
> > + */
> > + if (boot_test && write_ptr > LRNG_TESTING_RINGBUFFER_SIZE) {
> > + pr_warn_once("Boot time entropy collection test disabled\n");
> > + return false;
> > + }
> > +
> > + if (boot_test && !atomic_read(&lrng_rb_first_in))
> > + pr_warn("Boot time entropy collection test enabled\n");
> > +
> > + lrng_testing_rb[write_ptr & LRNG_TESTING_RINGBUFFER_MASK] = value;
>
> You’re writing *somewhere*, but not necessarily to the first open slot.
The idea is that there is a reader pointer and a writer pointer where the
reader always must be smaller or equal to the writer (modulo the size of the
ring buffer). So, I do not care where the writer ptr is.
All I need is that:
1. reader and writer ptr must start with the same value at boot time (e.g. 0)
2. reader ptr is always <= writer ptr in order for data to be read.
With these two conditions, when pulling data from the buffer, I need to pull
always the data from the reader ptr until the reader ptr reaches the writer
pointer.
Note, the reader/writer pointers are always set to 0 at the beginning of a new
read request from user space.
>
> > +
> > + /* We got at least one event, enable the reader now. */
> > + atomic_set(&lrng_rb_first_in, 1);
>
> But not necessarily in position 0.
Yes, this is perfectly ok.
>
> > +
> > + if (wq_has_sleeper(&lrng_raw_read_wait))
> > + wake_up_interruptible(&lrng_raw_read_wait);
> > +
> > + /*
> > + * Our writer is taking over the reader - this means the reader
> > + * one full ring buffer available. Thus we "push" the reader ahead
> > + * to guarantee that he will be able to consume the full ring.
> > + */
> > + if (!boot_test &&
> > + ((write_ptr & LRNG_TESTING_RINGBUFFER_MASK) ==
> > + (read_ptr & LRNG_TESTING_RINGBUFFER_MASK)))
> > + atomic_inc_return_relaxed(&lrng_rb_reader);
>
> Because you did a relaxed increment above, you don’t actually know this.
> Maybe it’s okay, but this is way too subtle.
You are absolutely correct, there should be no relaxed atomic operation. We
should take the atomic_inc above and here.
I fixed this.
>
> I think you should have a mutex for the read side and put all the
> complicated accounting inside the mutex. If the reader can’t figure out
> that the read pointer is too far behind the write pointer, then fix the
> reader.
Done - the writer now only writes the data and generates the boot log if the
boot time raw entropy gathering is enabled.
>
> I also don’t see how the reader is supposed to know how much data has
> actually been written. You don’t have any variable that says “all words up
> to X have been written”.
With the two rules above, I think the reader knows that: all data between the
reader ptr and the writer ptr modulo the size of the ring buffer.
But I simplified the code now, the code now only copies the data out if the
reader <= writer modulo the ring buffer size. In this case, if the writer is
much faster, then we loose some values.
With the old code, we simply would have lost it too, but just a bit later.
>
> I think you should stop trying to make the write side wait free.
> Instead,
> consider either using a lock or making it unreliable. For the former, just
> skip taking the lock if testing is off. For the latter, read write_ptr,
> write (using WRITE_ONCE) your data, then cmpxchg the write ptr from the
> value you read to that value plus one. And make sure that the reader never
> tries to read the first unwritten slot, i.e. never let the reader catch all
> the way up.
I have followed the locking approach as we need to get correct data.
>
> I’m also curious why you need entirely different infrastructure for testing
> as for normal operation.
I hope with the explanation above, the question is answered.
> > +
> > + return true;
> > +}
> > +
> > +static inline bool lrng_raw_have_data(void)
> > +{
> > + unsigned int read_ptr = (unsigned int)atomic_read(&lrng_rb_reader);
> > + unsigned int write_ptr = (unsigned int)atomic_read(&lrng_rb_writer);
> > +
> > + return (atomic_read(&lrng_rb_first_in) &&
> > + (write_ptr & LRNG_TESTING_RINGBUFFER_MASK) !=
> > + (read_ptr & LRNG_TESTING_RINGBUFFER_MASK));
> > +}
> > +
> > +static int lrng_raw_entropy_reader(u8 *outbuf, u32 outbuflen)
> > +{
> > + int collected_data = 0;
> > +
> > + if (!atomic_read(&lrng_testing_enabled) && !boot_test)
> > + return -EAGAIN;
> > +
> > + if (!atomic_read(&lrng_rb_first_in)) {
> > + wait_event_interruptible(lrng_raw_read_wait,
> > + lrng_raw_have_data());
> > + if (signal_pending(current))
> > + return -ERESTARTSYS;
> > + }
> > +
> > + while (outbuflen) {
> > + unsigned int read_ptr =
> > + (unsigned int)atomic_add_return_relaxed(
> > + 1, &lrng_rb_reader);
> > + unsigned int write_ptr =
> > + (unsigned int)atomic_read(&lrng_rb_writer);
> > +
> > + /*
> > + * For boot time testing, only output one round of ring buffer.
> > + */
> > + if (boot_test && read_ptr > LRNG_TESTING_RINGBUFFER_SIZE) {
> > + collected_data = -ENOMSG;
> > + goto out;
> > + }
> > +
> > + /* We reached the writer */
> > + if (!boot_test && ((write_ptr & LRNG_TESTING_RINGBUFFER_MASK) ==
> > + (read_ptr & LRNG_TESTING_RINGBUFFER_MASK))) {
> > +
>
> This is wrong. The fact that you haven’t reached the writer does not imply
> that you’re about to read valid data.
As I changed the code by using your locking suggestion. With that I think the
code should now always read correct data. I will send an updated patch set
tomorrow.
Thank you for your review.
[1] https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-90B.pdf
[2] https://github.com/usnistgov/SP800-90B_EntropyAssessment
[3] http://www.chronox.de/lrng/lrng-tests-20191116.tar.xz - see the sp80090b
directory for details
[4] http://www.chronox.de/lrng/doc/lrng.pdf
[5] https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/
LinuxRNG/LinuxRNG_EN.pdf
Ciao
Stephan
^ permalink raw reply
* Re: [LKP] Re: [pipe] d60337eff1: BUG:kernel_NULL_pointer_dereference,address
From: kernel test robot @ 2019-11-18 7:53 UTC (permalink / raw)
To: David Howells, kernel test robot
Cc: torvalds, Rasmus Villemoes, Greg Kroah-Hartman, Peter Zijlstra,
nicolas.dichtel, raven, Christian Brauner, keyrings, linux-usb,
linux-block, linux-security-module, linux-fsdevel, linux-api,
linux-kernel, lkp
In-Reply-To: <6853.1573834946@warthog.procyon.org.uk>
Hi David,
Yes, it can fix the problem.
Best Regards,
Rong Chen
On 11/16/2019 12:22 AM, David Howells wrote:
> Actually, no, this is the fix:
>
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 7006b5b2106d..be2fc5793ddd 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -537,7 +537,7 @@ static size_t push_pipe(struct iov_iter *i, size_t size,
> buf->ops = &default_pipe_buf_ops;
> buf->page = page;
> buf->offset = 0;
> - buf->len = max_t(ssize_t, left, PAGE_SIZE);
> + buf->len = min_t(ssize_t, left, PAGE_SIZE);
> left -= buf->len;
> iter_head++;
> pipe->head = iter_head;
>
> David
> _______________________________________________
> LKP mailing list -- lkp@lists.01.org
> To unsubscribe send an email to lkp-leave@lists.01.org
^ permalink raw reply
* [RESEND RFC PATCH 0/1] CAP_SYS_NICE inside user namespace
From: Prakash Sangappa @ 2019-11-18 17:01 UTC (permalink / raw)
To: linux-kernel, linux-api; +Cc: ebiederm, tglx, peterz, serge, prakash.sangappa
Some of the capabilities(7) which affect system wide resources, are ineffective
inside user namespaces. This restriction applies even to root user( uid 0)
from init namespace mapped into the user namespace. One such capability
is CAP_SYS_NICE which is required to change process priority. As a result of
which the root user cannot perform operations like increase a process priority
using -ve nice value or set RT priority on processes inside the user namespace.
A workaround to deal with this restriction is to use the help of a process /
daemon running outside the user namespace to change process priority, which is
a an inconvenience.
We could allow these restricted capabilities to take effect only for the root
user from init namespace mapped inside a user namespace and limit the effect
with use of cgroups. It would seem reasonable to deal with each of these
restricted capabilities on a case by case basis and address them. This patch
is concerning CAP_SYS_NICE capability. The proposal here is to selectively
allow CAP_SYS_NICE to take effect inside user namespace only for a root user
mapped from init name space.
Which user id gets to map the root user(uid 0) from init namespace inside its
user namespaces is authorized thru /etc/subuid & /etc/subgid entries. Only
system admin / root user on the system can add these entries.
Therefore any ordinary user cannot simply map the root user(uid 0) into
user namespaces created. Necessary cgroup bandwidth control can be used
to limit cpu usage for such user namespaces.
The capabilities(7) manpage lists all the operations / system calls that are
subject to CAP_SYS_NICE capability check. This patch currently allows
CAP_SYS_NICE to take effect inside a user namespace only for system calls
affecting process priority. For completeness sake should memory
operations(migrate_pages(2), move_pages(2), mbind(2)) mentioned in the
manpage, also be permitted? There are no cgroup controls to limit the effect
of these memory operations.
Looking for feedback on this approach.
Prakash Sangappa (1):
Selectively allow CAP_SYS_NICE capability inside user namespaces
kernel/sched/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply
* [RESEND RFC PATCH 1/1] Selectively allow CAP_SYS_NICE capability inside user namespaces
From: Prakash Sangappa @ 2019-11-18 17:01 UTC (permalink / raw)
To: linux-kernel, linux-api; +Cc: ebiederm, tglx, peterz, serge, prakash.sangappa
In-Reply-To: <1574096478-11520-1-git-send-email-prakash.sangappa@oracle.com>
Allow CAP_SYS_NICE to take effect for processes having effective uid of a
root user from init namespace.
Signed-off-by: Prakash Sangappa <prakash.sangappa@oracle.com>
---
kernel/sched/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7880f4f..628bd46 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4548,6 +4548,8 @@ int can_nice(const struct task_struct *p, const int nice)
int nice_rlim = nice_to_rlimit(nice);
return (nice_rlim <= task_rlimit(p, RLIMIT_NICE) ||
+ (ns_capable(__task_cred(p)->user_ns, CAP_SYS_NICE) &&
+ uid_eq(current_euid(), GLOBAL_ROOT_UID)) ||
capable(CAP_SYS_NICE));
}
@@ -4784,7 +4786,9 @@ static int __sched_setscheduler(struct task_struct *p,
/*
* Allow unprivileged RT tasks to decrease priority:
*/
- if (user && !capable(CAP_SYS_NICE)) {
+ if (user && !(ns_capable(__task_cred(p)->user_ns, CAP_SYS_NICE) &&
+ uid_eq(current_euid(), GLOBAL_ROOT_UID)) &&
+ !capable(CAP_SYS_NICE)) {
if (fair_policy(policy)) {
if (attr->sched_nice < task_nice(p) &&
!can_nice(p, attr->sched_nice))
--
2.7.4
^ permalink raw reply related
* Re: [RESEND RFC PATCH 1/1] Selectively allow CAP_SYS_NICE capability inside user namespaces
From: Jann Horn @ 2019-11-18 19:30 UTC (permalink / raw)
To: Prakash Sangappa
Cc: kernel list, Linux API, Eric W. Biederman, Thomas Gleixner,
Peter Zijlstra, Serge E. Hallyn, Christian Brauner
In-Reply-To: <1574096478-11520-2-git-send-email-prakash.sangappa@oracle.com>
On Mon, Nov 18, 2019 at 6:04 PM Prakash Sangappa
<prakash.sangappa@oracle.com> wrote:
> Allow CAP_SYS_NICE to take effect for processes having effective uid of a
> root user from init namespace.
[...]
> @@ -4548,6 +4548,8 @@ int can_nice(const struct task_struct *p, const int nice)
> int nice_rlim = nice_to_rlimit(nice);
>
> return (nice_rlim <= task_rlimit(p, RLIMIT_NICE) ||
> + (ns_capable(__task_cred(p)->user_ns, CAP_SYS_NICE) &&
> + uid_eq(current_euid(), GLOBAL_ROOT_UID)) ||
> capable(CAP_SYS_NICE));
I very strongly dislike tying such a feature to GLOBAL_ROOT_UID.
Wouldn't it be better to control this through procfs, similar to
uid_map and gid_map? If you really need an escape hatch to become
privileged outside a user namespace, then I'd much prefer a file
"cap_map" that lets someone with appropriate capabilities in the outer
namespace write a bitmask of capabilities that should have effect
outside the container, or something like that. And limit that to bits
where that's sane, like CAP_SYS_NICE.
If we tie features like this to GLOBAL_ROOT_UID, more people are going
to run their containers with GLOBAL_ROOT_UID. Which is a terrible,
terrible idea. GLOBAL_ROOT_UID gives you privilege over all sorts of
files that you shouldn't be able to access, and only things like mount
namespaces and possibly LSMs prevent you from exercising that
privilege. GLOBAL_ROOT_UID should only ever be given to processes that
you trust completely.
^ permalink raw reply
* Re: [RESEND RFC PATCH 0/1] CAP_SYS_NICE inside user namespace
From: Jann Horn @ 2019-11-18 19:36 UTC (permalink / raw)
To: Prakash Sangappa
Cc: kernel list, Linux API, Eric W. Biederman, Thomas Gleixner,
Peter Zijlstra, Serge E. Hallyn, Christian Brauner
In-Reply-To: <1574096478-11520-1-git-send-email-prakash.sangappa@oracle.com>
On Mon, Nov 18, 2019 at 6:04 PM Prakash Sangappa
<prakash.sangappa@oracle.com> wrote:
> Some of the capabilities(7) which affect system wide resources, are ineffective
> inside user namespaces. This restriction applies even to root user( uid 0)
> from init namespace mapped into the user namespace. One such capability
> is CAP_SYS_NICE which is required to change process priority. As a result of
> which the root user cannot perform operations like increase a process priority
> using -ve nice value or set RT priority on processes inside the user namespace.
> A workaround to deal with this restriction is to use the help of a process /
> daemon running outside the user namespace to change process priority, which is
> a an inconvenience.
What is the goal here, in the big picture? Is your goal to allow
container admins to control the priorities of their tasks *relative to
each other*, or do you actually explicitly want container A to be able
to decide that its current workload is more timing-sensitive than
container B's?
^ permalink raw reply
* Re: [RESEND RFC PATCH 0/1] CAP_SYS_NICE inside user namespace
From: Prakash Sangappa @ 2019-11-18 20:34 UTC (permalink / raw)
To: Jann Horn
Cc: kernel list, Linux API, Eric W. Biederman, Thomas Gleixner,
Peter Zijlstra, Serge E. Hallyn, Christian Brauner
In-Reply-To: <CAG48ez3HfUx2aRvqR_bWnGoTshrHnUzxUNt7K6Sv7cqtPDWaWw@mail.gmail.com>
On 11/18/19 11:36 AM, Jann Horn wrote:
> On Mon, Nov 18, 2019 at 6:04 PM Prakash Sangappa
> <prakash.sangappa@oracle.com> wrote:
>> Some of the capabilities(7) which affect system wide resources, are ineffective
>> inside user namespaces. This restriction applies even to root user( uid 0)
>> from init namespace mapped into the user namespace. One such capability
>> is CAP_SYS_NICE which is required to change process priority. As a result of
>> which the root user cannot perform operations like increase a process priority
>> using -ve nice value or set RT priority on processes inside the user namespace.
>> A workaround to deal with this restriction is to use the help of a process /
>> daemon running outside the user namespace to change process priority, which is
>> a an inconvenience.
> What is the goal here, in the big picture? Is your goal to allow
> container admins to control the priorities of their tasks *relative to
> each other*, or do you actually explicitly want container A to be able
> to decide that its current workload is more timing-sensitive than
> container B's?
It is more the latter. Admin should be able to explicitly decide that
container A
workload is to be given priority over other containers.
^ permalink raw reply
* Re: [RESEND RFC PATCH 1/1] Selectively allow CAP_SYS_NICE capability inside user namespaces
From: prakash.sangappa @ 2019-11-19 0:46 UTC (permalink / raw)
To: Jann Horn
Cc: kernel list, Linux API, Eric W. Biederman, Thomas Gleixner,
Peter Zijlstra, Serge E. Hallyn, Christian Brauner
In-Reply-To: <CAG48ez1i9LUd2DPoP9na9OyqXDZVRJZQqBbS1H6Dz_h7mtJ=Mw@mail.gmail.com>
On 11/18/2019 11:30 AM, Jann Horn wrote:
> On Mon, Nov 18, 2019 at 6:04 PM Prakash Sangappa
> <prakash.sangappa@oracle.com> wrote:
>> Allow CAP_SYS_NICE to take effect for processes having effective uid of a
>> root user from init namespace.
> [...]
>> @@ -4548,6 +4548,8 @@ int can_nice(const struct task_struct *p, const int nice)
>> int nice_rlim = nice_to_rlimit(nice);
>>
>> return (nice_rlim <= task_rlimit(p, RLIMIT_NICE) ||
>> + (ns_capable(__task_cred(p)->user_ns, CAP_SYS_NICE) &&
>> + uid_eq(current_euid(), GLOBAL_ROOT_UID)) ||
>> capable(CAP_SYS_NICE));
> I very strongly dislike tying such a feature to GLOBAL_ROOT_UID.
> Wouldn't it be better to control this through procfs, similar to
> uid_map and gid_map? If you really need an escape hatch to become
> privileged outside a user namespace, then I'd much prefer a file
> "cap_map" that lets someone with appropriate capabilities in the outer
> namespace write a bitmask of capabilities that should have effect
> outside the container, or something like that. And limit that to bits
> where that's sane, like CAP_SYS_NICE.
Sounds reasonable. Adding a 'cap_map' file to user namespace, would give
more control. We could allow the capability in 'cap_map' to take effect
only if corresponding capability is enabled for the user inside the user
namespace Ex uid 0. Start with support for CAP_SYS_NICE?
>
> If we tie features like this to GLOBAL_ROOT_UID, more people are going
> to run their containers with GLOBAL_ROOT_UID. Which is a terrible,
> terrible idea. GLOBAL_ROOT_UID gives you privilege over all sorts of
> files that you shouldn't be able to access, and only things like mount
> namespaces and possibly LSMs prevent you from exercising that
> privilege. GLOBAL_ROOT_UID should only ever be given to processes that
> you trust completely.
Agreed.
^ permalink raw reply
* Suggested Patch is not working for 22851 Bugzilla issue
From: Bala S @ 2019-11-19 5:07 UTC (permalink / raw)
To: mhocko; +Cc: linux-kernel, linux-api
Hi Mhocko,
https://sourceware.org/bugzilla/show_bug.cgi?id=22851
For the above issue, I have found the patch.
Patch link:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1561935.html
Only change i noticed is 'MAP_FIXED_NOREPLACE' is used instead of
'MAP_FIXED_SAFE'.
I ran test case on the following targets with this patch:
1. For X86-64, Still i could see the reported issue( 'libevil.so' just
runs ‘cat /etc/passwd')
2. For MIPS-64, i am not seeing the malicious file content as
reported. But ‘ldd’ could not found ‘libevil.so’.
root@qemumips64:~/LIN1019-1806# ldd ./main
linux-vdso.so.1 (0x000000fff1f20000)
libevil.so => not found
libc.so.6 => /lib/libc.so.6 (0x0000005e46f70000)
/lib/ld.so.1 (0x000000fff7888000)
I am not clear why this patch is not working for X86-64? But it is
working for MIPS-64 with some issue.
Please let me know, if anything is pending on this patch for the reported issue.
Thanks,
Bala
^ permalink raw reply
* Re: Suggested Patch is not working for 22851 Bugzilla issue
From: Michal Hocko @ 2019-11-19 9:57 UTC (permalink / raw)
To: Bala S; +Cc: linux-kernel, linux-api, Linus Torvalds, Kees Cook
In-Reply-To: <CAJKzgVtzD7ULwCDVRSLMCmGJNaMqvx+jVO619t3xuv2oiEsPMQ@mail.gmail.com>
Hi
let me add Kees Cook and Linus to the cc list. I didn't have much
time to study the bug report and cannot really comment on the security
aspect of it. But let me point out that a big part of
MAP_FIXED_NOREPLACE usage has been removed from the loader code just
recently because it has caused some regressions
http://lkml.kernel.org/r/20191005233227.GB25745@shell.armlinux.org.uk
b212921b13bd ("elf: don't use MAP_FIXED_NOREPLACE for elf executable mappings").
So you definitely want to look at the current Linus tree for your future
experiments.
On Tue 19-11-19 10:37:44, Bala S wrote:
> Hi Mhocko,
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=22851
> For the above issue, I have found the patch.
>
> Patch link:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1561935.html
>
> Only change i noticed is 'MAP_FIXED_NOREPLACE' is used instead of
> 'MAP_FIXED_SAFE'.
>
> I ran test case on the following targets with this patch:
>
> 1. For X86-64, Still i could see the reported issue( 'libevil.so' just
> runs ‘cat /etc/passwd')
>
> 2. For MIPS-64, i am not seeing the malicious file content as
> reported. But ‘ldd’ could not found ‘libevil.so’.
>
> root@qemumips64:~/LIN1019-1806# ldd ./main
> linux-vdso.so.1 (0x000000fff1f20000)
> libevil.so => not found
> libc.so.6 => /lib/libc.so.6 (0x0000005e46f70000)
> /lib/ld.so.1 (0x000000fff7888000)
>
> I am not clear why this patch is not working for X86-64? But it is
> working for MIPS-64 with some issue.
> Please let me know, if anything is pending on this patch for the reported issue.
>
> Thanks,
> Bala
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v25 12/12] LRNG - add interface for gathering of raw entropy
From: Andy Lutomirski @ 2019-11-19 10:04 UTC (permalink / raw)
To: Stephan Müller
Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Crypto Mailing List,
LKML, Linux API, 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
In-Reply-To: <15745039.MlzBmBdvSy@positron.chronox.de>
On Sun, Nov 17, 2019 at 2:55 PM Stephan Müller <smueller@chronox.de> wrote:
>
> Am Samstag, 16. November 2019, 17:51:24 CET schrieb Andy Lutomirski:
>
> Hi Andy,
>
> > > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de> wrote:
> > >
> > > The test interface allows a privileged process to capture the raw
> > > unconditioned noise that is collected by the LRNG for statistical
> > > analysis. Extracted noise data is not used to seed the LRNG. This
> > > is a test interface and not appropriate for production systems.
> > > Yet, the interface is considered to be sufficiently secured for
> > > production systems.
> > >
> > > Access to the data is given through the lrng_raw debugfs file. The
> > > data buffer should be multiples of sizeof(u32) to fill the entire
> > > buffer. Using the option lrng_testing.boot_test=1 the raw noise of
> > > the first 1000 entropy events since boot can be sampled.
> > >
> > > This test interface allows generating the data required for
> > > analysis whether the LRNG is in compliance with SP800-90B
> > > sections 3.1.3 and 3.1.4.
> > >
> > > CC: "Eric W. Biederman" <ebiederm@xmission.com>
> > > CC: "Alexander E. Patrakov" <patrakov@gmail.com>
> > > CC: "Ahmed S. Darwish" <darwish.07@gmail.com>
> > > CC: "Theodore Y. Ts'o" <tytso@mit.edu>
> > > CC: Willy Tarreau <w@1wt.eu>
> > > CC: Matthew Garrett <mjg59@srcf.ucam.org>
> > > CC: Vito Caputo <vcaputo@pengaru.com>
> > > CC: Andreas Dilger <adilger.kernel@dilger.ca>
> > > CC: Jan Kara <jack@suse.cz>
> > > CC: Ray Strode <rstrode@redhat.com>
> > > CC: William Jon McCann <mccann@jhu.edu>
> > > CC: zhangjs <zachary@baishancloud.com>
> > > CC: Andy Lutomirski <luto@kernel.org>
> > > CC: Florian Weimer <fweimer@redhat.com>
> > > CC: Lennart Poettering <mzxreary@0pointer.de>
> > > CC: Nicolai Stange <nstange@suse.de>
> > > Reviewed-by: Roman Drahtmueller <draht@schaltsekun.de>
> > > Tested-by: Roman Drahtmüller <draht@schaltsekun.de>
> > > Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> > > Tested-by: Neil Horman <nhorman@redhat.com>
> > > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > > ---
> > > drivers/char/lrng/Kconfig | 16 ++
> > > drivers/char/lrng/Makefile | 1 +
> > > drivers/char/lrng/lrng_testing.c | 324 +++++++++++++++++++++++++++++++
> > > 3 files changed, 341 insertions(+)
> > > create mode 100644 drivers/char/lrng/lrng_testing.c
> > >
> > > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > > index e6ca3acc1e48..4ccc710832ef 100644
> > > --- a/drivers/char/lrng/Kconfig
> > > +++ b/drivers/char/lrng/Kconfig
> > > @@ -169,4 +169,20 @@ config LRNG_APT_CUTOFF
> > >
> > > default 325 if !LRNG_APT_BROKEN
> > > default 32 if LRNG_APT_BROKEN
> > >
> > > +config LRNG_TESTING
> > > + bool "Enable entropy test interface to LRNG noise source"
> > > + select CONFIG_DEBUG_FS
> > > + help
> > > + The test interface allows a privileged process to capture
> > > + the raw unconditioned noise that is collected by the LRNG
> > > + for statistical analysis. Extracted noise data is not used
> > > + to seed the LRNG.
> > > +
> > > + The raw noise data can be obtained using the lrng_raw
> > > + debugfs file. Using the option lrng_testing.boot_test=1
> > > + the raw noise of the first 1000 entropy events since boot
> > > + can be sampled.
> > > +
> > > + If unsure, say N.
> > > +
> > > endif # LRNG
> > > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
> > > index 0713e9c0aa6e..c0b6cc4301fe 100644
> > > --- a/drivers/char/lrng/Makefile
> > > +++ b/drivers/char/lrng/Makefile
> > > @@ -16,3 +16,4 @@ obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
> > > obj-$(CONFIG_LRNG_JENT) += lrng_jent.o
> > > obj-$(CONFIG_LRNG_TRNG_SUPPORT) += lrng_trng.o
> > > obj-$(CONFIG_LRNG_HEALTH_TESTS) += lrng_health.o
> > > +obj-$(CONFIG_LRNG_TESTING) += lrng_testing.o
> > > diff --git a/drivers/char/lrng/lrng_testing.c
> > > b/drivers/char/lrng/lrng_testing.c new file mode 100644
> > > index 000000000000..5c33d3bd2172
> > > --- /dev/null
> > > +++ b/drivers/char/lrng/lrng_testing.c
> > > @@ -0,0 +1,324 @@
> > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > > +/*
> > > + * Linux Random Number Generator (LRNG) Raw entropy collection tool
> > > + *
> > > + * Copyright (C) 2019, Stephan Mueller <smueller@chronox.de>
> > > + */
> > > +
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include <linux/atomic.h>
> > > +#include <linux/bug.h>
> > > +#include <linux/debugfs.h>
> > > +#include <linux/module.h>
> > > +#include <linux/sched.h>
> > > +#include <linux/sched/signal.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > > +#include <linux/types.h>
> > > +#include <linux/uaccess.h>
> > > +#include <linux/workqueue.h>
> > > +#include <asm/errno.h>
> > > +
> > > +#include "lrng_internal.h"
> > > +
> > > +#define LRNG_TESTING_RINGBUFFER_SIZE 1024
> > > +#define LRNG_TESTING_RINGBUFFER_MASK (LRNG_TESTING_RINGBUFFER_SIZE -
> > > 1)
> > > +
> > > +static u32 lrng_testing_rb[LRNG_TESTING_RINGBUFFER_SIZE];
> > > +static atomic_t lrng_rb_reader = ATOMIC_INIT(0);
> > > +static atomic_t lrng_rb_writer = ATOMIC_INIT(0);
> > > +static atomic_t lrng_rb_first_in = ATOMIC_INIT(0);
> > > +static atomic_t lrng_testing_enabled = ATOMIC_INIT(0);
> > > +
> > > +static DECLARE_WAIT_QUEUE_HEAD(lrng_raw_read_wait);
> > > +
> > > +static u32 boot_test = 0;
> > > +module_param(boot_test, uint, 0644);
> > > +MODULE_PARM_DESC(boot_test, "Enable gathering boot time entropy of the
> > > first" + " entropy events");
> > > +
> > > +static inline void lrng_raw_entropy_reset(void)
> > > +{
> > > + atomic_set(&lrng_rb_reader, 0);
> > > + atomic_set(&lrng_rb_writer, 0);
> > > + atomic_set(&lrng_rb_first_in, 0);
> > > +}
> > > +
> > > +static void lrng_raw_entropy_init(void)
> > > +{
> > > + /*
> > > + * The boot time testing implies we have a running test. If the
> > > + * caller wants to clear it, he has to unset the boot_test flag
> > > + * at runtime via sysfs to enable regular runtime testing
> > > + */
> > > + if (boot_test)
> > > + return;
> > > +
> > > + lrng_raw_entropy_reset();
> > > + atomic_set(&lrng_testing_enabled, 1);
> > > + pr_warn("Enabling raw entropy collection\n");
> > > +}
> > > +
> > > +static void lrng_raw_entropy_fini(void)
> > > +{
> > > + if (boot_test)
> > > + return;
> > > +
> > > + lrng_raw_entropy_reset();
> > > + atomic_set(&lrng_testing_enabled, 0);
> > > + pr_warn("Disabling raw entropy collection\n");
> > > +}
> > > +
> > > +bool lrng_raw_entropy_store(u32 value)
> > > +{
> > > + unsigned int write_ptr;
> > > + unsigned int read_ptr;
> > > +
> > > + if (!atomic_read(&lrng_testing_enabled) && !boot_test)
> > > + return false;
> > > +
> > > + write_ptr = (unsigned int)atomic_add_return_relaxed(1,
> > > &lrng_rb_writer); + read_ptr = (unsigned
> > > int)atomic_read(&lrng_rb_reader);
>
> Before answering your comments, please allow me to clarify the following:
>
> This entire code is intended to obtain take raw unconditioned noise data that
> needs to be extracted from the kernel to user space to allow it to be further
> analyzed. This is also why it is mentioned in the Kconfig selection that in
> doubt one should select N and that this code is not intended for production
> kernels although this code should be secure enough to be present in production
> kernels.
>
> For example, raw unconditioned noise data needs to be processed by the
> complicated math outlined in chapter 6 of [1]. For that, there is a tool
> available, see [2]. For that tool, data is needed that is obtained with the
> getrawentropy tool available with [3] where this tool obtains the data from
> the SysFS file that is implemented with this C file.
>
> In addition, [1] even needs the data from the very first 1000 interrupts after
> boot. Hence, the LRNG needs to be able to store that data until user space can
> pick it up (see the boot_test variable).
>
> The assessment resulting from this can be reviewed at [4] section 3.2 In
> particular, the numbers provided at the end of sections 3.2.3 and 3.2.4 are
> obtained with this interface.
>
> Other examples where such raw unconditioned noise data is needed for further
> analysis is [5], especially chapter 6.
>
> This testing has nothing to do with the runtime testing provided with the
> patch set 11. All data that ends up here is not available to the LRNG and will
> not contribute to any entropy collection.
>
> See the following:
>
>
> if (this_cpu_read(cpu_tlbstate.loaded_mm) == &init_mm)
> return;
>
> --
> 2.17.1
static inline void lrng_time_process(void)
> {
> ...
> if (lrng_raw_entropy_store(now_time))
> return;
>
>
> bool lrng_raw_entropy_store(u32 value)
> {
> ...
> if (!atomic_read(&lrng_testing_enabled) && !boot_test)
> return false;
> ...
> return true;
>
> >
> > Am I correct in assuming that this function can be called concurrently in
> > different threads or CPUs?
>
> Yes, because it is called indirectly by add_interrupt_randomness.
> > > +
> > > + /*
> > > + * Disable entropy testing for boot time testing after ring buffer
> > > + * is filled.
> > > + */
> > > + if (boot_test && write_ptr > LRNG_TESTING_RINGBUFFER_SIZE) {
> > > + pr_warn_once("Boot time entropy collection test disabled\n");
> > > + return false;
> > > + }
> > > +
> > > + if (boot_test && !atomic_read(&lrng_rb_first_in))
> > > + pr_warn("Boot time entropy collection test enabled\n");
> > > +
> > > + lrng_testing_rb[write_ptr & LRNG_TESTING_RINGBUFFER_MASK] = value;
> >
> > You’re writing *somewhere*, but not necessarily to the first open slot.
>
> The idea is that there is a reader pointer and a writer pointer where the
> reader always must be smaller or equal to the writer (modulo the size of the
> ring buffer). So, I do not care where the writer ptr is.
>
> All I need is that:
>
> 1. reader and writer ptr must start with the same value at boot time (e.g. 0)
>
> 2. reader ptr is always <= writer ptr in order for data to be read.
You're making an implicit assumption, though: that the writer has
actually written the data indicated by the writer ptr. But you're
updating the writer ptr *before* you write, so that assumption is
false.
> But I simplified the code now, the code now only copies the data out if the
> reader <= writer modulo the ring buffer size. In this case, if the writer is
> much faster, then we loose some values.
I'll look at the new code. I'm not sure what you mean by a <= b
modulo N -- in a cyclic group (e.g. the integers 0 .. N-1), there
isn't really a well-defined concept of <=.
^ permalink raw reply
* Re: [PATCH v25 03/12] LRNG - /proc interface
From: Andy Lutomirski @ 2019-11-19 10:06 UTC (permalink / raw)
To: Stephan Müller
Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Crypto Mailing List,
LKML, Linux API, 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
In-Reply-To: <3043322.Kq9igzfA0K@positron.chronox.de>
On Sun, Nov 17, 2019 at 4:16 AM Stephan Müller <smueller@chronox.de> wrote:
>
> Am Samstag, 16. November 2019, 17:39:40 CET schrieb Andy Lutomirski:
>
> Hi Andy,
>
> > > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de> wrote:
> > >
> > > The LRNG /proc interface provides the same files as the legacy
> > > /dev/random. These files behave identically. Yet, all files are
> > > documented at [1].
> >
> > Why?
>
> I am not sure here: are you referring to the documentation? Or the one
> additional file?
>
> If it is the documentation, do you want me to add it to the patch description?
> I initially did not add it as these files were present and seemingly known
> what they provide. But I would add that documentation to the patch description
> if this is desired.
Sorry, I should have been a lot more explicit. Why do you want to add
a new interface to read the RNG? What's wrong with the old one?
I think your patch description should explain the purpose of the patch.
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Andy Lutomirski @ 2019-11-19 10:07 UTC (permalink / raw)
To: Stephan Müller
Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Crypto Mailing List,
LKML, Linux API, 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
In-Reply-To: <3159012.PsEOTp9LqO@positron.chronox.de>
On Sun, Nov 17, 2019 at 3:11 AM Stephan Müller <smueller@chronox.de> wrote:
>
> Am Samstag, 16. November 2019, 17:09:09 CET schrieb Andy Lutomirski:
>
> Hi Andy,
>
> > > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de> wrote:
> > >
> > > The True Random Number Generator (TRNG) provides a random number
> > > generator with prediction resistance (SP800-90A terminology) or an NTG.1
> > > (AIS 31 terminology).
> >
> > ...
> >
> > > The secondary DRNGs seed from the TRNG if it is present. In addition,
> > > the /dev/random device accesses the TRNG.
> > >
> > > If the TRNG is disabled, the secondary DRNGs seed from the entropy pool
> > > and /dev/random behaves like getrandom(2).
> >
> > As mentioned before, I don’t like this API. An application that, for some
> > reason, needs a TRNG, should have an API by which it either gets a TRNG or
> > an error. Similarly, an application that wants cryptographically secure
> > random numbers efficiently should have an API that does that. With your
> > design, /dev/random tries to cater to both use cases, but one of the use
> > cases fails depending on kernel config.
> >
> > I think /dev/random should wait for enough entropy to initialize the system
> > but should not block after that. A TRNG should have an entirely new API
> > that is better than /dev/random.
>
> I apologize for the misunderstanding. I assumed we would introduce such /dev/
> true_random at a later stage.
>
> If you agree, I can certainly add /dev/true_random right now that links with
> the TRNG and make /dev/random behave as discussed, i.e. behave exactly like
> getrandom(..., 0);
Given that your series is already a decently large ABI change, I think
it would be polite to users to make all the anticipated changes all at
once to reduce the amount of churn everyone needs to deal with.
>
> As this would introduce a new device file now, is there a special process that
> I need to follow or do I need to copy? Which major/minor number should I use?
>
> Looking into static const struct memdev devlist[] I see
>
> [8] = { "random", 0666, &random_fops, 0 },
> [9] = { "urandom", 0666, &urandom_fops, 0 },
>
> Shall a true_random be added here with [10]?
I am not at all an expert on chardevs, but this sounds generally
reasonable. gregkh is probably the real authority here.
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Stephan Mueller @ 2019-11-19 10:46 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Crypto Mailing List,
LKML, Linux API, 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, Florian Weimer,
Lennart Poettering, Nicolai Stange <nstang>
In-Reply-To: <CALCETrUKDO1LSMnHNcPiAFQh2ri6saRiRBi9b5e699cm1_Mgsw@mail.gmail.com>
Am Dienstag, 19. November 2019, 11:07:40 CET schrieb Andy Lutomirski:
Hi Andy,
> On Sun, Nov 17, 2019 at 3:11 AM Stephan Müller <smueller@chronox.de> wrote:
> > Am Samstag, 16. November 2019, 17:09:09 CET schrieb Andy Lutomirski:
> >
> > Hi Andy,
> >
> > > > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de>
> > > > wrote:
> > > >
> > > > The True Random Number Generator (TRNG) provides a random number
> > > > generator with prediction resistance (SP800-90A terminology) or an
> > > > NTG.1
> > > > (AIS 31 terminology).
> > >
> > > ...
> > >
> > > > The secondary DRNGs seed from the TRNG if it is present. In addition,
> > > > the /dev/random device accesses the TRNG.
> > > >
> > > > If the TRNG is disabled, the secondary DRNGs seed from the entropy
> > > > pool
> > > > and /dev/random behaves like getrandom(2).
> > >
> > > As mentioned before, I don’t like this API. An application that, for
> > > some
> > > reason, needs a TRNG, should have an API by which it either gets a TRNG
> > > or
> > > an error. Similarly, an application that wants cryptographically secure
> > > random numbers efficiently should have an API that does that. With your
> > > design, /dev/random tries to cater to both use cases, but one of the use
> > > cases fails depending on kernel config.
> > >
> > > I think /dev/random should wait for enough entropy to initialize the
> > > system
> > > but should not block after that. A TRNG should have an entirely new API
> > > that is better than /dev/random.
> >
> > I apologize for the misunderstanding. I assumed we would introduce such
> > /dev/ true_random at a later stage.
> >
> > If you agree, I can certainly add /dev/true_random right now that links
> > with the TRNG and make /dev/random behave as discussed, i.e. behave
> > exactly like getrandom(..., 0);
>
> Given that your series is already a decently large ABI change, I think
> it would be polite to users to make all the anticipated changes all at
> once to reduce the amount of churn everyone needs to deal with.
I fully agree with that. This is the reason for my question to ask for how to
approach adding such additional interface for the TRNG. Considering that there
are use cases for the blocking interfaces, I would like to have it present
from the start.
That said, are you planning to keep the blocking_pool considering our
discussion?
But may I ask why you say that there are ABI changes? The LRNG set tries to be
100% API and ABI compliant to the existing random.c interfaces.
The only change is the runtime-behavior (e.g. time when something is seeded),
performance of the interrupt handling and the kernel/user interfaces that may
be noticeable by users. I.e. there are timing changes, but no technical
changes.
The LRNG though would add one optional interface for kernel internal use and
then the discussed /dev/true_random interface.
>
> > As this would introduce a new device file now, is there a special process
> > that I need to follow or do I need to copy? Which major/minor number
> > should I use?
> >
> > Looking into static const struct memdev devlist[] I see
> >
> > [8] = { "random", 0666, &random_fops, 0 },
> > [9] = { "urandom", 0666, &urandom_fops, 0 },
> >
> > Shall a true_random be added here with [10]?
>
> I am not at all an expert on chardevs, but this sounds generally
> reasonable. gregkh is probably the real authority here.
Thank you for the hint. Then I would like to wait for Greg on this.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 03/12] LRNG - /proc interface
From: Stephan Mueller @ 2019-11-19 10:55 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Crypto Mailing List,
LKML, Linux API, 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, Florian Weimer,
Lennart Poettering, Nicolai Stange <nstang>
In-Reply-To: <CALCETrVXGuShozaf5RpgmQnwtTpAbmaTVny+E0q8OE4OLuWwAQ@mail.gmail.com>
Am Dienstag, 19. November 2019, 11:06:02 CET schrieb Andy Lutomirski:
Hi Andy,
> On Sun, Nov 17, 2019 at 4:16 AM Stephan Müller <smueller@chronox.de> wrote:
> > Am Samstag, 16. November 2019, 17:39:40 CET schrieb Andy Lutomirski:
> >
> > Hi Andy,
> >
> > > > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de>
> > > > wrote:
> > > >
> > > > The LRNG /proc interface provides the same files as the legacy
> > > > /dev/random. These files behave identically. Yet, all files are
> > > > documented at [1].
> > >
> > > Why?
> >
> > I am not sure here: are you referring to the documentation? Or the one
> > additional file?
> >
> > If it is the documentation, do you want me to add it to the patch
> > description? I initially did not add it as these files were present and
> > seemingly known what they provide. But I would add that documentation to
> > the patch description if this is desired.
>
> Sorry, I should have been a lot more explicit. Why do you want to add
> a new interface to read the RNG? What's wrong with the old one?
There is nothing wrong at all. I actually want to be 100% API and ABI
compliant with the existing random.c. Thus, the list of the sysctls are
identical to the existing random.c with the same behavior (hence I skipped the
documentation of these files).
Yet, the wiring up of the interfaces to internal data structures and handlers
is different than for the existing random.c.
Also, the reason why I created a separate lrng_proc.c (and lrng_interface.c)
is to allow a possible merger of this similar code with the existing random.c.
The only question that needs to be solved is to find a common way to invoke
the random.c internal logic and the LRNG internal logic with these interfaces.
>
> I think your patch description should explain the purpose of the patch.
Ok, I can surely add a description for each file to the patch description.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v25 10/12] LRNG - add TRNG support
From: Greg Kroah-Hartman @ 2019-11-19 12:41 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephan Müller, Arnd Bergmann, Linux Crypto Mailing List,
LKML, Linux API, 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, Florian Weimer,
Lennart Poettering
In-Reply-To: <CALCETrUKDO1LSMnHNcPiAFQh2ri6saRiRBi9b5e699cm1_Mgsw@mail.gmail.com>
On Tue, Nov 19, 2019 at 02:07:40AM -0800, Andy Lutomirski wrote:
> > As this would introduce a new device file now, is there a special process that
> > I need to follow or do I need to copy? Which major/minor number should I use?
> >
> > Looking into static const struct memdev devlist[] I see
> >
> > [8] = { "random", 0666, &random_fops, 0 },
> > [9] = { "urandom", 0666, &urandom_fops, 0 },
> >
> > Shall a true_random be added here with [10]?
>
> I am not at all an expert on chardevs, but this sounds generally
> reasonable. gregkh is probably the real authority here.
[10] is the aio char device node, so you better not try to overlap it or
bad things will happen :(
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v25 12/12] LRNG - add interface for gathering of raw entropy
From: Randy Dunlap @ 2019-11-19 17:17 UTC (permalink / raw)
To: Stephan Müller, Arnd Bergmann
Cc: Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
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" <matth>
In-Reply-To: <3610406.x8mDjznOIz@positron.chronox.de>
Hi,
On 11/16/19 1:38 AM, Stephan Müller wrote:
> diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> index e6ca3acc1e48..4ccc710832ef 100644
> --- a/drivers/char/lrng/Kconfig
> +++ b/drivers/char/lrng/Kconfig
> @@ -169,4 +169,20 @@ config LRNG_APT_CUTOFF
> default 325 if !LRNG_APT_BROKEN
> default 32 if LRNG_APT_BROKEN
>
> +config LRNG_TESTING
> + bool "Enable entropy test interface to LRNG noise source"
> + select CONFIG_DEBUG_FS
That should be spelled as DEBUG_FS and preferably would be "depends on"
instead of 'select'.
> + help
> + The test interface allows a privileged process to capture
> + the raw unconditioned noise that is collected by the LRNG
> + for statistical analysis. Extracted noise data is not used
> + to seed the LRNG.
> +
> + The raw noise data can be obtained using the lrng_raw
> + debugfs file. Using the option lrng_testing.boot_test=1
> + the raw noise of the first 1000 entropy events since boot
> + can be sampled.
> +
> + If unsure, say N.
> +
> endif # LRNG
thanks.
--
~Randy
^ permalink raw reply
* Re: [PATCH v25 03/12] LRNG - /proc interface
From: Andy Lutomirski @ 2019-11-19 17:40 UTC (permalink / raw)
To: Stephan Mueller
Cc: Andy Lutomirski, Arnd Bergmann, Greg Kroah-Hartman,
Linux Crypto Mailing List, LKML, Linux API, 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, Florian Weimer,
Lennart Poettering
In-Reply-To: <5323691.yyFvDVlHDV@tauon.chronox.de>
On Tue, Nov 19, 2019 at 2:57 AM Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Dienstag, 19. November 2019, 11:06:02 CET schrieb Andy Lutomirski:
>
> Hi Andy,
>
> > On Sun, Nov 17, 2019 at 4:16 AM Stephan Müller <smueller@chronox.de> wrote:
> > > Am Samstag, 16. November 2019, 17:39:40 CET schrieb Andy Lutomirski:
> > >
> > > Hi Andy,
> > >
> > > > > On Nov 16, 2019, at 1:40 AM, Stephan Müller <smueller@chronox.de>
> > > > > wrote:
> > > > >
> > > > > The LRNG /proc interface provides the same files as the legacy
> > > > > /dev/random. These files behave identically. Yet, all files are
> > > > > documented at [1].
> > > >
> > > > Why?
> > >
> > > I am not sure here: are you referring to the documentation? Or the one
> > > additional file?
> > >
> > > If it is the documentation, do you want me to add it to the patch
> > > description? I initially did not add it as these files were present and
> > > seemingly known what they provide. But I would add that documentation to
> > > the patch description if this is desired.
> >
> > Sorry, I should have been a lot more explicit. Why do you want to add
> > a new interface to read the RNG? What's wrong with the old one?
>
> There is nothing wrong at all. I actually want to be 100% API and ABI
> compliant with the existing random.c. Thus, the list of the sysctls are
> identical to the existing random.c with the same behavior (hence I skipped the
> documentation of these files).
Whoops, I misunderstood your commit message. You said "The LRNG /proc
interface provides the same files as the legacy
/dev/random.". I assumed that meant that you had a file in /proc that
worked like /dev/random.
So never mind.
^ permalink raw reply
* [PATCH RESEND v17 00/13] open: introduce openat2(2) syscall
From: Aleksa Sarai @ 2019-11-20 5:06 UTC (permalink / raw)
To: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells, Shuah Khan, Shuah Khan, Ingo Molnar,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Jonathan Corbet
Cc: Aleksa Sarai, Eric Biederman, Andy Lutomirski, Andrew Morton,
Kees Cook, Jann Horn, Tycho Andersen, David Drysdale, Chanho Min,
Oleg Nesterov, Rasmus Villemoes, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Christian Brauner, Aleksa Sarai, Linus Torvalds,
dev, containers, bpf, netdev
This patchset is being developed here:
<https://github.com/cyphar/linux/tree/openat2/master>
This is a re-send of
<https://lore.kernel.org/lkml/20191117011713.13032-1-cyphar@cyphar.com/>
but rebased on top of 5.4-rc8 (also my mails got duplicated the first
time I sent v17 -- hopefully that doesn't happen this time).
Patch changelog:
v17:
* Add a path_is_under() check for LOOKUP_IS_SCOPED in complete_walk(), as a
last line of defence to ensure that namei bugs will not break the contract
of LOOKUP_BENEATH or LOOKUP_IN_ROOT.
* Update based on feedback by Al Viro:
* Make nd_jump_link() free the passed path on error, so that callers don't
need to worry about it in the error path.
* Remove needless m_retry and r_retry variables in handle_dots().
* Always return -ECHILD from follow_dotdot_rcu().
v16: <https://lore.kernel.org/lkml/20191116002802.6663-1-cyphar@cyphar.com/>
v15: <https://lore.kernel.org/lkml/20191105090553.6350-1-cyphar@cyphar.com/>
v14: <https://lore.kernel.org/lkml/20191010054140.8483-1-cyphar@cyphar.com/>
<https://lore.kernel.org/lkml/20191026185700.10708-1-cyphar@cyphar.com>
v13: <https://lore.kernel.org/lkml/20190930183316.10190-1-cyphar@cyphar.com/>
v12: <https://lore.kernel.org/lkml/20190904201933.10736-1-cyphar@cyphar.com/>
v11: <https://lore.kernel.org/lkml/20190820033406.29796-1-cyphar@cyphar.com/>
<https://lore.kernel.org/lkml/20190728010207.9781-1-cyphar@cyphar.com/>
v10: <https://lore.kernel.org/lkml/20190719164225.27083-1-cyphar@cyphar.com/>
v09: <https://lore.kernel.org/lkml/20190706145737.5299-1-cyphar@cyphar.com/>
v08: <https://lore.kernel.org/lkml/20190520133305.11925-1-cyphar@cyphar.com/>
v07: <https://lore.kernel.org/lkml/20190507164317.13562-1-cyphar@cyphar.com/>
v06: <https://lore.kernel.org/lkml/20190506165439.9155-1-cyphar@cyphar.com/>
v05: <https://lore.kernel.org/lkml/20190320143717.2523-1-cyphar@cyphar.com/>
v04: <https://lore.kernel.org/lkml/20181112142654.341-1-cyphar@cyphar.com/>
v03: <https://lore.kernel.org/lkml/20181009070230.12884-1-cyphar@cyphar.com/>
v02: <https://lore.kernel.org/lkml/20181009065300.11053-1-cyphar@cyphar.com/>
v01: <https://lore.kernel.org/lkml/20180929103453.12025-1-cyphar@cyphar.com/>
For a very long time, extending openat(2) with new features has been
incredibly frustrating. This stems from the fact that openat(2) is
possibly the most famous counter-example to the mantra "don't silently
accept garbage from userspace" -- it doesn't check whether unknown flags
are present[1].
This means that (generally) the addition of new flags to openat(2) has
been fraught with backwards-compatibility issues (O_TMPFILE has to be
defined as __O_TMPFILE|O_DIRECTORY|[O_RDWR or O_WRONLY] to ensure old
kernels gave errors, since it's insecure to silently ignore the
flag[2]). All new security-related flags therefore have a tough road to
being added to openat(2).
Furthermore, the need for some sort of control over VFS's path resolution (to
avoid malicious paths resulting in inadvertent breakouts) has been a very
long-standing desire of many userspace applications. This patchset is a revival
of Al Viro's old AT_NO_JUMPS[3] patchset (which was a variant of David
Drysdale's O_BENEATH patchset[4] which was a spin-off of the Capsicum
project[5]) with a few additions and changes made based on the previous
discussion within [6] as well as others I felt were useful.
In line with the conclusions of the original discussion of AT_NO_JUMPS, the
flag has been split up into separate flags. However, instead of being an
openat(2) flag it is provided through a new syscall openat2(2) which provides
several other improvements to the openat(2) interface (see the patch
description for more details). The following new LOOKUP_* flags are added:
* LOOKUP_NO_XDEV blocks all mountpoint crossings (upwards, downwards,
or through absolute links). Absolute pathnames alone in openat(2) do not
trigger this. Magic-link traversal which implies a vfsmount jump is also
blocked (though magic-link jumps on the same vfsmount are permitted).
* LOOKUP_NO_MAGICLINKS blocks resolution through /proc/$pid/fd-style
links. This is done by blocking the usage of nd_jump_link() during
resolution in a filesystem. The term "magic-links" is used to match
with the only reference to these links in Documentation/, but I'm
happy to change the name.
It should be noted that this is different to the scope of
~LOOKUP_FOLLOW in that it applies to all path components. However,
you can do openat2(NO_FOLLOW|NO_MAGICLINKS) on a magic-link and it
will *not* fail (assuming that no parent component was a
magic-link), and you will have an fd for the magic-link.
In order to correctly detect magic-links, the introduction of a new
LOOKUP_MAGICLINK_JUMPED state flag was required.
* LOOKUP_BENEATH disallows escapes to outside the starting dirfd's
tree, using techniques such as ".." or absolute links. Absolute
paths in openat(2) are also disallowed. Conceptually this flag is to
ensure you "stay below" a certain point in the filesystem tree --
but this requires some additional to protect against various races
that would allow escape using "..".
Currently LOOKUP_BENEATH implies LOOKUP_NO_MAGICLINKS, because it
can trivially beam you around the filesystem (breaking the
protection). In future, there might be similar safety checks done as
in LOOKUP_IN_ROOT, but that requires more discussion.
In addition, two new flags are added that expand on the above ideas:
* LOOKUP_NO_SYMLINKS does what it says on the tin. No symlink
resolution is allowed at all, including magic-links. Just as with
LOOKUP_NO_MAGICLINKS this can still be used with NOFOLLOW to open an
fd for the symlink as long as no parent path had a symlink
component.
* LOOKUP_IN_ROOT is an extension of LOOKUP_BENEATH that, rather than
blocking attempts to move past the root, forces all such movements
to be scoped to the starting point. This provides chroot(2)-like
protection but without the cost of a chroot(2) for each filesystem
operation, as well as being safe against race attacks that chroot(2)
is not.
If a race is detected (as with LOOKUP_BENEATH) then an error is
generated, and similar to LOOKUP_BENEATH it is not permitted to cross
magic-links with LOOKUP_IN_ROOT.
The primary need for this is from container runtimes, which
currently need to do symlink scoping in userspace[7] when opening
paths in a potentially malicious container. There is a long list of
CVEs that could have bene mitigated by having RESOLVE_THIS_ROOT
(such as CVE-2017-1002101, CVE-2017-1002102, CVE-2018-15664, and
CVE-2019-5736, just to name a few).
In order to make all of the above more usable, I'm working on
libpathrs[8] which is a C-friendly library for safe path resolution. It
features a userspace-emulated backend if the kernel doesn't support
openat2(2). Hopefully we can get userspace to switch to using it, and
thus get openat2(2) support for free once it's ready.
Future work would include implementing things like RESOLVE_NO_AUTOMOUNT and
possibly a RESOLVE_NO_REMOTE (to allow programs to be sure they don't hit DoSes
though stale NFS handles).
[1]: https://lwn.net/Articles/588444/
[2]: https://lore.kernel.org/lkml/CA+55aFyyxJL1LyXZeBsf2ypriraj5ut1XkNDsunRBqgVjZU_6Q@mail.gmail.com
[3]: https://lore.kernel.org/lkml/20170429220414.GT29622@ZenIV.linux.org.uk
[4]: https://lore.kernel.org/lkml/1415094884-18349-1-git-send-email-drysdale@google.com
[5]: https://lore.kernel.org/lkml/1404124096-21445-1-git-send-email-drysdale@google.com
[6]: https://lwn.net/Articles/723057/
[7]: https://github.com/cyphar/filepath-securejoin
[8]: https://github.com/openSUSE/libpathrs
The current draft of the openat2(2) man-page is included below.
--8<---------------------------------------------------------------------------
OPENAT2(2) Linux Programmer's Manual OPENAT2(2)
NAME
openat2 - open and possibly create a file (extended)
SYNOPSIS
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
int openat2(int dirfd, const char *pathname, struct open_how *how, size_t size);
Note: There is no glibc wrapper for this system call; see NOTES.
DESCRIPTION
The openat2() system call opens the file specified by pathname. If the specified file
does not exist, it may optionally (if O_CREAT is specified in how.flags) be created by
openat2().
As with openat(2), if pathname is relative, then it is interpreted relative to the direc-
tory referred to by the file descriptor dirfd (or the current working directory of the
calling process, if dirfd is the special value AT_FDCWD.) If pathname is absolute, then
dirfd is ignored (unless how.resolve contains RESOLVE_IN_ROOT, in which case pathname is
resolved relative to dirfd.)
The openat2() system call is an extension of openat(2) and provides a superset of its
functionality. Rather than taking a single flag argument, an extensible structure (how)
is passed instead to allow for future extensions. size must be set to sizeof(struct
open_how), to facilitate future extensions (see the "Extensibility" section of the NOTES
for more detail on how extensions are handled.)
The open_how structure
The following structure indicates how pathname should be opened, and acts as a superset of
the flag and mode arguments to openat(2).
struct open_how {
__aligned_u64 flags; /* O_* flags. */
__u16 mode; /* Mode for O_{CREAT,TMPFILE}. */
__u16 __padding[3]; /* Must be zeroed. */
__aligned_u64 resolve; /* RESOLVE_* flags. */
};
Any future extensions to openat2() will be implemented as new fields appended to the above
structure (or through reuse of pre-existing padding space), with the zero value of the new
fields acting as though the extension were not present.
The meaning of each field is as follows:
flags
The file creation and status flags to use for this operation. All of the
O_* flags defined for openat(2) are valid openat2() flag values.
Unlike openat(2), it is an error to provide openat2() unknown or conflicting
flags in flags.
mode
File mode for the new file, with identical semantics to the mode argument to
openat(2). However, unlike openat(2), it is an error to provide openat2()
with a mode which contains bits other than 0777.
It is an error to provide openat2() a non-zero mode if flags does not con-
tain O_CREAT or O_TMPFILE.
resolve
Change how the components of pathname will be resolved (see path_resolu-
tion(7) for background information.) The primary use case for these flags
is to allow trusted programs to restrict how untrusted paths (or paths in-
side untrusted directories) are resolved. The full list of resolve flags is
given below.
RESOLVE_NO_XDEV
Disallow traversal of mount points during path resolution (including
all bind mounts).
Users of this flag are encouraged to make its use configurable (un-
less it is used for a specific security purpose), as bind mounts are
very widely used by end-users. Setting this flag indiscrimnately for
all uses of openat2() may result in spurious errors on previously-
functional systems.
RESOLVE_NO_SYMLINKS
Disallow resolution of symbolic links during path resolution. This
option implies RESOLVE_NO_MAGICLINKS.
If the trailing component is a symbolic link, and flags contains both
O_PATH and O_NOFOLLOW, then an O_PATH file descriptor referencing the
symbolic link will be returned.
Users of this flag are encouraged to make its use configurable (un-
less it is used for a specific security purpose), as symbolic links
are very widely used by end-users. Setting this flag indiscrimnately
for all uses of openat2() may result in spurious errors on previ-
ously-functional systems.
RESOLVE_NO_MAGICLINKS
Disallow all magic link resolution during path resolution.
If the trailing component is a magic link, and flags contains both
O_PATH and O_NOFOLLOW, then an O_PATH file descriptor referencing the
magic link will be returned.
Magic-links are symbolic link-like objects that are most notably
found in proc(5) (examples include /proc/[pid]/exe and
/proc/[pid]/fd/*.) Due to the potential danger of unknowingly open-
ing these magic links, it may be preferable for users to disable
their resolution entirely (see symboliclink(7) for more details.)
RESOLVE_BENEATH
Do not permit the path resolution to succeed if any component of the
resolution is not a descendant of the directory indicated by dirfd.
This results in absolute symbolic links (and absolute values of path-
name) to be rejected.
Currently, this flag also disables magic link resolution. However,
this may change in the future. The caller should explicitly specify
RESOLVE_NO_MAGICLINKS to ensure that magic links are not resolved.
RESOLVE_IN_ROOT
Treat dirfd as the root directory while resolving pathname (as though
the user called chroot(2) with dirfd as the argument.) Absolute sym-
bolic links and ".." path components will be scoped to dirfd. If
pathname is an absolute path, it is also treated relative to dirfd.
However, unlike chroot(2) (which changes the filesystem root perma-
nently for a process), RESOLVE_IN_ROOT allows a program to effi-
ciently restrict path resolution for only certain operations. It
also has several hardening features (such detecting escape attempts
during .. resolution) which chroot(2) does not.
Currently, this flag also disables magic link resolution. However,
this may change in the future. The caller should explicitly specify
RESOLVE_NO_MAGICLINKS to ensure that magic links are not resolved.
It is an error to provide openat2() unknown flags in resolve.
RETURN VALUE
On success, a new file descriptor is returned. On error, -1 is returned, and errno is set
appropriately.
ERRORS
The set of errors returned by openat2() includes all of the errors returned by openat(2),
as well as the following additional errors:
EINVAL An unknown flag or invalid value was specified in how.
EINVAL mode is non-zero, but flags does not contain O_CREAT or O_TMPFILE.
EINVAL size was smaller than any known version of struct open_how.
E2BIG An extension was specified in how, which the current kernel does not support (see
the "Extensibility" section of the NOTES for more detail on how extensions are han-
dled.)
EAGAIN resolve contains either RESOLVE_IN_ROOT or RESOLVE_BENEATH, and the kernel could
not ensure that a ".." component didn't escape (due to a race condition or poten-
tial attack.) Callers may choose to retry the openat2() call.
EXDEV resolve contains either RESOLVE_IN_ROOT or RESOLVE_BENEATH, and an escape from the
root during path resolution was detected.
EXDEV resolve contains RESOLVE_NO_XDEV, and a path component attempted to cross a mount
point.
ELOOP resolve contains RESOLVE_NO_SYMLINKS, and one of the path components was a symbolic
link (or magic link).
ELOOP resolve contains RESOLVE_NO_MAGICLINKS, and one of the path components was a magic
link.
VERSIONS
openat2() was added to Linux in kernel 5.FOO.
CONFORMING TO
This system call is Linux-specific.
The semantics of RESOLVE_BENEATH were modelled after FreeBSD's O_BENEATH.
NOTES
Glibc does not provide a wrapper for this system call; call it using systemcall(2).
Extensibility
In order to allow for struct open_how to be extended in future kernel revisions, openat2()
requires userspace to specify the size of struct open_how structure they are passing. By
providing this information, it is possible for openat2() to provide both forwards- and
backwards-compatibility — with size acting as an implicit version number (because new ex-
tension fields will always be appended, the size will always increase.) This extensibil-
ity design is very similar to other system calls such as perf_setattr(2),
perf_event_open(2), and clone(3).
If we let usize be the size of the structure according to userspace and ksize be the size
of the structure which the kernel supports, then there are only three cases to consider:
* If ksize equals usize, then there is no version mismatch and how can be used
verbatim.
* If ksize is larger than usize, then there are some extensions the kernel sup-
ports which the userspace program is unaware of. Because all extensions must
have their zero values be a no-op, the kernel treats all of the extension fields
not set by userspace to have zero values. This provides backwards-compatibil-
ity.
* If ksize is smaller than usize, then there are some extensions which the
userspace program is aware of but the kernel does not support. Because all ex-
tensions must have their zero values be a no-op, the kernel can safely ignore
the unsupported extension fields if they are all-zero. If any unsupported ex-
tension fields are non-zero, then -1 is returned and errno is set to E2BIG.
This provides forwards-compatibility.
Therefore, most userspace programs will not need to have any special handling of exten-
sions. However, if a userspace program wishes to determine what extensions the running
kernel supports, they may conduct a binary search on size (to find the largest value which
doesn't produce an error of E2BIG.)
SEE ALSO
openat(2), path_resolution(7), symlink(7)
Linux 2019-11-05 OPENAT2(2)
--8<---------------------------------------------------------------------------
Aleksa Sarai (13):
namei: only return -ECHILD from follow_dotdot_rcu()
nsfs: clean-up ns_get_path() signature to return int
namei: allow nd_jump_link() to produce errors
namei: allow set_root() to produce errors
namei: LOOKUP_NO_SYMLINKS: block symlink resolution
namei: LOOKUP_NO_MAGICLINKS: block magic-link resolution
namei: LOOKUP_NO_XDEV: block mountpoint crossing
namei: LOOKUP_BENEATH: O_BENEATH-like scoped resolution
namei: LOOKUP_IN_ROOT: chroot-like scoped resolution
namei: LOOKUP_{IN_ROOT,BENEATH}: permit limited ".." resolution
open: introduce openat2(2) syscall
selftests: add openat2(2) selftests
Documentation: path-lookup: include new LOOKUP flags
CREDITS | 4 +-
Documentation/filesystems/path-lookup.rst | 68 ++-
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
fs/namei.c | 185 +++++--
fs/nsfs.c | 29 +-
fs/open.c | 149 +++--
fs/proc/base.c | 3 +-
fs/proc/namespaces.c | 20 +-
include/linux/fcntl.h | 12 +-
include/linux/namei.h | 12 +-
include/linux/proc_ns.h | 4 +-
include/linux/syscalls.h | 3 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/fcntl.h | 40 ++
kernel/bpf/offload.c | 12 +-
kernel/events/core.c | 2 +-
security/apparmor/apparmorfs.c | 6 +-
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/openat2/.gitignore | 1 +
tools/testing/selftests/openat2/Makefile | 8 +
tools/testing/selftests/openat2/helpers.c | 109 ++++
tools/testing/selftests/openat2/helpers.h | 107 ++++
.../testing/selftests/openat2/openat2_test.c | 316 +++++++++++
.../selftests/openat2/rename_attack_test.c | 160 ++++++
.../testing/selftests/openat2/resolve_test.c | 523 ++++++++++++++++++
42 files changed, 1686 insertions(+), 113 deletions(-)
create mode 100644 tools/testing/selftests/openat2/.gitignore
create mode 100644 tools/testing/selftests/openat2/Makefile
create mode 100644 tools/testing/selftests/openat2/helpers.c
create mode 100644 tools/testing/selftests/openat2/helpers.h
create mode 100644 tools/testing/selftests/openat2/openat2_test.c
create mode 100644 tools/testing/selftests/openat2/rename_attack_test.c
create mode 100644 tools/testing/selftests/openat2/resolve_test.c
base-commit: af42d3466bdc8f39806b26f593604fdc54140bcb
--
2.24.0
^ permalink raw reply
* [PATCH RESEND v17 01/13] namei: only return -ECHILD from follow_dotdot_rcu()
From: Aleksa Sarai @ 2019-11-20 5:06 UTC (permalink / raw)
To: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells, Shuah Khan, Shuah Khan, Ingo Molnar,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Jonathan Corbet
Cc: Aleksa Sarai, Eric W. Biederman, Andy Lutomirski, Andrew Morton,
Kees Cook, Jann Horn, Tycho Andersen, David Drysdale, Chanho Min,
Oleg Nesterov, Rasmus Villemoes, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Christian Brauner, Aleksa Sarai, Linus Torvalds,
dev, containers, bpf, netdev
In-Reply-To: <20191120050631.12816-1-cyphar@cyphar.com>
It's over-zealous to return hard errors under RCU-walk here, given that
a REF-walk will be triggered for all other cases handling ".." under
RCU.
The original purpose of this check was to ensure that if a rename occurs
such that a directory is moved outside of the bind-mount which the
resolution started in, it would be detected and blocked to avoid being
able to mess with paths outside of the bind-mount. However, triggering a
new REF-walk is just as effective a solution.
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Fixes: 397d425dc26d ("vfs: Test for and handle paths that are unreachable from their mnt_root")
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
fs/namei.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/namei.c b/fs/namei.c
index 671c3c1a3425..5a47d9c09581 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1359,7 +1359,7 @@ static int follow_dotdot_rcu(struct nameidata *nd)
nd->path.dentry = parent;
nd->seq = seq;
if (unlikely(!path_connected(&nd->path)))
- return -ENOENT;
+ return -ECHILD;
break;
} else {
struct mount *mnt = real_mount(nd->path.mnt);
--
2.24.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox