* Re: [PATCH v24 00/12] /dev/random - a new approach with full SP800-90B compliance
From: Stephan Müller @ 2019-11-12 23:03 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: <CALCETrVBzuOsDfaz5y3V4v+6xmeWufOYsOGnpZrRju6Pfsi6gg@mail.gmail.com>
Am Dienstag, 12. November 2019, 16:33:59 CET schrieb Andy Lutomirski:
Hi Andy,
> On Mon, Nov 11, 2019 at 11:13 AM Stephan Müller <smueller@chronox.de> wrote:
> > The following patch set provides a different approach to /dev/random which
> > is called Linux Random Number Generator (LRNG) to collect entropy within
> > the Linux kernel. The main improvements compared to the existing
> > /dev/random is to provide sufficient entropy during boot time as well as
> > in virtual environments and when using SSDs. A secondary design goal is
> > to limit the impact of the entropy collection on massive parallel systems
> > and also allow the use accelerated cryptographic primitives. Also, all
> > steps of the entropic data processing are testable.
>
> This is very nice!
>
> > The LRNG patch set allows a user to select use of the existing /dev/random
> > or the LRNG during compile time. As the LRNG provides API and ABI
> > compatible interfaces to the existing /dev/random implementation, the
> > user can freely chose the RNG implementation without affecting kernel or
> > user space operations.
> >
> > This patch set provides early boot-time entropy which implies that no
> > additional flags to the getrandom(2) system call discussed recently on
> > the LKML is considered to be necessary.
>
> I'm uneasy about this. I fully believe that, *on x86*, this works.
> But on embedded systems with in-order CPUs, a single clock, and very
> lightweight boot processes, most or all of boot might be too
> deterministic for this to work.
I agree that in such cases, my LRNG getrandom(2) would also block until the
LRNG thinks it collected 256 bits of entropy. However, I am under the
impression that the LRNG collects that entropy faster that the existing /dev/
random implementation, even in this case.
Nicolai is copied on this thread. He promised to have the LRNG tested on such
a minimalistic system that you describe. I hope he could contribute some
numbers from that test helping us to understand how much of a problem we face.
>
> I have a somewhat competing patch set here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=random
> /kill-it
>
> (Ignore the "horrible test hack" and the debugfs part.)
>
> The basic summary is that I change /dev/random so that it becomes
> functionally identical to getrandom(..., 0) -- in other words, it
> blocks until the CRNG is initialized but is then identical to
> /dev/urandom.
This would be equal to the LRNG code without compiling the TRNG.
> And I add getrandom(...., GRND_INSECURE) that is
> functionally identical to the existing /dev/urandom: it always returns
> *something* immediately, but it may or may not actually be
> cryptographically random or even random at all depending on system
> details.
Ok, if it is suggested that getrandom(2) should also have a mode to behave
exactly like /dev/urandom by not waiting until it is fully seeded, I am happy
to add that.
>
> In other words, my series simplifies the ABI that we support. Right
> now, we have three ways to ask for random numbers with different
> semantics and we need to have to RNGs in the kernel at all time. With
> my changes, we have only two ways to ask for random numbers, and the
> /dev/random pool is entirely gone.
Again, I do not want to stand in the way of changing the ABI if this is the
agreed way. All I want to say is that the LRNG seemingly is initialized much
faster than the existing /dev/random. If this is not fast enough for some
embedded environments, I would not want to stand in the way to make their life
easier.
>
> Would you be amenable to merging this into your series (i.e. either
> merging the code or just the ideas)?
Absolutely. I would be happy to do that.
Allow me to pull your code (I am currently behind a slow line) and review it
to see how best to integrate it.
> This would let you get rid of
> things like the compile-time selection of the blocking TRNG, since the
> blocking TRNG would be entirely gone.
Hm, I am not so sure we should do that.
Allow me to explain: I am also collaborating on the European side with the
German BSI. They love /dev/random as it is a "NTG.1" RNG based on their AIS 31
standard.
In order to seed a deterministic RNG (like OpenSSL, GnuTLS, etc. which are all
defined to be "DRG.3" or "DRG.2"), BSI mandates that the seed source is an
NTG.1.
By getting rid of the TRNG entirely and having /dev/random entirely behaving
like /dev/urandom or getrandom(2) without the GRND_RANDOM flag, the kernel
would "only" provide a "DRG.3" type RNG. This type of RNG would be disallowed
to seed another "DRG.3" or "DRG.2".
In plain English that means that for BSI's requirements, if the TRNG is gone
there would be no native seed source on Linux any more that can satisfy the
requirement. This is the ultimate reason why I made the TRNG compile-time
selectable: to support embedded systems but also support use cases like the
BSI case.
Please consider that I maintain a study over the last years for BSI trying to
ensure that the NTG.1 property is always met [1] [2]. The sole purpose of that
study is around this NTG.1.
>
> Or do you think that a kernel-provided blocking TRNG is a genuinely
> useful thing to keep around?
Yes, as I hope I explained it appropriately above, there are standardization
requirements that need the TRNG.
PS: When I was forwarding Linus' email on eliminating the blocking_pool to
BSI, I saw unhappy faces. :-)
I would like to help both sides here.
[1] https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/
LinuxRNG/NTG1_Kerneltabelle_EN.pdf?__blob=publicationFile&v=3
[2] https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/
LinuxRNG/NTG1_Kerneltabelle_EN.pdf?__blob=publicationFile&v=3
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v15 0/9] open: introduce openat2(2) syscall
From: Christian Brauner @ 2019-11-12 23:06 UTC (permalink / raw)
To: Kees Cook, Al Viro, Linus Torvalds
Cc: Aleksa Sarai, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells, Shuah Khan, Shuah Khan, Ingo Molnar,
Peter Zijlstra, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Jann Horn, Tycho Andersen, David Drysdale,
Chanho Min, Oleg Nesterov, Rasmus Villemoes, Alexander Shishkin,
Jiri
In-Reply-To: <201911121457.7D02692@keescook>
On Tue, Nov 12, 2019 at 03:01:26PM -0800, Kees Cook wrote:
> On Tue, Nov 12, 2019 at 12:24:04AM +1100, Aleksa Sarai wrote:
> > On 2019-11-05, Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > This patchset is being developed here:
> > > <https://github.com/cyphar/linux/tree/openat2/master>
> > >
> > > Patch changelog:
> > > v15:
> > > * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus Torvalds]
> > > * Split out patches for each individual LOOKUP flag.
> > > * Reword commit messages to give more background information about the
> > > series, as well as mention the semantics of each flag in more detail.
> > > [...]
> >
> > Ping -- this patch hasn't been touched for a week. Thanks.
>
> If I've been following correctly, everyone is happy with this series.
> (i.e. Linus's comment appear to have been addressed.)
>
> Perhaps the next question is should this go via a pull request by you to
> Linus directly during the v5.5 merge window, via akpm, via akpm, via
> Christian, or some other path? Besides Linus, it's not been clear who
> should "claim" this series. :)
I like this series and the same with the copy_struct_from_user() part of
it I've taken I'm happy to stuff this into a dedicated branch, merge it
into my for-next and send it for v5.5.
Though I'd _much_ rather see Al pick this up or have him give his
blessing first.
Christian
^ permalink raw reply
* Re: [PATCH v24 11/12] LRNG - add SP800-90B compliant health tests
From: Stephan Müller @ 2019-11-12 23:11 UTC (permalink / raw)
To: Alexander E. Patrakov
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, 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: <556c89ae-4272-970d-1644-cb77dc3c7946@gmail.com>
Am Dienstag, 12. November 2019, 20:58:32 CET schrieb Alexander E. Patrakov:
Hi Alexander,
> 11.11.2019 23:26, Stephan Müller пишет:
> > Implement health tests for LRNG's slow noise sources as mandated by
> > SP-800-90B The file contains the following health tests:
> >
> > - stuck test: The stuck test calculates the first, second and third
> >
> > discrete derivative of the time stamp to be processed by the LFSR.
> > Only if all three values are zero, the received time delta is
> > considered to be non-stuck.
>
> The other way round?
Opps, yes, of course. If all three a non-zero...
Fixed.
>
> > - SP800-90B Repetition Count Test (RCT): The LRNG uses an enhanced
> >
> > version of the RCT specified in SP800-90B section 4.4.1. Instead of
> > counting identical back-to-back values, the input to the RCT is the
> > counting of the stuck values during the processing of received
> > interrupt events. The RCT is applied with alpha=2^-30 compliant to
> > the recommendation of FIPS 140-2 IG 9.8. During the counting operation,
> > the LRNG always calculates the RCT cut-off value of C. If that value
> > exceeds the allowed cut-off value, the LRNG will trigger the health
> > test failure discussed below. An error is logged to the kernel log
> > that such RCT failure occurred. This test is only applied and
> > enforced in FIPS mode, i.e. when the kernel compiled with
> > CONFIG_CONFIG_FIPS is started with fips=1.
> >
> > - SP800-90B Adaptive Proportion Test (APT): The LRNG implements the
> >
> > APT as defined in SP800-90B section 4.4.2. The assumed
>
> The sentence ends in the middle.
Fixed:
"""
SP800-90B Adaptive Proportion Test (APT): The LRNG implements the
APT as defined in SP800-90B section 4.4.2. The applied significance
level again is alpha=2^-30 compliant to the recommendation of FIPS
140-2 IG 9.8.
"""
>
> > The aforementioned health tests are applied to the first 1,024 time stamps
> > obtained from interrupt events. In case one error is identified for either
> > the RCT, or the APT, the collected entropy is invalidated and the
> > SP800-90B startup health test is restarted.
> >
> > As long as the SP800-90B startup health test is not completed, all LRNG
> > random number output interfaces that may block will block and not generate
> > any data. This implies that only those potentially blocking interfaces are
> > defined to provide random numbers that are seeded with the interrupt noise
> > source being SP800-90B compliant. All other output interfaces will not be
> > affected by the SP800-90B startup test and thus are not considered
> > SP800-90B compliant.
> >
> > At runtime, the SP800-90B APT and RCT are applied to each time stamp
> > generated for a received interrupt. When either the APT and RCT indicates
> > a noise source failure, the LRNG is reset to a state it has immediately
> > after boot:
> >
> > - all entropy counters are set to zero
> >
> > - the SP800-90B startup tests are re-performed which implies that
> > getrandom(2) would block again until new entropy was collected
> >
> > To summarize, the following rules apply:
> >
> > • SP800-90B compliant output interfaces
> >
> > - /dev/random
> >
> > - getrandom(2) system call
> >
> > - get_random_bytes kernel-internal interface when being triggered by
> >
> > the callback registered with add_random_ready_callback
> >
> > • SP800-90B non-compliant output interfaces
> >
> > - /dev/urandom
> >
> > - get_random_bytes kernel-internal interface called directly
> >
> > - randomize_page kernel-internal interface
> >
> > - get_random_u32 and get_random_u64 kernel-internal interfaces
> >
> > - get_random_u32_wait, get_random_u64_wait, get_random_int_wait, and
> >
> > get_random_long_wait kernel-internal interfaces
> >
> > If either the RCT, or the APT health test fails irrespective whether
> >
> > during initialization or runtime, the following actions occur:
> > 1. The entropy of the entire entropy pool is invalidated.
> >
> > 2. The primary and all secondary DRNGs are reset which imply that they
> >
> > are treated as being not seeded and require a reseed during next
> > invocation.
> >
> > 3. The SP800-90B startup health test are initiated with all
> >
> > implications of the startup tests. That implies that from that point
> > on, new events must be observed and its entropy must be inserted
> > into
> > the entropy pool before random numbers are calculated from the
> > entropy pool.
> >
> > Further details on the SP800-90B compliance and the availability of all
> > test tools required to perform all tests mandated by SP800-90B are
> > provided at [1].
> >
> > The entire health testing code is compile-time configurable.
> >
> > 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 | 13 +
> > drivers/char/lrng/Makefile | 1 +
> > drivers/char/lrng/lrng_health.c | 424 ++++++++++++++++++++++++++++++++
> > 3 files changed, 438 insertions(+)
> > create mode 100644 drivers/char/lrng/lrng_health.c
> >
> > diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
> > index efc5f9aaa2a3..4373a1a19538 100644
> > --- a/drivers/char/lrng/Kconfig
> > +++ b/drivers/char/lrng/Kconfig
> > @@ -113,4 +113,17 @@ config LRNG_TRNG_SUPPORT
> >
> > With the TRNG support the /dev/random device will block
> > if insufficient entropy is available.
> >
> > +config LRNG_HEALTH_TESTS
> > + bool "Enable noise source online health tests"
> > + help
> > + The online health tests validate the noise source at
> > + runtime for fatal errors. These tests include SP800-90B
> > + compliant tests which are invoked if the system is booted
> > + with fips=1. In case of fatal errors during active
> > + SP800-90B tests, the issue is logged and the noise
> > + data is discarded. These tests are required for full
> > + compliance with SP800-90B.
>
> How have you tested that these tests work at runtime? Maybe add some
> code under a new CONFIG item that depends on CONFIG_BROKEN that
> deliberately botches the RNG and triggers failures?
I manually broke it for testing as follows: I set the LRNG_APT_CUTOFF to 1 to
trigger the APT failure and I changed the line
if (rct_count >= 30) {
to
if (rct_count >= 1) {
which effectively sets the RCT cutoff value to 1.
Allow me to check how CONFIG_BROKEN really works to see how I can roll this
test into CONFIG_BROKEN.
Thank you very much for your review.
>
> > +
> > + If unsure, say Y.
> > +
> >
> > endif # LRNG
> >
> > diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
> > index 1c72bc060bce..0713e9c0aa6e 100644
> > --- a/drivers/char/lrng/Makefile
> > +++ b/drivers/char/lrng/Makefile
> > @@ -15,3 +15,4 @@ obj-$(CONFIG_LRNG_DRBG) += lrng_drbg.o
> >
> > 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
> > diff --git a/drivers/char/lrng/lrng_health.c
> > b/drivers/char/lrng/lrng_health.c new file mode 100644
> > index 000000000000..b8d96cec1f71
> > --- /dev/null
> > +++ b/drivers/char/lrng/lrng_health.c
> > @@ -0,0 +1,424 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > +/*
> > + * Linux Random Number Generator (LRNG) Health Testing
> > + *
> > + * Copyright (C) 2019, Stephan Mueller <smueller@chronox.de>
> > + *
> > + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> > + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
> > + * WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
> > + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> > + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
> > + * DAMAGE.
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/fips.h>
> > +#include <linux/module.h>
> > +
> > +#include "lrng_internal.h"
> > +
> > +/* Stuck Test */
> > +struct lrng_stuck_test {
> > + u32 last_time; /* Stuck test: time of previous IRQ */
> > + u32 last_delta; /* Stuck test: delta of previous IRQ */
> > + u32 last_delta2; /* Stuck test: 2. time derivation of prev IRQ */
> > +};
> > +
> > +/* Repetition Count Test */
> > +struct lrng_rct {
> > + atomic_t rct_count; /* Number of stuck values */
> > +};
> > +
> > +/* Adaptive Proportion Test */
> > +struct lrng_apt {
> > + /* Taken from SP800-90B sec 4.4.2 - significance level 2^-30 */
> > +#define LRNG_APT_CUTOFF 325
> > + /* Data window size */
> > +#define LRNG_APT_WINDOW_SIZE 512
> > + /* LSB of time stamp to process */
> > +#define LRNG_APT_LSB 16
> > +#define LRNG_APT_WORD_MASK (LRNG_APT_LSB - 1)
> > + atomic_t apt_count; /* APT counter */
> > + atomic_t apt_base; /* APT base reference */
> > +
> > + atomic_t apt_trigger;
> > + bool apt_base_set; /* Is APT base set? */
> > +};
> > +
> > +/* The health test code must operate lock-less */
> > +struct lrng_health {
> > + struct lrng_rct rct;
> > + struct lrng_apt apt;
> > +
> > + bool health_test_enabled;
> > +
> > + /* SP800-90B startup health tests */
> > +#define LRNG_SP80090B_STARTUP_SAMPLES 1024
> > +#define LRNG_SP80090B_STARTUP_BLOCKS ((LRNG_SP80090B_STARTUP_SAMPLES +
> > \
> > + LRNG_APT_WINDOW_SIZE - 1) / \
> > + LRNG_APT_WINDOW_SIZE)
> > + bool sp80090b_startup_done;
> > + atomic_t sp80090b_startup_blocks;
> > +};
> > +
> > +static struct lrng_health lrng_health = {
> > + .rct.rct_count = ATOMIC_INIT(0),
> > +
> > + .apt.apt_count = ATOMIC_INIT(0),
> > + .apt.apt_base = ATOMIC_INIT(-1),
> > + .apt.apt_trigger = ATOMIC_INIT(LRNG_APT_WINDOW_SIZE),
> > + .apt.apt_base_set = false,
> > +
> > + .health_test_enabled = true,
> > +
> > + .sp80090b_startup_blocks = ATOMIC_INIT(LRNG_SP80090B_STARTUP_BLOCKS),
> > + .sp80090b_startup_done = false,
> > +};
> > +
> > +static DEFINE_PER_CPU(struct lrng_stuck_test, lrng_stuck_test);
> > +
> > +static inline bool lrng_sp80090b_health_requested(void)
> > +{
> > + /* Health tests are only requested in FIPS mode */
> > + return fips_enabled;
> > +}
> > +
> > +static inline bool lrng_sp80090b_health_enabled(void)
> > +{
> > + struct lrng_health *health = &lrng_health;
> > +
> > + return lrng_sp80090b_health_requested() && health->health_test_enabled;
> > +}
> > +
> > +/************************************************************************
> > *** + * SP800-90B Compliance
> > + *
> > + * If the Linux-RNG is booted into FIPS mode, the following interfaces
> > + * provide an SP800-90B compliant noise source:
> > + *
> > + * * /dev/random
> > + * * getrandom(2)
> > + * * get_random_bytes when using it in conjunction with
> > + * add_random_ready_callback
> > + *
> > + * All other interfaces, including /dev/urandom or get_random_bytes
> > without + * the add_random_ready_callback cannot claim to use an
> > SP800-90B compliant + * noise source.
> > +
> > *************************************************************************
> > **/ +
> > +/**
> > + * Perform SP800-90B startup testing
> > + */
> > +static inline void lrng_sp80090b_startup(struct lrng_health *health)
> > +{
> > + if (!health->sp80090b_startup_done &&
> > + atomic_dec_and_test(&health->sp80090b_startup_blocks)) {
> > + health->sp80090b_startup_done = true;
> > + pr_info("SP800-90B startup health tests completed\n");
> > + lrng_init_ops(0);
> > +
> > + /*
> > + * Force a reseed of secondary DRNGs to ensure they are
> > + * seeded with entropy that passed the SP800-90B health tests.
> > + * As the primary DRNG always will reseed before generating
> > + * random numbers, it does not need a reseed trigger.
> > + */
> > + lrng_sdrng_force_reseed();
> > + }
> > +}
> > +
> > +/**
> > + * Handle failure of SP800-90B startup testing
> > + */
> > +static inline void lrng_sp80090b_startup_failure(struct lrng_health
> > *health) +{
> > + /* Reset of LRNG and its entropy - NOTE: we are in atomic context */
> > + lrng_reset();
> > +
> > + /*
> > + * Reset the SP800-90B startup test.
> > + *
> > + * NOTE SP800-90B section 4.3 bullet 4 does not specify what
> > + * exactly is to be done in case of failure! Thus, we do what
> > + * makes sense, i.e. restarting the health test and thus gating
> > + * the output function of /dev/random and getrandom(2).
> > + */
> > + atomic_set(&health->sp80090b_startup_blocks,
> > + LRNG_SP80090B_STARTUP_BLOCKS);
> > +}
> > +
> > +/**
> > + * Handle failure of SP800-90B runtime testing
> > + */
> > +static inline void lrng_sp80090b_runtime_failure(struct lrng_health
> > *health) +{
> > + lrng_sp80090b_startup_failure(health);
> > + health->sp80090b_startup_done = false;
> > +}
> > +
> > +static inline void lrng_sp80090b_failure(struct lrng_health *health)
> > +{
> > + if (health->sp80090b_startup_done) {
> > + pr_err("SP800-90B runtime health test failure - invalidating "
> > + "all existing entropy and initiate SP800-90B startup\n");
> > + lrng_sp80090b_runtime_failure(health);
> > + } else {
> > + pr_err("SP800-90B startup test failure - resetting\n");
> > + lrng_sp80090b_startup_failure(health);
> > + }
> > +}
> > +
> > +/**
> > + * Is the SP800-90B startup testing complete?
> > + *
> > + * This function is called by the LRNG to determine whether to unblock
> > + * a certain user interface. Therefore, only the potentially blocking
> > + * user interfaces are considered SP800-90B compliant.
> > + */
> > +bool lrng_sp80090b_startup_complete(void)
> > +{
> > + struct lrng_health *health = &lrng_health;
> > +
> > + return (lrng_sp80090b_health_enabled()) ? health->sp80090b_startup_done:
> > + true;
> > +}
> > +
> > +bool lrng_sp80090b_compliant(void)
> > +{
> > + struct lrng_health *health = &lrng_health;
> > +
> > + return lrng_sp80090b_health_enabled() && health->sp80090b_startup_done;
> > +}
> > +
> > +/************************************************************************
> > *** + * Adaptive Proportion Test
> > + *
> > + * This test complies with SP800-90B section 4.4.2.
> > +
> > *************************************************************************
> > **/ +
> > +/**
> > + * Reset the APT counter
> > + *
> > + * @health [in] Reference to health state
> > + */
> > +static inline void lrng_apt_reset(struct lrng_health *health,
> > + unsigned int time_masked)
> > +{
> > + struct lrng_apt *apt = &health->apt;
> > +
> > + pr_debug("APT value %d for base %d\n",
> > + atomic_read(&apt->apt_count), atomic_read(&apt->apt_base));
> > +
> > + /* Reset APT */
> > + atomic_set(&apt->apt_count, 0);
> > + atomic_set(&apt->apt_base, time_masked);
> > +}
> > +
> > +static inline void lrng_apt_restart(struct lrng_health *health)
> > +{
> > + struct lrng_apt *apt = &health->apt;
> > +
> > + atomic_set(&apt->apt_trigger, LRNG_APT_WINDOW_SIZE);
> > +}
> > +
> > +/**
> > + * Insert a new entropy event into APT
> > + *
> > + * This function does is void as it does not decide about the fate of a
> > time + * stamp. An APT failure can only happen at the same time of a
> > stuck test + * failure. Thus, the stuck failure will already decide how
> > the time stamp + * is handled.
> > + *
> > + * @health [in] Reference to health state
> > + * @now_time [in] Time stamp to process
> > + */
> > +static inline void lrng_apt_insert(struct lrng_health *health,
> > + unsigned int now_time)
> > +{
> > + struct lrng_apt *apt = &health->apt;
> > +
> > + if (!lrng_sp80090b_health_requested())
> > + return;
> > +
> > + now_time &= LRNG_APT_WORD_MASK;
> > +
> > + /* Initialization of APT */
> > + if (!apt->apt_base_set) {
> > + atomic_set(&apt->apt_base, now_time);
> > + apt->apt_base_set = true;
> > + return;
> > + }
> > +
> > + if (now_time == (unsigned int)atomic_read(&apt->apt_base)) {
> > + u32 apt_val = (u32)atomic_inc_return_relaxed(&apt->apt_count);
> > +
> > + if (apt_val >= LRNG_APT_CUTOFF)
> > + lrng_sp80090b_failure(health);
> > + }
> > +
> > + if (atomic_dec_and_test(&apt->apt_trigger)) {
> > + lrng_apt_restart(health);
> > + lrng_apt_reset(health, now_time);
> > + lrng_sp80090b_startup(health);
> > + }
> > +}
> > +
> > +/************************************************************************
> > *** + * Repetition Count Test
> > + *
> > + * The LRNG uses an enhanced version of the Repetition Count Test
> > + * (RCT) specified in SP800-90B section 4.4.1. Instead of counting
> > identical + * back-to-back values, the input to the RCT is the counting
> > of the stuck + * values while filling the entropy pool.
> > + *
> > + * The RCT is applied with an alpha of 2^-30 compliant to FIPS 140-2 IG
> > 9.8. + *
> > + * During the counting operation, the LRNG always calculates the RCT
> > + * cut-off value of C. If that value exceeds the allowed cut-off value,
> > + * the LRNG will invalidate all entropy for the entropy pool which
> > implies
> > + * that no data can be extracted from the entropy pool unless new entropy
> > + * is received.
> > +
> > *************************************************************************
> > **/ +
> > +/**
> > + * Hot code path - Insert data for Repetition Count Test
> > + *
> > + * @health: Reference to health information
> > + * @stuck: Decision of stuck test
> > + */
> > +static inline void lrng_rct(struct lrng_health *health, int stuck)
> > +{
> > + struct lrng_rct *rct = &health->rct;
> > +
> > + if (!lrng_sp80090b_health_requested())
> > + return;
> > +
> > + if (stuck) {
> > + u32 rct_count = atomic_add_return_relaxed(1, &rct->rct_count);
> > +
> > + pr_debug("RCT count: %u\n", rct_count);
> > +
> > + /*
> > + * The cutoff value is based on the following consideration:
> > + * alpha = 2^-30 as recommended in FIPS 140-2 IG 9.8.
> > + * In addition, we imply an entropy value H of 1 bit as this
> > + * is the minimum entropy required to provide full entropy.
> > + *
> > + * Note, rct_count (which equals to value B in the
> > + * pseudo code of SP800-90B section 4.4.1) starts with zero.
> > + * Hence we need to subtract one from the cutoff value as
> > + * calculated following SP800-90B.
> > + */
> > + if (rct_count >= 30) {
> > + atomic_set(&rct->rct_count, 0);
> > +
> > + /*
> > + * APT must start anew as we consider all previously
> > + * recorded data to contain no entropy.
> > + */
> > + lrng_apt_restart(health);
> > +
> > + lrng_sp80090b_failure(health);
> > + }
> > + } else {
> > + atomic_set(&rct->rct_count, 0);
> > + }
> > +}
> > +
> > +/************************************************************************
> > *** + * Stuck Test
> > + *
> > + * Checking the:
> > + * 1st derivative of the event occurrence (time delta)
> > + * 2nd derivative of the event occurrence (delta of time deltas)
> > + * 3rd derivative of the event occurrence (delta of delta of time
> > deltas) + *
> > + * All values must always be non-zero. The stuck test is only valid
> > disabled if + * high-resolution time stamps are identified after
> > initialization. +
> > *************************************************************************
> > **/ +
> > +static inline u32 lrng_delta(u32 prev, u32 next)
> > +{
> > + /*
> > + * Note that this (unsigned) subtraction does yield the correct value
> > + * in the wraparound-case, i.e. when next < prev.
> > + */
> > + return (next - prev);
> > +}
> > +
> > +/**
> > + * Hot code path
> > + *
> > + * @health: Reference to health information
> > + * @now: Event time
> > + * @return: 0 event occurrence not stuck (good time stamp)
> > + * != 0 event occurrence stuck (reject time stamp)
> > + */
> > +static inline int lrng_irq_stuck(struct lrng_stuck_test *stuck, u32
> > now_time) +{
> > + u32 delta = lrng_delta(stuck->last_time, now_time);
> > + u32 delta2 = lrng_delta(stuck->last_delta, delta);
> > + u32 delta3 = lrng_delta(stuck->last_delta2, delta2);
> > +
> > + stuck->last_time = now_time;
> > + stuck->last_delta = delta;
> > + stuck->last_delta2 = delta2;
> > +
> > + if (!delta || !delta2 || !delta3)
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +/************************************************************************
> > *** + * Health test interfaces
> > +
> > *************************************************************************
> > **/ +
> > +/**
> > + * Disable all health tests
> > + */
> > +void lrng_health_disable(void)
> > +{
> > + struct lrng_health *health = &lrng_health;
> > +
> > + health->health_test_enabled = false;
> > +
> > + if (lrng_sp80090b_health_requested())
> > + pr_warn("SP800-90B compliance requested but the Linux RNG is "
> > + "NOT SP800-90B compliant\n");
> > +}
> > +
> > +/**
> > + * Hot code path - Perform health test on time stamp received from an
> > event + *
> > + * @now_time Time stap
> > + */
> > +enum lrng_health_res lrng_health_test(u32 now_time)
> > +{
> > + struct lrng_health *health = &lrng_health;
> > + struct lrng_stuck_test *stuck_test = this_cpu_ptr(&lrng_stuck_test);
> > + int stuck;
> > +
> > + if (!health->health_test_enabled)
> > + return lrng_health_pass;
> > +
> > + lrng_apt_insert(health, now_time);
> > +
> > + stuck = lrng_irq_stuck(stuck_test, now_time);
> > + lrng_rct(health, stuck);
> > + if (stuck) {
> > + /* SP800-90B disallows using a failing health test time stamp */
> > + return lrng_sp80090b_health_requested() ?
> > + lrng_health_fail_drop : lrng_health_fail_use;
> > + }
> > +
> > + return lrng_health_pass;
> > +}
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v24 12/12] LRNG - add interface for gathering of raw entropy
From: Stephan Müller @ 2019-11-12 23:13 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, 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, Ni
In-Reply-To: <201911130440.pXLcGJwR%lkp@intel.com>
Am Dienstag, 12. November 2019, 21:55:10 CET schrieb kbuild test robot:
Hi kbuild,
> 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 v5.4-rc7 next-20191111]
> [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-compliance/20191113-040847 base:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> 01b59c763fe2de845b65900485b141fdd7bbf93e config: sh-allmodconfig (attached
> as .config)
> compiler: sh4-linux-gcc (GCC) 7.4.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=sh
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/char/lrng/lrng_testing.c: In function 'lrng_raw_extract_user':
> >> drivers/char/lrng/lrng_testing.c:237:1: warning: the frame size of 1076
> >> bytes is larger than 1024 bytes [-Wframe-larger-than=]
> }
> ^
>
> vim +237 drivers/char/lrng/lrng_testing.c
>
> 196
>
> 197 /**********************************************************************
> **** 198 * Debugfs interface
> 199
> **************************************************************************/
> 200 static int lrng_raw_extract_user(void __user *buf, size_t nbytes) 201 {
> 202 u8 tmp[LRNG_TESTING_RINGBUFFER_SIZE]
__aligned(sizeof(u32));
This is the offending line.
I will need to kzalloc the code here.
Thank you.
> 203 int ret = 0, large_request = (nbytes > 256);
> 204
> 205 while (nbytes) {
> 206 int i;
> 207
> 208 if (large_request && need_resched()) {
> 209 if (signal_pending(current)) {
> 210 if (ret == 0)
> 211 ret = -ERESTARTSYS;
> 212 break;
> 213 }
> 214 schedule();
> 215 }
> 216
> 217 i = min_t(int, nbytes, sizeof(tmp));
> 218 i = lrng_raw_entropy_reader(tmp, i);
> 219 if (i <= 0) {
> 220 if (i < 0)
> 221 ret = i;
> 222 break;
> 223 }
> 224 if (copy_to_user(buf, tmp, i)) {
> 225 ret = -EFAULT;
> 226 break;
> 227 }
> 228
> 229 nbytes -= i;
> 230 buf = (u8 *)buf + i;
> 231 ret += i;
> 232 }
> 233
> 234 memzero_explicit(tmp, sizeof(tmp));
> 235
> 236 return ret;
>
> > 237 }
>
> 238
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology
> Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel
> Corporation
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v24 01/12] Linux Random Number Generator
From: Stephan Müller @ 2019-11-12 23:15 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, 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, Ni
In-Reply-To: <201911130648.YIqEERFA%lkp@intel.com>
Am Dienstag, 12. November 2019, 23:30:29 CET schrieb kbuild test robot:
Hi kbuild,
> Hi "Stephan,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on char-misc/char-misc-testing]
> [also build test ERROR on v5.4-rc7 next-20191112]
> [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-compliance/20191113-040847 base:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> 01b59c763fe2de845b65900485b141fdd7bbf93e config:
> i386-randconfig-f003-201945 (attached as .config)
> compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> In file included from <command-line>:0:0:
>
> include/linux/lrng.h: In function 'lrng_set_drng_cb':
> >> include/linux/lrng.h:80:61: error: 'EOPNOTSUPP' undeclared (first use in
> >> this function)
I need to include errno.h in lrng.h.
Thank you, it will be fixed in the next installment.
> lrng_set_drng_cb(const struct lrng_crypto_cb *cb) { return -EOPNOTSUPP;
> } ^~~~~~~~~~ include/linux/lrng.h:80:61: note: each undeclared identifier
> is reported only once for each function it appears in
>
> vim +/EOPNOTSUPP +80 include/linux/lrng.h
>
> 74
> 75 /* Register cryptographic backend */
> 76 #ifdef CONFIG_LRNG_DRNG_SWITCH
> 77 int lrng_set_drng_cb(const struct lrng_crypto_cb *cb);
> 78 #else /* CONFIG_LRNG_DRNG_SWITCH */
> 79 static inline int
>
> > 80 lrng_set_drng_cb(const struct lrng_crypto_cb *cb) { return
> > -EOPNOTSUPP; }
> 81 #endif /* CONFIG_LRNG_DRNG_SWITCH */
> 82
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology
> Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel
> Corporation
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH] Allow restricting permissions in /proc/sys
From: Christian Brauner @ 2019-11-12 23:22 UTC (permalink / raw)
To: Topi Miettinen
Cc: Luis Chamberlain, Kees Cook, Alexey Dobriyan,
linux-kernel@vger.kernel.org,
open list:FILESYSTEMS (VFS and infrastructure), linux-api
In-Reply-To: <74a91362-247c-c749-5200-7bdce704ed9e@gmail.com>
[Cc+ linux-api@vger.kernel.org]
since that's potentially relevant to quite a few people.
On Sun, Nov 03, 2019 at 04:55:48PM +0200, Topi Miettinen wrote:
> Several items in /proc/sys need not be accessible to unprivileged
> tasks. Let the system administrator change the permissions, but only
> to more restrictive modes than what the sysctl tables allow.
>
> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
> ---
> fs/proc/proc_sysctl.c | 41 +++++++++++++++++++++++++++++++----------
> 1 file changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index d80989b6c344..88c4ca7d2782 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -818,6 +818,10 @@ static int proc_sys_permission(struct inode *inode, int
> mask)
> if ((mask & MAY_EXEC) && S_ISREG(inode->i_mode))
> return -EACCES;
>
> + error = generic_permission(inode, mask);
> + if (error)
> + return error;
> +
> head = grab_header(inode);
> if (IS_ERR(head))
> return PTR_ERR(head);
> @@ -837,9 +841,35 @@ static int proc_sys_setattr(struct dentry *dentry,
> struct iattr *attr)
> struct inode *inode = d_inode(dentry);
> int error;
>
> - if (attr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
> + if (attr->ia_valid & (ATTR_UID | ATTR_GID))
> return -EPERM;
>
> + if (attr->ia_valid & ATTR_MODE) {
> + struct ctl_table_header *head = grab_header(inode);
> + struct ctl_table *table = PROC_I(inode)->sysctl_entry;
> + umode_t max_mode = 0777; /* Only these bits may change */
> +
> + if (IS_ERR(head))
> + return PTR_ERR(head);
> +
> + if (!table) /* global root - r-xr-xr-x */
> + max_mode &= ~0222;
> + else /*
> + * Don't allow permissions to become less
> + * restrictive than the sysctl table entry
> + */
> + max_mode &= table->mode;
> +
> + sysctl_head_finish(head);
> +
> + /* Execute bits only allowed for directories */
> + if (!S_ISDIR(inode->i_mode))
> + max_mode &= ~0111;
> +
> + if (attr->ia_mode & ~S_IFMT & ~max_mode)
> + return -EPERM;
> + }
> +
> error = setattr_prepare(dentry, attr);
> if (error)
> return error;
> @@ -853,17 +883,8 @@ static int proc_sys_getattr(const struct path *path,
> struct kstat *stat,
> u32 request_mask, unsigned int query_flags)
> {
> struct inode *inode = d_inode(path->dentry);
> - struct ctl_table_header *head = grab_header(inode);
> - struct ctl_table *table = PROC_I(inode)->sysctl_entry;
> -
> - if (IS_ERR(head))
> - return PTR_ERR(head);
>
> generic_fillattr(inode, stat);
> - if (table)
> - stat->mode = (stat->mode & S_IFMT) | table->mode;
> -
> - sysctl_head_finish(head);
> return 0;
> }
>
> --
> 2.24.0.rc1
>
^ permalink raw reply
* Re: [PATCH v24 00/12] /dev/random - a new approach with full SP800-90B compliance
From: Stephan Müller @ 2019-11-12 23:26 UTC (permalink / raw)
To: Stephan Müller
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: <3282061.iY3hP4IT6m@positron.chronox.de>
Am Mittwoch, 13. November 2019, 00:03:47 CET schrieb Stephan Müller:
Hi Stephan,
> Am Dienstag, 12. November 2019, 16:33:59 CET schrieb Andy Lutomirski:
>
> Hi Andy,
>
> > On Mon, Nov 11, 2019 at 11:13 AM Stephan Müller <smueller@chronox.de>
wrote:
> > > The following patch set provides a different approach to /dev/random
> > > which
> > > is called Linux Random Number Generator (LRNG) to collect entropy within
> > > the Linux kernel. The main improvements compared to the existing
> > > /dev/random is to provide sufficient entropy during boot time as well as
> > > in virtual environments and when using SSDs. A secondary design goal is
> > > to limit the impact of the entropy collection on massive parallel
> > > systems
> > > and also allow the use accelerated cryptographic primitives. Also, all
> > > steps of the entropic data processing are testable.
> >
> > This is very nice!
> >
> > > The LRNG patch set allows a user to select use of the existing
> > > /dev/random
> > > or the LRNG during compile time. As the LRNG provides API and ABI
> > > compatible interfaces to the existing /dev/random implementation, the
> > > user can freely chose the RNG implementation without affecting kernel or
> > > user space operations.
> > >
> > > This patch set provides early boot-time entropy which implies that no
> > > additional flags to the getrandom(2) system call discussed recently on
> > > the LKML is considered to be necessary.
> >
> > I'm uneasy about this. I fully believe that, *on x86*, this works.
> > But on embedded systems with in-order CPUs, a single clock, and very
> > lightweight boot processes, most or all of boot might be too
> > deterministic for this to work.
>
> I agree that in such cases, my LRNG getrandom(2) would also block until the
> LRNG thinks it collected 256 bits of entropy. However, I am under the
> impression that the LRNG collects that entropy faster that the existing
> /dev/ random implementation, even in this case.
>
> Nicolai is copied on this thread. He promised to have the LRNG tested on
> such a minimalistic system that you describe. I hope he could contribute
> some numbers from that test helping us to understand how much of a problem
> we face.
> > I have a somewhat competing patch set here:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=rand
> > om /kill-it
> >
> > (Ignore the "horrible test hack" and the debugfs part.)
> >
> > The basic summary is that I change /dev/random so that it becomes
> > functionally identical to getrandom(..., 0) -- in other words, it
> > blocks until the CRNG is initialized but is then identical to
> > /dev/urandom.
>
> This would be equal to the LRNG code without compiling the TRNG.
>
> > And I add getrandom(...., GRND_INSECURE) that is
> > functionally identical to the existing /dev/urandom: it always returns
> > *something* immediately, but it may or may not actually be
> > cryptographically random or even random at all depending on system
> > details.
>
> Ok, if it is suggested that getrandom(2) should also have a mode to behave
> exactly like /dev/urandom by not waiting until it is fully seeded, I am
> happy to add that.
>
> > In other words, my series simplifies the ABI that we support. Right
> > now, we have three ways to ask for random numbers with different
> > semantics and we need to have to RNGs in the kernel at all time. With
> > my changes, we have only two ways to ask for random numbers, and the
> > /dev/random pool is entirely gone.
>
> Again, I do not want to stand in the way of changing the ABI if this is the
> agreed way. All I want to say is that the LRNG seemingly is initialized much
> faster than the existing /dev/random. If this is not fast enough for some
> embedded environments, I would not want to stand in the way to make their
> life easier.
>
> > Would you be amenable to merging this into your series (i.e. either
> > merging the code or just the ideas)?
>
> Absolutely. I would be happy to do that.
>
> Allow me to pull your code (I am currently behind a slow line) and review it
> to see how best to integrate it.
>
> > This would let you get rid of
> > things like the compile-time selection of the blocking TRNG, since the
> > blocking TRNG would be entirely gone.
>
> Hm, I am not so sure we should do that.
>
> Allow me to explain: I am also collaborating on the European side with the
> German BSI. They love /dev/random as it is a "NTG.1" RNG based on their AIS
> 31 standard.
>
> In order to seed a deterministic RNG (like OpenSSL, GnuTLS, etc. which are
> all defined to be "DRG.3" or "DRG.2"), BSI mandates that the seed source is
> an NTG.1.
>
> By getting rid of the TRNG entirely and having /dev/random entirely behaving
> like /dev/urandom or getrandom(2) without the GRND_RANDOM flag, the kernel
> would "only" provide a "DRG.3" type RNG. This type of RNG would be
> disallowed to seed another "DRG.3" or "DRG.2".
>
> In plain English that means that for BSI's requirements, if the TRNG is gone
> there would be no native seed source on Linux any more that can satisfy the
> requirement. This is the ultimate reason why I made the TRNG compile-time
> selectable: to support embedded systems but also support use cases like the
> BSI case.
>
> Please consider that I maintain a study over the last years for BSI trying
> to ensure that the NTG.1 property is always met [1] [2]. The sole purpose
> of that study is around this NTG.1.
>
> > Or do you think that a kernel-provided blocking TRNG is a genuinely
> > useful thing to keep around?
>
> Yes, as I hope I explained it appropriately above, there are standardization
> requirements that need the TRNG.
>
> PS: When I was forwarding Linus' email on eliminating the blocking_pool to
> BSI, I saw unhappy faces. :-)
>
> I would like to help both sides here.
>
> [1]
> https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/
> LinuxRNG/NTG1_Kerneltabelle_EN.pdf?__blob=publicationFile&v=3
>
> [2]
> https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/
> LinuxRNG/NTG1_Kerneltabelle_EN.pdf?__blob=publicationFile&v=3
Sorry, the copy did not work:
[2] https://bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/Studies/
LinuxRNG/LinuxRNG_EN.pdf?__blob=publicationFile&v=16
>
> Ciao
> Stephan
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v24 01/12] Linux Random Number Generator
From: kbuild test robot @ 2019-11-13 0:14 UTC (permalink / raw)
To: Stephan Müller
Cc: kbuild-all, 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, Ni
In-Reply-To: <2369119.jSEA3qhmGI@positron.chronox.de>
[-- Attachment #1: Type: text/plain, Size: 2677 bytes --]
Hi "Stephan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.4-rc7 next-20191112]
[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-compliance/20191113-040847
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 01b59c763fe2de845b65900485b141fdd7bbf93e
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/char/lrng/lrng_sw_noise.c: In function 'add_interrupt_randomness':
>> drivers/char/lrng/lrng_sw_noise.c:145:23: error: invalid application of 'sizeof' to incomplete type 'struct pt_regs'
size_t n = (sizeof(struct pt_regs) / sizeof(u32));
^~~~~~
>> drivers/char/lrng/lrng_sw_noise.c:147:9: error: implicit declaration of function 'instruction_pointer'; did you mean 'instruction_hazard'? [-Werror=implicit-function-declaration]
ip = instruction_pointer(regs);
^~~~~~~~~~~~~~~~~~~
instruction_hazard
cc1: some warnings being treated as errors
vim +145 drivers/char/lrng/lrng_sw_noise.c
125
126 /**
127 * Hot code path - Callback for interrupt handler
128 */
129 void add_interrupt_randomness(int irq, int irq_flags)
130 {
131 lrng_time_process();
132
133 if (!lrng_pool_highres_timer()) {
134 struct pt_regs *regs = get_irq_regs();
135 static atomic_t reg_idx = ATOMIC_INIT(0);
136 u64 ip;
137
138 lrng_pool_lfsr_u32(jiffies);
139 lrng_pool_lfsr_u32(irq);
140 lrng_pool_lfsr_u32(irq_flags);
141
142 if (regs) {
143 u32 *ptr = (u32 *)regs;
144 int reg_ptr = atomic_add_return_relaxed(1, ®_idx);
> 145 size_t n = (sizeof(struct pt_regs) / sizeof(u32));
146
> 147 ip = instruction_pointer(regs);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62104 bytes --]
^ permalink raw reply
* Re: [PATCH v24 01/12] Linux Random Number Generator
From: Stephan Müller @ 2019-11-13 0:25 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, 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, Ni
In-Reply-To: <201911130807.RnsJ5SVf%lkp@intel.com>
Am Mittwoch, 13. November 2019, 01:14:05 CET schrieb kbuild test robot:
Hi kbuild,
> Hi "Stephan,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on char-misc/char-misc-testing]
> [also build test ERROR on v5.4-rc7 next-20191112]
> [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-compliance/20191113-040847 base:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> 01b59c763fe2de845b65900485b141fdd7bbf93e config: mips-allmodconfig
> (attached as .config)
> compiler: mips-linux-gcc (GCC) 7.4.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=mips
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> drivers/char/lrng/lrng_sw_noise.c: In function
'add_interrupt_randomness':
> >> drivers/char/lrng/lrng_sw_noise.c:145:23: error: invalid application of
> >> 'sizeof' to incomplete type 'struct pt_regs'
> size_t n = (sizeof(struct pt_regs) / sizeof(u32));
> ^~~~~~
>
> >> drivers/char/lrng/lrng_sw_noise.c:147:9: error: implicit declaration of
> >> function 'instruction_pointer'; did you mean 'instruction_hazard'?
> >> [-Werror=implicit-function-declaration]
> ip = instruction_pointer(regs);
> ^~~~~~~~~~~~~~~~~~~
> instruction_hazard
> cc1: some warnings being treated as errors
>
> vim +145 drivers/char/lrng/lrng_sw_noise.c
Thank you for the report.
Both issues are fixed by including <asm/ptrace.h>.
This will be fixed with the next installment of the patch.
>
> 125
> 126 /**
> 127 * Hot code path - Callback for interrupt handler
> 128 */
> 129 void add_interrupt_randomness(int irq, int irq_flags)
> 130 {
> 131 lrng_time_process();
> 132
> 133 if (!lrng_pool_highres_timer()) {
> 134 struct pt_regs *regs = get_irq_regs();
> 135 static atomic_t reg_idx = ATOMIC_INIT(0);
> 136 u64 ip;
> 137
> 138 lrng_pool_lfsr_u32(jiffies);
> 139 lrng_pool_lfsr_u32(irq);
> 140 lrng_pool_lfsr_u32(irq_flags);
> 141
> 142 if (regs) {
> 143 u32 *ptr = (u32 *)regs;
> 144 int reg_ptr = atomic_add_return_relaxed(1,
®_idx);
>
> > 145 size_t n = (sizeof(struct pt_regs) /
sizeof(u32));
>
> 146
>
> > 147 ip = instruction_pointer(regs);
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology
> Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel
> Corporation
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v24 11/12] LRNG - add SP800-90B compliant health tests
From: Stephan Müller @ 2019-11-13 0:36 UTC (permalink / raw)
To: Alexander E. Patrakov
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, 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: <556c89ae-4272-970d-1644-cb77dc3c7946@gmail.com>
Am Dienstag, 12. November 2019, 20:58:32 CET schrieb Alexander E. Patrakov:
Hi Alexander,
> > +config LRNG_HEALTH_TESTS
> > + bool "Enable noise source online health tests"
> > + help
> > + The online health tests validate the noise source at
> > + runtime for fatal errors. These tests include SP800-90B
> > + compliant tests which are invoked if the system is booted
> > + with fips=1. In case of fatal errors during active
> > + SP800-90B tests, the issue is logged and the noise
> > + data is discarded. These tests are required for full
> > + compliance with SP800-90B.
>
> How have you tested that these tests work at runtime? Maybe add some
> code under a new CONFIG item that depends on CONFIG_BROKEN that
> deliberately botches the RNG and triggers failures?
I am unable to find sensible information about CONFIG_BROKEN in the recent
kernel tree.
Do you happen to have a pointer on how that option is to be used?
Thanks a lot
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v15 0/9] open: introduce openat2(2) syscall
From: Aleksa Sarai @ 2019-11-13 0:46 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells, Shuah Khan, Shuah Khan, Ingo Molnar,
Peter Zijlstra, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Jann Horn, Tycho Andersen, David Drysdale,
Chanho Min, Oleg Nesterov, Rasmus Villemoes, Alexander Shishkin
In-Reply-To: <201911121457.7D02692@keescook>
[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]
On 2019-11-12, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Nov 12, 2019 at 12:24:04AM +1100, Aleksa Sarai wrote:
> > On 2019-11-05, Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > This patchset is being developed here:
> > > <https://github.com/cyphar/linux/tree/openat2/master>
> > >
> > > Patch changelog:
> > > v15:
> > > * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus Torvalds]
> > > * Split out patches for each individual LOOKUP flag.
> > > * Reword commit messages to give more background information about the
> > > series, as well as mention the semantics of each flag in more detail.
> > > [...]
> >
> > Ping -- this patch hasn't been touched for a week. Thanks.
>
> If I've been following correctly, everyone is happy with this series.
> (i.e. Linus's comment appear to have been addressed.)
>
> Perhaps the next question is should this go via a pull request by you to
> Linus directly during the v5.5 merge window, via akpm, via akpm, via
> Christian, or some other path? Besides Linus, it's not been clear who
> should "claim" this series. :)
Given the namei changes, I wanted to avoid stepping on Al's toes. Though
he did review the series a few versions ago, the discussion didn't focus
on the openat2(2) semantics (which have also changed since then). I'm
not sure whether to interpret the silence to mean he's satisfied with
things as they are, or if he hasn't had more time to look at the series.
As for which tree it should be routed to, I don't mind -- Christian is
the most straight-forward choice (but if Al wants to route it, that's
fine with me too).
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v15 2/9] namei: LOOKUP_NO_MAGICLINKS: block magic-link resolution
From: Al Viro @ 2019-11-13 1:24 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, David Drysdale, Andy Lutomirski,
Linus Torvalds, Eric Biederman, Andrew Morton, Alexei Starovoitov,
Kees Cook, Jann Horn, Tycho Andersen, Chanho Min, Oleg Nesterov,
Ras
In-Reply-To: <20191105090553.6350-3-cyphar@cyphar.com>
On Tue, Nov 05, 2019 at 08:05:46PM +1100, Aleksa Sarai wrote:
> @@ -1078,6 +1079,10 @@ const char *get_link(struct nameidata *nd)
> } else {
> res = get(dentry, inode, &last->done);
> }
> + if (nd->flags & LOOKUP_MAGICLINK_JUMPED) {
> + if (unlikely(nd->flags & LOOKUP_NO_MAGICLINKS))
> + return ERR_PTR(-ELOOP);
> + }
Minor nit - the first check probably wants unlikely() more than the
second one; it's probably noise anyway, but most of the symlinks
traversed are not going to be procfs ones, so you get test + branch
taken most of the time.
OTOH, that just might compile into
fetch nd->flags
and with LOOKUP_MAGICLINK_JUMPED | LOOKUP_NO_MAGICLINKS
compare with the same constant
unlikely branch when equal
Anyway, that's no more than a minor nit and can be dealt with later (if
at all)
^ permalink raw reply
* Re: [PATCH v15 3/9] namei: LOOKUP_NO_XDEV: block mountpoint crossing
From: Al Viro @ 2019-11-13 1:36 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, David Drysdale, Andy Lutomirski,
Linus Torvalds, Eric Biederman, Andrew Morton, Alexei Starovoitov,
Kees Cook, Jann Horn, Tycho Andersen, Chanho Min, Oleg Nesterov,
Ras
In-Reply-To: <20191105090553.6350-4-cyphar@cyphar.com>
On Tue, Nov 05, 2019 at 08:05:47PM +1100, Aleksa Sarai wrote:
> @@ -862,6 +870,8 @@ static int nd_jump_root(struct nameidata *nd)
> void nd_jump_link(struct path *path)
> {
> struct nameidata *nd = current->nameidata;
> +
> + nd->last_magiclink.same_mnt = (nd->path.mnt == path->mnt);
> path_put(&nd->path);
>
> nd->path = *path;
> @@ -1082,6 +1092,10 @@ const char *get_link(struct nameidata *nd)
> if (nd->flags & LOOKUP_MAGICLINK_JUMPED) {
> if (unlikely(nd->flags & LOOKUP_NO_MAGICLINKS))
> return ERR_PTR(-ELOOP);
> + if (unlikely(nd->flags & LOOKUP_NO_XDEV)) {
> + if (!nd->last_magiclink.same_mnt)
> + return ERR_PTR(-EXDEV);
> + }
> }
Ugh... Wouldn't it be better to take that logics (some equivalent thereof)
into nd_jump_link()? Or just have nd_jump_link() return an error...
I mean, look at the callers of nd_jump_link().
static const char *policy_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
struct aa_ns *ns;
struct path path;
if (!dentry)
return ERR_PTR(-ECHILD);
ns = aa_get_current_ns();
path.mnt = mntget(aafs_mnt);
path.dentry = dget(ns_dir(ns));
nd_jump_link(&path);
aa_put_ns(ns);
return NULL;
}
- very close to the end of ->get_link() instance.
static const char *proc_pid_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
struct path path;
int error = -EACCES;
if (!dentry)
return ERR_PTR(-ECHILD);
/* Are we allowed to snoop on the tasks file descriptors? */
if (!proc_fd_access_allowed(inode))
goto out;
error = PROC_I(inode)->op.proc_get_link(dentry, &path);
if (error)
goto out;
nd_jump_link(&path);
return NULL;
out:
return ERR_PTR(error);
}
Ditto.
static const char *proc_ns_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops;
struct task_struct *task;
struct path ns_path;
void *error = ERR_PTR(-EACCES);
if (!dentry)
return ERR_PTR(-ECHILD);
task = get_proc_task(inode);
if (!task)
return error;
if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) {
error = ns_get_path(&ns_path, task, ns_ops);
if (!error)
nd_jump_link(&ns_path);
}
put_task_struct(task);
return error;
}
The same. And that's it - there's no more of them. So how about
this in the beginning of the series, then having your magiclink
error handling done in nd_jump_link()?
diff --git a/fs/namei.c b/fs/namei.c
index 671c3c1a3425..8ec924813c30 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -859,7 +859,7 @@ static int nd_jump_root(struct nameidata *nd)
* Helper to directly jump to a known parsed path from ->get_link,
* caller must have taken a reference to path beforehand.
*/
-void nd_jump_link(struct path *path)
+const char *nd_jump_link(struct path *path)
{
struct nameidata *nd = current->nameidata;
path_put(&nd->path);
@@ -867,6 +867,7 @@ void nd_jump_link(struct path *path)
nd->path = *path;
nd->inode = nd->path.dentry->d_inode;
nd->flags |= LOOKUP_JUMPED;
+ return NULL;
}
static inline void put_link(struct nameidata *nd)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ebea9501afb8..ac4e57a3dfa5 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1626,8 +1626,7 @@ static const char *proc_pid_get_link(struct dentry *dentry,
if (error)
goto out;
- nd_jump_link(&path);
- return NULL;
+ return nd_jump_link(&path);
out:
return ERR_PTR(error);
}
diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index dd2b35f78b09..dde0c501b2f3 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -54,7 +54,7 @@ static const char *proc_ns_get_link(struct dentry *dentry,
if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) {
error = ns_get_path(&ns_path, task, ns_ops);
if (!error)
- nd_jump_link(&ns_path);
+ error = nd_jump_link(&ns_path);
}
put_task_struct(task);
return error;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 397a08ade6a2..f3e8438e5631 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -68,7 +68,7 @@ extern int follow_up(struct path *);
extern struct dentry *lock_rename(struct dentry *, struct dentry *);
extern void unlock_rename(struct dentry *, struct dentry *);
-extern void nd_jump_link(struct path *path);
+extern const char *nd_jump_link(struct path *path);
static inline void nd_terminate_link(void *name, size_t len, size_t maxlen)
{
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 45d13b6462aa..98aef94b4777 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2453,18 +2453,16 @@ static const char *policy_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
{
- struct aa_ns *ns;
- struct path path;
-
- if (!dentry)
- return ERR_PTR(-ECHILD);
- ns = aa_get_current_ns();
- path.mnt = mntget(aafs_mnt);
- path.dentry = dget(ns_dir(ns));
- nd_jump_link(&path);
- aa_put_ns(ns);
-
- return NULL;
+ const char *err = ERR_PTR(-ECHILD);
+
+ if (dentry) {
+ struct aa_ns *ns = aa_get_current_ns();
+ struct path path = {.mnt = mntget(aafs_mnt),
+ .dentry = ns_dir(ns)};
+ err = nd_jump_link(&path);
+ aa_put_ns(ns);
+ }
+ return err;
}
static int policy_readlink(struct dentry *dentry, char __user *buffer,
^ permalink raw reply related
* Re: [PATCH v15 4/9] namei: LOOKUP_BENEATH: O_BENEATH-like scoped resolution
From: Al Viro @ 2019-11-13 1:55 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Christian Brauner, Shuah Khan, linux-arch, linux-s390,
Tycho Andersen, Aleksa Sarai, Jiri Olsa, Alexander Shishkin,
Ingo Molnar, linux-arm-kernel, linux-mips, linux-xtensa,
Kees Cook, Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k
In-Reply-To: <20191105090553.6350-5-cyphar@cyphar.com>
On Tue, Nov 05, 2019 at 08:05:48PM +1100, Aleksa Sarai wrote:
Minor nit here - I'd split "move the conditional call of set_root()
into nd_jump_root()" into a separate patch before that one. Makes
for fewer distractions in this one. I'd probably fold "and be
ready for errors other than -ECHILD" into the same preliminary
patch.
> + /* Not currently safe for scoped-lookups. */
> + if (unlikely(nd->flags & LOOKUP_IS_SCOPED))
> + return ERR_PTR(-EXDEV);
Also a candidate for doing in nd_jump_link()...
> @@ -1373,8 +1403,11 @@ static int follow_dotdot_rcu(struct nameidata *nd)
> struct inode *inode = nd->inode;
>
> while (1) {
> - if (path_equal(&nd->path, &nd->root))
> + if (path_equal(&nd->path, &nd->root)) {
> + if (unlikely(nd->flags & LOOKUP_BENEATH))
> + return -EXDEV;
Umm... Are you sure it's not -ECHILD?
^ permalink raw reply
* Re: [PATCH v15 5/9] namei: LOOKUP_IN_ROOT: chroot-like scoped resolution
From: Al Viro @ 2019-11-13 2:03 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Kees Cook, Jann Horn, Tycho Andersen,
David Drysdale, Chanho Min, Oleg Nesterov, Rasmus Villemoes,
Alexan
In-Reply-To: <20191105090553.6350-6-cyphar@cyphar.com>
On Tue, Nov 05, 2019 at 08:05:49PM +1100, Aleksa Sarai wrote:
> @@ -2277,12 +2277,20 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
>
> nd->m_seq = read_seqbegin(&mount_lock);
>
> - /* Figure out the starting path and root (if needed). */
> - if (*s == '/') {
> + /* Absolute pathname -- fetch the root. */
> + if (flags & LOOKUP_IN_ROOT) {
> + /* With LOOKUP_IN_ROOT, act as a relative path. */
> + while (*s == '/')
> + s++;
Er... Why bother skipping slashes? I mean, not only link_path_walk()
will skip them just fine, you are actually risking breakage in this:
if (*s && unlikely(!d_can_lookup(dentry))) {
fdput(f);
return ERR_PTR(-ENOTDIR);
}
which is downstream from there with you patch, AFAICS.
^ permalink raw reply
* Re: [PATCH v15 6/9] namei: LOOKUP_{IN_ROOT,BENEATH}: permit limited ".." resolution
From: Al Viro @ 2019-11-13 2:09 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, Jann Horn, Linus Torvalds, Eric Biederman,
Andy Lutomirski, Andrew Morton, Alexei Starovoitov, Kees Cook,
Tycho Andersen, David Drysdale, Chanho Min, Oleg Nesterov
In-Reply-To: <20191105090553.6350-7-cyphar@cyphar.com>
On Tue, Nov 05, 2019 at 08:05:50PM +1100, Aleksa Sarai wrote:
> One other possible alternative (which previous versions of this patch
> used) would be to check with path_is_under() if there was a racing
> rename or mount (after re-taking the relevant seqlocks). While this does
> work, it results in possible O(n*m) behaviour if there are many renames
> or mounts occuring *anywhere on the system*.
BTW, do you realize that open-by-fhandle (or working nfsd, for that matter)
will trigger arseloads of write_seqlock(&rename_lock) simply on d_splice_alias()
bringing disconnected subtrees in contact with parent?
^ permalink raw reply
* Re: [PATCH v15 7/9] open: introduce openat2(2) syscall
From: Al Viro @ 2019-11-13 2:29 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Kees Cook, Jann Horn, Tycho Andersen,
David Drysdale, Chanho Min, Oleg Nesterov, Rasmus Villemoes,
Alexander
In-Reply-To: <20191105090553.6350-8-cyphar@cyphar.com>
On Tue, Nov 05, 2019 at 08:05:51PM +1100, Aleksa Sarai wrote:
> +/*
> + * Arguments for how openat2(2) should open the target path. If @resolve is
> + * zero, then openat2(2) operates very similarly to openat(2).
> + *
> + * However, unlike openat(2), unknown bits in @flags result in -EINVAL rather
> + * than being silently ignored. @mode must be zero unless one of {O_CREAT,
> + * O_TMPFILE} are set, and @upgrade_mask must be zero unless O_PATH is set.
> + *
> + * @flags: O_* flags.
> + * @mode: O_CREAT/O_TMPFILE file mode.
> + * @upgrade_mask: UPGRADE_* flags (to restrict O_PATH re-opening).
???
> + * @resolve: RESOLVE_* flags.
> + */
> +struct open_how {
> + __aligned_u64 flags;
> + __u16 mode;
> + __u16 __padding[3]; /* must be zeroed */
> + __aligned_u64 resolve;
> +};
^ permalink raw reply
* Re: [PATCH v15 7/9] open: introduce openat2(2) syscall
From: Aleksa Sarai @ 2019-11-13 2:35 UTC (permalink / raw)
To: Al Viro
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Jiri Olsa, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Shuah Khan, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan <skha>
In-Reply-To: <20191113022906.GD26530@ZenIV.linux.org.uk>
[-- Attachment #1.1: Type: text/plain, Size: 1144 bytes --]
On 2019-11-13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Tue, Nov 05, 2019 at 08:05:51PM +1100, Aleksa Sarai wrote:
> > +/*
> > + * Arguments for how openat2(2) should open the target path. If @resolve is
> > + * zero, then openat2(2) operates very similarly to openat(2).
> > + *
> > + * However, unlike openat(2), unknown bits in @flags result in -EINVAL rather
> > + * than being silently ignored. @mode must be zero unless one of {O_CREAT,
> > + * O_TMPFILE} are set, and @upgrade_mask must be zero unless O_PATH is set.
> > + *
> > + * @flags: O_* flags.
> > + * @mode: O_CREAT/O_TMPFILE file mode.
> > + * @upgrade_mask: UPGRADE_* flags (to restrict O_PATH re-opening).
>
> ???
Sorry, that was left over from a previous revision (where the magic-link
re-opening restrictions were part of this series).
> > + * @resolve: RESOLVE_* flags.
> > + */
> > +struct open_how {
> > + __aligned_u64 flags;
> > + __u16 mode;
> > + __u16 __padding[3]; /* must be zeroed */
> > + __aligned_u64 resolve;
> > +};
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v15 5/9] namei: LOOKUP_IN_ROOT: chroot-like scoped resolution
From: Aleksa Sarai @ 2019-11-13 2:44 UTC (permalink / raw)
To: Al Viro
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Kees Cook, Jann Horn, Tycho Andersen,
David Drysdale, Chanho Min, Oleg Nesterov, Rasmus Villemoes,
Alexan
In-Reply-To: <20191113020307.GB26530@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]
On 2019-11-13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Tue, Nov 05, 2019 at 08:05:49PM +1100, Aleksa Sarai wrote:
>
> > @@ -2277,12 +2277,20 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> >
> > nd->m_seq = read_seqbegin(&mount_lock);
> >
> > - /* Figure out the starting path and root (if needed). */
> > - if (*s == '/') {
> > + /* Absolute pathname -- fetch the root. */
> > + if (flags & LOOKUP_IN_ROOT) {
> > + /* With LOOKUP_IN_ROOT, act as a relative path. */
> > + while (*s == '/')
> > + s++;
>
> Er... Why bother skipping slashes? I mean, not only link_path_walk()
> will skip them just fine, you are actually risking breakage in this:
> if (*s && unlikely(!d_can_lookup(dentry))) {
> fdput(f);
> return ERR_PTR(-ENOTDIR);
> }
> which is downstream from there with you patch, AFAICS.
I switched to stripping the slashes at your suggestion a few revisions
ago[1], and had (wrongly) assumed we needed to handle "/" somehow in
path_init(). But you're quite right about link_path_walk() -- and I'd be
more than happy to drop it.
[1]: https://lore.kernel.org/lkml/20190712125552.GL17978@ZenIV.linux.org.uk/
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v15 5/9] namei: LOOKUP_IN_ROOT: chroot-like scoped resolution
From: Al Viro @ 2019-11-13 2:59 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Kees Cook, Jann Horn, Tycho Andersen,
David Drysdale, Chanho Min, Oleg Nesterov, Rasmus Villemoes,
Alexan
In-Reply-To: <20191113024414.wlmvtjstpnkxa36n@yavin.dot.cyphar.com>
On Wed, Nov 13, 2019 at 01:44:14PM +1100, Aleksa Sarai wrote:
> On 2019-11-13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Tue, Nov 05, 2019 at 08:05:49PM +1100, Aleksa Sarai wrote:
> >
> > > @@ -2277,12 +2277,20 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> > >
> > > nd->m_seq = read_seqbegin(&mount_lock);
> > >
> > > - /* Figure out the starting path and root (if needed). */
> > > - if (*s == '/') {
> > > + /* Absolute pathname -- fetch the root. */
> > > + if (flags & LOOKUP_IN_ROOT) {
> > > + /* With LOOKUP_IN_ROOT, act as a relative path. */
> > > + while (*s == '/')
> > > + s++;
> >
> > Er... Why bother skipping slashes? I mean, not only link_path_walk()
> > will skip them just fine, you are actually risking breakage in this:
> > if (*s && unlikely(!d_can_lookup(dentry))) {
> > fdput(f);
> > return ERR_PTR(-ENOTDIR);
> > }
> > which is downstream from there with you patch, AFAICS.
>
> I switched to stripping the slashes at your suggestion a few revisions
> ago[1], and had (wrongly) assumed we needed to handle "/" somehow in
> path_init(). But you're quite right about link_path_walk() -- and I'd be
> more than happy to drop it.
That, IIRC, was about untangling the weirdness around multiple calls of
dirfd_path_init() and basically went "we might want just strip the slashes
in case of that flag very early in the entire thing, so that later the
normal logics for absolute/relative would DTRT". Since your check is
right next to checking for absolute pathnames (and not in the very
beginning of path_init()), we might as well turn the check for
absolute pathname into *s == '/' && !(flags & LOOKUP_IN_ROOT) and be
done with that.
^ permalink raw reply
* Re: [PATCH v15 5/9] namei: LOOKUP_IN_ROOT: chroot-like scoped resolution
From: Aleksa Sarai @ 2019-11-13 3:55 UTC (permalink / raw)
To: Al Viro
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Christian Brauner, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Kees Cook, Jann Horn, Tycho Andersen,
David Drysdale, Chanho Min, Oleg Nesterov, Rasmus Villemoes,
Alexan
In-Reply-To: <20191113025941.GE26530@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]
On 2019-11-13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Wed, Nov 13, 2019 at 01:44:14PM +1100, Aleksa Sarai wrote:
> > On 2019-11-13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > On Tue, Nov 05, 2019 at 08:05:49PM +1100, Aleksa Sarai wrote:
> > >
> > > > @@ -2277,12 +2277,20 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> > > >
> > > > nd->m_seq = read_seqbegin(&mount_lock);
> > > >
> > > > - /* Figure out the starting path and root (if needed). */
> > > > - if (*s == '/') {
> > > > + /* Absolute pathname -- fetch the root. */
> > > > + if (flags & LOOKUP_IN_ROOT) {
> > > > + /* With LOOKUP_IN_ROOT, act as a relative path. */
> > > > + while (*s == '/')
> > > > + s++;
> > >
> > > Er... Why bother skipping slashes? I mean, not only link_path_walk()
> > > will skip them just fine, you are actually risking breakage in this:
> > > if (*s && unlikely(!d_can_lookup(dentry))) {
> > > fdput(f);
> > > return ERR_PTR(-ENOTDIR);
> > > }
> > > which is downstream from there with you patch, AFAICS.
> >
> > I switched to stripping the slashes at your suggestion a few revisions
> > ago[1], and had (wrongly) assumed we needed to handle "/" somehow in
> > path_init(). But you're quite right about link_path_walk() -- and I'd be
> > more than happy to drop it.
>
> That, IIRC, was about untangling the weirdness around multiple calls of
> dirfd_path_init() and basically went "we might want just strip the slashes
> in case of that flag very early in the entire thing, so that later the
> normal logics for absolute/relative would DTRT".
Ah okay, I'd misunderstood the point you were making in that thread.
> Since your check is right next to checking for absolute pathnames (and
> not in the very beginning of path_init()), we might as well turn the
> check for absolute pathname into *s == '/' && !(flags &
> LOOKUP_IN_ROOT) and be done with that.
Yup, agreed.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v24 00/12] /dev/random - a new approach with full SP800-90B compliance
From: Stephan Müller @ 2019-11-13 4:24 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: <CALCETrVBzuOsDfaz5y3V4v+6xmeWufOYsOGnpZrRju6Pfsi6gg@mail.gmail.com>
Am Dienstag, 12. November 2019, 16:33:59 CET schrieb Andy Lutomirski:
Hi Andy,
> On Mon, Nov 11, 2019 at 11:13 AM Stephan Müller <smueller@chronox.de> wrote:
> > The following patch set provides a different approach to /dev/random which
> > is called Linux Random Number Generator (LRNG) to collect entropy within
> > the Linux kernel. The main improvements compared to the existing
> > /dev/random is to provide sufficient entropy during boot time as well as
> > in virtual environments and when using SSDs. A secondary design goal is
> > to limit the impact of the entropy collection on massive parallel systems
> > and also allow the use accelerated cryptographic primitives. Also, all
> > steps of the entropic data processing are testable.
>
> This is very nice!
>
> > The LRNG patch set allows a user to select use of the existing /dev/random
> > or the LRNG during compile time. As the LRNG provides API and ABI
> > compatible interfaces to the existing /dev/random implementation, the
> > user can freely chose the RNG implementation without affecting kernel or
> > user space operations.
> >
> > This patch set provides early boot-time entropy which implies that no
> > additional flags to the getrandom(2) system call discussed recently on
> > the LKML is considered to be necessary.
>
> I'm uneasy about this. I fully believe that, *on x86*, this works.
> But on embedded systems with in-order CPUs, a single clock, and very
> lightweight boot processes, most or all of boot might be too
> deterministic for this to work.
>
> I have a somewhat competing patch set here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=random
> /kill-it
>
> (Ignore the "horrible test hack" and the debugfs part.)
>
> The basic summary is that I change /dev/random so that it becomes
> functionally identical to getrandom(..., 0) -- in other words, it
> blocks until the CRNG is initialized but is then identical to
> /dev/urandom. And I add getrandom(...., GRND_INSECURE) that is
> functionally identical to the existing /dev/urandom: it always returns
> *something* immediately, but it may or may not actually be
> cryptographically random or even random at all depending on system
> details.
>
> In other words, my series simplifies the ABI that we support. Right
> now, we have three ways to ask for random numbers with different
> semantics and we need to have to RNGs in the kernel at all time. With
> my changes, we have only two ways to ask for random numbers, and the
> /dev/random pool is entirely gone.
>
> Would you be amenable to merging this into your series (i.e. either
> merging the code or just the ideas)? This would let you get rid of
> things like the compile-time selection of the blocking TRNG, since the
> blocking TRNG would be entirely gone.
I pulled your code and found the following based on my explanation that I
would suggest to keep the TRNG at least as an option.
- 7d54ef8512b06baf396f12584f7f48a9558ecd0f does not seem applicable: I also do
have an equivalent "lrng_init_wait" wait queue. This wait queue is used to let
in-kernel users wait until the LRNG obtained 128 bits of entropy. In addition,
this wait queue is used to let user space is invoked after the LRNG has
received 256 bits of entropy (which implies that the kernel waiters are
invoked earlier). In kernel waiters are all that call wait_for_random_bytes
and its derivatives. User space callers have to call getrandom(..., 0); to be
registered in this wait queue. So, I think the wakeup calls I have in the LRNG
for lrng_init_wait should remain.
- 6a26a3146e5fb90878dca9fde8caa1ca4233156a: My handler for /dev/urandom and
getrandom(..., 0) are using one callback which issues a warning in both use
cases (see lrng_sdrng_read). So I think this patch may not be applicable as
the LRNG code implements warning about being unseeded.
- 3e8e159da49b44ae0bb08e68fa2be760722fa033: I am happy to take that code which
would almost directly apply. The last hunk however would be:
if (!(flags & GRND_INSECURE) && unlikely(!lrng_state_operational())) {
==> Shall I apply it to my code base? If yes, how shall the changes to
random.h be handled?
- 920e97e7fc508e6f0da9c7dec94c8073fd63ab4d: I would pass on this patch due to
the following: it unconditionally starts removing the access to the TRNG (the
LRNG's logical equivalent to the blocking_pool). As patch 10/12 of the LRNG
patch series provides the TRNG that is a compile time option, your patch would
logically and functionally be equivalent when deselecting
CONFIG_LRNG_TRNG_SUPPORT in the LRNG without any further changes to the LRNG
code.
- 693b9ffdf0fdc93456b5ad293ac05edf240a531b: This patch is applicable to the
LRNG. In case CONFIG_LRNG_TRNG_SUPPORT is not set, the TRNG is not present.
Yet, the /dev/random and getrandom(GRND_RANDOM) would behave blocked until
fully initialized. I have now added the general blocking until the LRNG is
fully initialized to the common /dev/random and getrandom(GRND_RANDOM)
interface function of lrng_trng_read_common. With that, the LRNG would be
fully equivalent to this patch if CONFIG_LRNG_TRNG_SUPPORT is not set.
- 66f660842ec6d34134b9c3c1c9c65972834797f6: This patch is implicit with
CONFIG_LRNG_TRNG_SUPPORT being not set.
- d8f59b5c25af22fb9d85b7fa96de601ea03f2eac: This patch is not applicable to
the LRNG as the deactivation of CONFIG_LRNG_TRNG_SUPPORT implies that there
should be no unused code left in the LRNG.
- 4046ac638761821aef67af10537ebcbc80715785: In theory that patch is applicable
to the LRNG as well. The LRNG has the lrng_read_wait queue. If
CONFIG_LRNG_TRNG_SUPPORT is not set, there will never be the code triggered to
add a caller to this wait queue. To avoid cluttering the LRNG code with
ifdefs, may I suggest to leave these several lines even though it is dead
code?
Bottom line: the only patch that I seems to be relevant and that I would be
happy to apply is the one adding GRND_INSECURE. All other patches are
implicitly covered by deselecting CONFIG_LRNG_TRNG_SUPPORT.
By making the TRNG compile-time selectable, I was hoping to serve all users: I
wanted to cover the conclusions of the discussion to remove the blocking_pool.
On the other hand, however, I want to support requirements that need the
blocking behavior.
The current LRNG patch set, however, defaults to Y for
CONFIG_LRNG_TRNG_SUPPORT. I would see no issue if it defaults to N.
Thank you very much.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v24 00/12] /dev/random - a new approach with full SP800-90B compliance
From: Andy Lutomirski @ 2019-11-13 4:48 UTC (permalink / raw)
To: Stephan Müller
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: <3208655.cZiRAY37Id@positron.chronox.de>
On Tue, Nov 12, 2019 at 8:25 PM Stephan Müller <smueller@chronox.de> wrote:
>
> Am Dienstag, 12. November 2019, 16:33:59 CET schrieb Andy Lutomirski:
>
> Hi Andy,
>
> > On Mon, Nov 11, 2019 at 11:13 AM Stephan Müller <smueller@chronox.de> wrote:
> > > The following patch set provides a different approach to /dev/random which
> > > is called Linux Random Number Generator (LRNG) to collect entropy within
> > > the Linux kernel. The main improvements compared to the existing
> > > /dev/random is to provide sufficient entropy during boot time as well as
> > > in virtual environments and when using SSDs. A secondary design goal is
> > > to limit the impact of the entropy collection on massive parallel systems
> > > and also allow the use accelerated cryptographic primitives. Also, all
> > > steps of the entropic data processing are testable.
> >
> > This is very nice!
> >
> > > The LRNG patch set allows a user to select use of the existing /dev/random
> > > or the LRNG during compile time. As the LRNG provides API and ABI
> > > compatible interfaces to the existing /dev/random implementation, the
> > > user can freely chose the RNG implementation without affecting kernel or
> > > user space operations.
> > >
> > > This patch set provides early boot-time entropy which implies that no
> > > additional flags to the getrandom(2) system call discussed recently on
> > > the LKML is considered to be necessary.
> >
> > I'm uneasy about this. I fully believe that, *on x86*, this works.
> > But on embedded systems with in-order CPUs, a single clock, and very
> > lightweight boot processes, most or all of boot might be too
> > deterministic for this to work.
> >
> > I have a somewhat competing patch set here:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=random
> > /kill-it
> >
> > (Ignore the "horrible test hack" and the debugfs part.)
> >
> > The basic summary is that I change /dev/random so that it becomes
> > functionally identical to getrandom(..., 0) -- in other words, it
> > blocks until the CRNG is initialized but is then identical to
> > /dev/urandom. And I add getrandom(...., GRND_INSECURE) that is
> > functionally identical to the existing /dev/urandom: it always returns
> > *something* immediately, but it may or may not actually be
> > cryptographically random or even random at all depending on system
> > details.
> >
> > In other words, my series simplifies the ABI that we support. Right
> > now, we have three ways to ask for random numbers with different
> > semantics and we need to have to RNGs in the kernel at all time. With
> > my changes, we have only two ways to ask for random numbers, and the
> > /dev/random pool is entirely gone.
> >
> > Would you be amenable to merging this into your series (i.e. either
> > merging the code or just the ideas)? This would let you get rid of
> > things like the compile-time selection of the blocking TRNG, since the
> > blocking TRNG would be entirely gone.
>
> I pulled your code and found the following based on my explanation that I
> would suggest to keep the TRNG at least as an option.
>
> - 7d54ef8512b06baf396f12584f7f48a9558ecd0f does not seem applicable:
Not surprising. It's just a cleanup to the existing code, and I doubt
you inherited the oddity I'm fixing.
> - 6a26a3146e5fb90878dca9fde8caa1ca4233156a: My handler for /dev/urandom and
> getrandom(..., 0) are using one callback which issues a warning in both use
> cases (see lrng_sdrng_read). So I think this patch may not be applicable as
> the LRNG code implements warning about being unseeded.
Probably true.
What is the actual semantics of /dev/urandom with your series applied?
Is there any situation in which it will block?
>
> - 3e8e159da49b44ae0bb08e68fa2be760722fa033: I am happy to take that code which
> would almost directly apply. The last hunk however would be:
>
> if (!(flags & GRND_INSECURE) && unlikely(!lrng_state_operational())) {
>
> ==> Shall I apply it to my code base? If yes, how shall the changes to
> random.h be handled?
>
This might be a question for Ted. Once the merge window opens, I'll
resubmit it.
>
> - 920e97e7fc508e6f0da9c7dec94c8073fd63ab4d: I would pass on this patch due to
> the following: it unconditionally starts removing the access to the TRNG (the
> LRNG's logical equivalent to the blocking_pool). As patch 10/12 of the LRNG
> patch series provides the TRNG that is a compile time option, your patch would
> logically and functionally be equivalent when deselecting
> CONFIG_LRNG_TRNG_SUPPORT in the LRNG without any further changes to the LRNG
> code.
Given your previous email about the TRNG, I'm wondering what the API
for the TRNG should be. I am willing to grant that there are users
who need a TRNG for various reasons, and that not all of them can use
hwrng. (And the current hwrng API is pretty bad.) But I'm not
convinced that /dev/random or getrandom(..., GRND_RANDOM) is a
reasonable way to access it. A blocking_pool-style TRNG is a very
limited resource, and I think it could make sense to require some sort
of actual permission to use it. GRND_RANDOM has no access control at
all, and everyone expects /dev/random to be world-readable. The most
widespread user of /dev/random that I know of is gnupg, and gnupg
really should not be using it.
Would it make sense to have a /dev/true_random that is 0400 by default
for users who actually need it? Then /dev/random and GRND_RANDOM
could work as they do with my patch, and maybe it does the right thing
for everyone.
>
> - 693b9ffdf0fdc93456b5ad293ac05edf240a531b: This patch is applicable to the
> LRNG. In case CONFIG_LRNG_TRNG_SUPPORT is not set, the TRNG is not present.
> Yet, the /dev/random and getrandom(GRND_RANDOM) would behave blocked until
> fully initialized. I have now added the general blocking until the LRNG is
> fully initialized to the common /dev/random and getrandom(GRND_RANDOM)
> interface function of lrng_trng_read_common. With that, the LRNG would be
> fully equivalent to this patch if CONFIG_LRNG_TRNG_SUPPORT is not set.
Sounds reasonable.
> By making the TRNG compile-time selectable, I was hoping to serve all users: I
> wanted to cover the conclusions of the discussion to remove the blocking_pool.
> On the other hand, however, I want to support requirements that need the
> blocking behavior.
I find it odd that /dev/random would be either a TRNG or not a TRNG
depending on kernel configuration. For the small fraction of users
that actually want a TRNG, wouldn't it be better to have an interface
that fails outright if the TRNG is not enabled?
--Andy
^ permalink raw reply
* Re: [PATCH] Allow restricting permissions in /proc/sys
From: Andy Lutomirski @ 2019-11-13 4:50 UTC (permalink / raw)
To: Christian Brauner
Cc: Topi Miettinen, Luis Chamberlain, Kees Cook, Alexey Dobriyan,
linux-kernel@vger.kernel.org,
open list:FILESYSTEMS (VFS and infrastructure), Linux API
In-Reply-To: <20191112232239.yevpeemgxz4wy32b@wittgenstein>
On Tue, Nov 12, 2019 at 3:22 PM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> [Cc+ linux-api@vger.kernel.org]
>
> since that's potentially relevant to quite a few people.
>
> On Sun, Nov 03, 2019 at 04:55:48PM +0200, Topi Miettinen wrote:
> > Several items in /proc/sys need not be accessible to unprivileged
> > tasks. Let the system administrator change the permissions, but only
> > to more restrictive modes than what the sysctl tables allow.
> >
> > Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
> > ---
> > fs/proc/proc_sysctl.c | 41 +++++++++++++++++++++++++++++++----------
> > 1 file changed, 31 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > index d80989b6c344..88c4ca7d2782 100644
> > --- a/fs/proc/proc_sysctl.c
> > +++ b/fs/proc/proc_sysctl.c
> > @@ -818,6 +818,10 @@ static int proc_sys_permission(struct inode *inode, int
> > mask)
> > if ((mask & MAY_EXEC) && S_ISREG(inode->i_mode))
> > return -EACCES;
> >
> > + error = generic_permission(inode, mask);
> > + if (error)
> > + return error;
> > +
> > head = grab_header(inode);
> > if (IS_ERR(head))
> > return PTR_ERR(head);
> > @@ -837,9 +841,35 @@ static int proc_sys_setattr(struct dentry *dentry,
> > struct iattr *attr)
> > struct inode *inode = d_inode(dentry);
> > int error;
> >
> > - if (attr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
> > + if (attr->ia_valid & (ATTR_UID | ATTR_GID))
> > return -EPERM;
Supporting at least ATTR_GID would make this much more useful.
> >
> > + if (attr->ia_valid & ATTR_MODE) {
> > + struct ctl_table_header *head = grab_header(inode);
> > + struct ctl_table *table = PROC_I(inode)->sysctl_entry;
> > + umode_t max_mode = 0777; /* Only these bits may change */
> > +
> > + if (IS_ERR(head))
> > + return PTR_ERR(head);
> > +
> > + if (!table) /* global root - r-xr-xr-x */
> > + max_mode &= ~0222;
> > + else /*
> > + * Don't allow permissions to become less
> > + * restrictive than the sysctl table entry
> > + */
> > + max_mode &= table->mode;
Style nit: please put braces around multi-line if and else branches,
even if they're only multi-line because of comments.
> > +
> > + sysctl_head_finish(head);
> > +
> > + /* Execute bits only allowed for directories */
> > + if (!S_ISDIR(inode->i_mode))
> > + max_mode &= ~0111;
Why is this needed?
^ permalink raw reply
* [PATCH v6 0/2] add performance reporting support to FPGA DFL drivers
From: Wu Hao @ 2019-11-13 5:24 UTC (permalink / raw)
To: mdf, will, mark.rutland, linux-fpga, linux-kernel
Cc: linux-api, atull, gregkh, Wu Hao
Hi Moritz and all,
This patchset adds performance reporting support for FPGA DFL drivers. It
introduces one pmu to expose userspace interfaces via standard perf API.
User could use standard perf tool to access perf events exposed via pmu.
This patchset is generated based on fpga/for-next branch.
Main changes from v5:
- use dev_ext_attribute instead of fme_perf_event_attr.
- use is_visible function to decide which events to expose per
hardware capability, and add event_init checking for all events.
Main changes from v4:
- rebase and clean up.
- update Kconfig for PERF_EVENTS dependency.
Main changes from v3:
- add more descriptions in doc, including how to use perf tool for these
hardware counters. (patch #1)
- use standard perf API instead of sysfs entries. (patch #2)
Wu Hao (1):
fpga: dfl: fme: add performance reporting support
Xu Yilun (1):
Documentation: fpga: dfl: add description for performance reporting
support
Documentation/fpga/dfl.rst | 83 ++++
drivers/fpga/Makefile | 1 +
drivers/fpga/dfl-fme-main.c | 4 +
drivers/fpga/dfl-fme-perf.c | 943 ++++++++++++++++++++++++++++++++++++++++++++
drivers/fpga/dfl-fme.h | 2 +
drivers/fpga/dfl.h | 2 +
6 files changed, 1035 insertions(+)
create mode 100644 drivers/fpga/dfl-fme-perf.c
--
1.8.3.1
^ permalink raw reply
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