* [PATCH v29 10/12] LRNG - add SP800-90B compliant health tests
From: Stephan Müller @ 2020-01-19 21:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange, "Peter, Matthias" <matth>
In-Reply-To: <1967138.dxe05VgvIB@positron.chronox.de>
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 non-zero, the received time delta is
considered to be non-stuck.
- 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 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. All 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.
The patch provides a CONFIG_BROKEN configuration of the APT / RCT cutoff
values which have a high likelihood to trigger the health test failure.
The BROKEN APT cutoff is set to the exact mean of the expected value if
the time stamps are equally distributed (512 time stamps divided by 16
possible values due to using the 4 LSB of the time stamp). The BROKEN
RCT cutoff value is set to 1 which is likely to be triggered during
regular operation.
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 | 56 +++++
drivers/char/lrng/Makefile | 1 +
drivers/char/lrng/lrng_health.c | 407 ++++++++++++++++++++++++++++++++
3 files changed, 464 insertions(+)
create mode 100644 drivers/char/lrng/lrng_health.c
diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
index 4684e838f011..323c657f7005 100644
--- a/drivers/char/lrng/Kconfig
+++ b/drivers/char/lrng/Kconfig
@@ -106,4 +106,60 @@ config LRNG_JENT
time or at runtime with the lrng_base.jitterrng configuration
variable.
+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.
+
+ If unsure, say Y.
+
+config LRNG_RCT_BROKEN
+ bool "SP800-90B RCT with dangerous low cutoff value"
+ depends on LRNG_HEALTH_TESTS
+ depends on BROKEN
+ default n
+ help
+ This option enables a dangerously low SP800-90B repetitive
+ count test (RCT) cutoff value which makes it very likely
+ that the RCT is triggered to raise a self test failure.
+
+ This option is ONLY intended for developers wanting to
+ test the effectiveness of the SP800-90B RCT health test.
+
+ If unsure, say N.
+
+config LRNG_APT_BROKEN
+ bool "SP800-90B APT with dangerous low cutoff value"
+ depends on LRNG_HEALTH_TESTS
+ depends on BROKEN
+ default n
+ help
+ This option enables a dangerously low SP800-90B adaptive
+ proportion test (APT) cutoff value which makes it very
+ likely that the RCT is triggered to raise a self test
+ failure.
+
+ This option is ONLY intended for developers wanting to
+ test the effectiveness of the SP800-90B APT health test.
+
+ If unsure, say N.
+
+# Default taken from SP800-90B sec 4.4.1 - significance level 2^-30
+config LRNG_RCT_CUTOFF
+ int
+ default 31 if !LRNG_RCT_BROKEN
+ default 1 if LRNG_RCT_BROKEN
+
+# Default taken from SP800-90B sec 4.4.2 - significance level 2^-30
+config LRNG_APT_CUTOFF
+ int
+ default 325 if !LRNG_APT_BROKEN
+ default 32 if LRNG_APT_BROKEN
+
endif # LRNG
diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
index 4f5b6f38f0c4..c3008763dd14 100644
--- a/drivers/char/lrng/Makefile
+++ b/drivers/char/lrng/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_LRNG_DRNG_SWITCH) += lrng_switch.o
obj-$(CONFIG_LRNG_DRBG) += lrng_drbg.o
obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
obj-$(CONFIG_LRNG_JENT) += lrng_jent.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..7817aa6f3357
--- /dev/null
+++ b/drivers/char/lrng/lrng_health.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * Linux Random Number Generator (LRNG) Health Testing
+ *
+ * Copyright (C) 2019 - 2020, Stephan Mueller <smueller@chronox.de>
+ */
+
+#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 {
+ /* 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 DRNGs to ensure they are seeded with
+ * entropy that passed the SP800-90B health tests.
+ * As the DRNG always will reseed before generating
+ * random numbers, it does not need a reseed trigger.
+ */
+ lrng_drng_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 >= CONFIG_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 >= CONFIG_LRNG_RCT_CUTOFF) {
+ 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 stamp
+ */
+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;
+}
--
2.24.1
^ permalink raw reply related
* [PATCH v29 11/12] LRNG - add interface for gathering of raw entropy
From: Stephan Müller @ 2020-01-19 21:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, linux-crypto-u79uwXL29TY76Z2rM5mHXA, LKML,
linux-api-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
Alexander E. Patrakov, Ahmed S. Darwish, Theodore Y. Ts'o,
Willy Tarreau, Matthew Garrett, Vito Caputo, Andreas Dilger,
Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange, "Peter, Matthias" <matth>
In-Reply-To: <1967138.dxe05VgvIB-jJGQKZiSfeo1haGO/jJMPxvVK+yQ3ZXh@public.gmane.org>
The test interface allows a privileged process to capture the raw
unconditioned noise that is collected by the LRNG for statistical
analysis. Such testing allows the analysis how much entropy
the interrupt noise source provides on a given platform.
Extracted noise data is not used to seed the LRNG. This
is a test interface and not appropriate for production systems.
Yet, the interface is considered to be sufficiently secured for
production systems.
Access to the data is given through the lrng_raw debugfs file. The
data buffer should be multiples of sizeof(u32) to fill the entire
buffer. Using the option lrng_testing.boot_test=1 the raw noise of
the first 1000 entropy events since boot can be sampled.
This test interface allows generating the data required for
analysis whether the LRNG is in compliance with SP800-90B
sections 3.1.3 and 3.1.4.
CC: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
CC: "Alexander E. Patrakov" <patrakov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: "Ahmed S. Darwish" <darwish.07-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: "Theodore Y. Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
CC: Willy Tarreau <w@1wt.eu>
CC: Matthew Garrett <mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
CC: Vito Caputo <vcaputo-IiWei5kqaphBDgjK7y7TUQ@public.gmane.org>
CC: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
CC: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
CC: Ray Strode <rstrode-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
CC: William Jon McCann <mccann-4GNroTWusrE@public.gmane.org>
CC: zhangjs <zachary-+jQ8zvd9CV5SzTKKI/wJ2A@public.gmane.org>
CC: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
CC: Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
CC: Lennart Poettering <mzxreary-uLTowLwuiw4b1SvskN2V4Q@public.gmane.org>
CC: Nicolai Stange <nstange-l3A5Bk7waGM@public.gmane.org>
Reviewed-by: Roman Drahtmueller <draht-UTA6Eu5RHcbyVfKhlJ+g5A@public.gmane.org>
Tested-by: Roman Drahtmüller <draht-UTA6Eu5RHcbyVfKhlJ+g5A@public.gmane.org>
Tested-by: Marcelo Henrique Cerri <marcelo.cerri-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Tested-by: Neil Horman <nhorman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
---
drivers/char/lrng/Kconfig | 16 ++
drivers/char/lrng/Makefile | 1 +
drivers/char/lrng/lrng_testing.c | 269 +++++++++++++++++++++++++++++++
3 files changed, 286 insertions(+)
create mode 100644 drivers/char/lrng/lrng_testing.c
diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
index 323c657f7005..014ee82df79a 100644
--- a/drivers/char/lrng/Kconfig
+++ b/drivers/char/lrng/Kconfig
@@ -162,4 +162,20 @@ config LRNG_APT_CUTOFF
default 325 if !LRNG_APT_BROKEN
default 32 if LRNG_APT_BROKEN
+config LRNG_TESTING
+ bool "Enable entropy test interface to LRNG noise source"
+ depends on DEBUG_FS
+ help
+ The test interface allows a privileged process to capture
+ the raw unconditioned noise that is collected by the LRNG
+ for statistical analysis. Extracted noise data is not used
+ to seed the LRNG.
+
+ The raw noise data can be obtained using the lrng_raw
+ debugfs file. Using the option lrng_testing.boot_test=1
+ the raw noise of the first 1000 entropy events since boot
+ can be sampled.
+
+ If unsure, say N.
+
endif # LRNG
diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
index c3008763dd14..b2ce1979dc4b 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_HEALTH_TESTS) += lrng_health.o
+obj-$(CONFIG_LRNG_TESTING) += lrng_testing.o
diff --git a/drivers/char/lrng/lrng_testing.c b/drivers/char/lrng/lrng_testing.c
new file mode 100644
index 000000000000..996fc665a0a0
--- /dev/null
+++ b/drivers/char/lrng/lrng_testing.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * Linux Random Number Generator (LRNG) Raw entropy collection tool
+ *
+ * Copyright (C) 2019 - 2020, Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/atomic.h>
+#include <linux/bug.h>
+#include <linux/debugfs.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/workqueue.h>
+#include <asm/errno.h>
+
+#include "lrng_internal.h"
+
+#define LRNG_TESTING_RINGBUFFER_SIZE 1024
+#define LRNG_TESTING_RINGBUFFER_MASK (LRNG_TESTING_RINGBUFFER_SIZE - 1)
+
+static u32 lrng_testing_rb[LRNG_TESTING_RINGBUFFER_SIZE];
+static u32 lrng_rb_reader = 0;
+static u32 lrng_rb_writer = 0;
+static atomic_t lrng_testing_enabled = ATOMIC_INIT(0);
+
+static DECLARE_WAIT_QUEUE_HEAD(lrng_raw_read_wait);
+static DEFINE_SPINLOCK(lrng_raw_lock);
+
+/*
+ * 0 ==> No boot test, gathering of runtime data allowed
+ * 1 ==> Boot test enabled and ready for collecting data, gathering runtime
+ * data is disabled
+ * 2 ==> Boot test completed and disabled, gathering of runtime data is
+ * disabled
+ */
+static u32 boot_test = 0;
+module_param(boot_test, uint, 0644);
+MODULE_PARM_DESC(boot_test, "Enable gathering boot time entropy of the first entropy events");
+
+static inline void lrng_raw_entropy_reset(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&lrng_raw_lock, flags);
+ lrng_rb_reader = 0;
+ lrng_rb_writer = 0;
+ spin_unlock_irqrestore(&lrng_raw_lock, flags);
+}
+
+static void lrng_raw_entropy_init(void)
+{
+ /*
+ * The boot time testing implies we have a running test. If the
+ * caller wants to clear it, he has to unset the boot_test flag
+ * at runtime via sysfs to enable regular runtime testing
+ */
+ if (boot_test)
+ return;
+
+ lrng_raw_entropy_reset();
+ atomic_set(&lrng_testing_enabled, 1);
+ pr_warn("Enabling raw entropy collection\n");
+}
+
+static void lrng_raw_entropy_fini(void)
+{
+ if (boot_test)
+ return;
+
+ atomic_set(&lrng_testing_enabled, 0);
+ lrng_raw_entropy_reset();
+ pr_warn("Disabling raw entropy collection\n");
+}
+
+bool lrng_raw_entropy_store(u32 value)
+{
+ unsigned long flags;
+
+ if (!atomic_read(&lrng_testing_enabled) && (boot_test != 1))
+ return false;
+
+ spin_lock_irqsave(&lrng_raw_lock, flags);
+
+ /*
+ * Disable entropy testing for boot time testing after ring buffer
+ * is filled.
+ */
+ if (boot_test) {
+ if (lrng_rb_writer > LRNG_TESTING_RINGBUFFER_SIZE) {
+ boot_test = 2;
+ pr_warn_once("Boot time entropy collection test disabled\n");
+ spin_unlock_irqrestore(&lrng_raw_lock, flags);
+ return false;
+ }
+
+ if (lrng_rb_writer == 1)
+ pr_warn("Boot time entropy collection test enabled\n");
+ }
+
+ lrng_testing_rb[lrng_rb_writer & LRNG_TESTING_RINGBUFFER_MASK] = value;
+ lrng_rb_writer++;
+
+ spin_unlock_irqrestore(&lrng_raw_lock, flags);
+
+ if (wq_has_sleeper(&lrng_raw_read_wait))
+ wake_up_interruptible(&lrng_raw_read_wait);
+
+ return true;
+}
+
+static inline bool lrng_raw_have_data(void)
+{
+ return ((lrng_rb_writer & LRNG_TESTING_RINGBUFFER_MASK) !=
+ (lrng_rb_reader & LRNG_TESTING_RINGBUFFER_MASK));
+}
+
+static int lrng_raw_entropy_reader(u8 *outbuf, u32 outbuflen)
+{
+ unsigned long flags;
+ int collected_data = 0;
+
+ lrng_raw_entropy_init();
+
+ while (outbuflen) {
+ spin_lock_irqsave(&lrng_raw_lock, flags);
+
+ /* We have no data or reached the writer. */
+ if (!lrng_rb_writer || (lrng_rb_writer == lrng_rb_reader)) {
+
+ spin_unlock_irqrestore(&lrng_raw_lock, flags);
+
+ /*
+ * Now we gathered all boot data, enable regular data
+ * collection.
+ */
+ if (boot_test) {
+ boot_test = 0;
+ goto out;
+ }
+
+ wait_event_interruptible(lrng_raw_read_wait,
+ lrng_raw_have_data());
+ if (signal_pending(current)) {
+ collected_data = -ERESTARTSYS;
+ goto out;
+ }
+
+ continue;
+ }
+
+ /* We copy out word-wise */
+ if (outbuflen < sizeof(u32)) {
+ spin_unlock_irqrestore(&lrng_raw_lock, flags);
+ goto out;
+ }
+
+ memcpy(outbuf, &lrng_testing_rb[lrng_rb_reader], sizeof(u32));
+ lrng_rb_reader++;
+
+ spin_unlock_irqrestore(&lrng_raw_lock, flags);
+
+ outbuf += sizeof(u32);
+ outbuflen -= sizeof(u32);
+ collected_data += sizeof(u32);
+ }
+
+out:
+ lrng_raw_entropy_fini();
+ return collected_data;
+}
+
+/**************************************************************************
+ * Debugfs interface
+ **************************************************************************/
+static int lrng_raw_extract_user(char __user *buf, size_t nbytes)
+{
+ u8 *tmp, *tmp_aligned;
+ int ret = 0, large_request = (nbytes > 256);
+
+ /*
+ * The intention of this interface is for collecting at least
+ * 1000 samples due to the SP800-90B requirements. So, we make no
+ * effort in avoiding allocating more memory that actually needed
+ * by the user. Hence, we allocate sufficient memory to always hold
+ * that amount of data.
+ */
+ tmp = kmalloc(LRNG_TESTING_RINGBUFFER_SIZE + sizeof(u32), GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+
+ tmp_aligned = PTR_ALIGN(tmp, sizeof(u32));
+
+ while (nbytes) {
+ int i;
+
+ if (large_request && need_resched()) {
+ if (signal_pending(current)) {
+ if (ret == 0)
+ ret = -ERESTARTSYS;
+ break;
+ }
+ schedule();
+ }
+
+ i = min_t(int, nbytes, LRNG_TESTING_RINGBUFFER_SIZE);
+ i = lrng_raw_entropy_reader(tmp_aligned, i);
+ if (i <= 0) {
+ if (i < 0)
+ ret = i;
+ break;
+ }
+ if (copy_to_user(buf, tmp_aligned, i)) {
+ ret = -EFAULT;
+ break;
+ }
+
+ nbytes -= i;
+ buf += i;
+ ret += i;
+ }
+
+ kzfree(tmp);
+ return ret;
+}
+
+/* DebugFS operations and definition of the debugfs files */
+static ssize_t lrng_raw_read(struct file *file, char __user *to,
+ size_t count, loff_t *ppos)
+{
+ loff_t pos = *ppos;
+ int ret;
+
+ if (!count)
+ return 0;
+
+ ret = lrng_raw_extract_user(to, count);
+ if (ret < 0)
+ return ret;
+
+ count -= ret;
+ *ppos = pos + count;
+
+ return ret;
+}
+
+static const struct file_operations lrng_raw_name_fops = {
+ .owner = THIS_MODULE,
+ .read = lrng_raw_read,
+};
+
+static int __init lrng_raw_init(void)
+{
+ struct dentry *lrng_raw_debugfs_root;
+
+ lrng_raw_debugfs_root = debugfs_create_dir(KBUILD_MODNAME, NULL);
+ debugfs_create_file_unsafe("lrng_raw", 0400, lrng_raw_debugfs_root,
+ NULL, &lrng_raw_name_fops);
+
+ return 0;
+}
+
+module_init(lrng_raw_init);
--
2.24.1
^ permalink raw reply related
* [PATCH v29 12/12] LRNG - add power-on and runtime self-tests
From: Stephan Müller @ 2020-01-19 21:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange, "Peter, Matthias" <matth>
In-Reply-To: <1967138.dxe05VgvIB@positron.chronox.de>
Parts of the LRNG are already covered by self-tests, including:
* Self-test of SP800-90A DRBG provided by the Linux kernel crypto API.
* Self-test of the PRNG provided by the Linux kernel crypto API.
* Raw noise source data testing including SP800-90B compliant
tests when enabling CONFIG_LRNG_HEALTH_TESTS
This patch adds the self-tests for the remaining critical functions of
the LRNG that are essential to maintain entropy and provide
cryptographic strong random numbers. The following self-tests are
implemented:
* Self-test of the time array maintenance. This test verifies whether
the time stamp array management to store multiple values in one integer
implements a concatenation of the data.
* Self-test of the LFSR operation. This test injects a monotonic
increasing counter into the LFSR. After completion of the injection of
the counter, 3 pool words are compared with known good values. The known
good values are calculated with the newly-developed tool
lfsr_testvector_generation provided as part of the LRNG test tool set at
[1].
* Self-test of the Hash_DF operation ensures that this function operates
compliant to the specification. The self-test performs a Hash_DF
operation of a zeroized entropy pool state. The test vectors are
generated using the newly-developed tool hash_df_testvector_generation
provided as part of the LRNG test tool set at [1].
* Self-test of the ChaCha20 DRNG is based on the self-tests that are
already present and implemented with the stand-alone user space
ChaCha20 DRNG implementation available at [2]. The self-tests cover
different use cases of the DRNG seeded with known seed data.
The status of the LRNG self-tests is provided with the selftest_status
SysFS file. If the file contains a zero, the self-tests passed. The
value 0xffffffff means that the self-tests were not executed. Any other
value indicates a self-test failure.
The self-test may be compiled to panic the system if the self-test
fails.
All self-tests operate on private state data structures. This implies
that none of the self-tests have any impact on the regular LRNG
operations. This allows the self-tests to be repeated at runtime by
writing anything into the selftest_status SysFS file.
[1] https://www.chronox.de/lrng.html
[2] https://www.chronox.de/chacha20.html
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>
CC: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
CC: Neil Horman <nhorman@redhat.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/char/lrng/Kconfig | 25 ++
drivers/char/lrng/Makefile | 1 +
drivers/char/lrng/lrng_selftest.c | 418 ++++++++++++++++++++++++++++++
3 files changed, 444 insertions(+)
create mode 100644 drivers/char/lrng/lrng_selftest.c
diff --git a/drivers/char/lrng/Kconfig b/drivers/char/lrng/Kconfig
index 014ee82df79a..896726138651 100644
--- a/drivers/char/lrng/Kconfig
+++ b/drivers/char/lrng/Kconfig
@@ -178,4 +178,29 @@ config LRNG_TESTING
If unsure, say N.
+config LRNG_SELFTEST
+ bool "Enable power-on and on-demand self-tests"
+ help
+ The power-on self-tests are executed during boot time
+ covering the ChaCha20 DRNG, the LFSR processing and the
+ time stamp management of the LRNG.
+
+ The on-demand self-tests are triggered by writing any
+ value into the SysFS file selftest_status. At the same
+ time, when reading this file, the test status is
+ returned. A zero indicates that all tests were executed
+ successfully.
+
+ If unsure, say Y.
+
+if LRNG_SELFTEST
+
+config LRNG_SELFTEST_PANIC
+ bool "Panic the kernel upon self-test failure"
+ help
+ If the option is enabled, the kernel is terminated if an
+ LRNG power-on self-test failure is detected.
+
+endif # LRNG_SELFTEST
+
endif # LRNG
diff --git a/drivers/char/lrng/Makefile b/drivers/char/lrng/Makefile
index b2ce1979dc4b..92219c565f66 100644
--- a/drivers/char/lrng/Makefile
+++ b/drivers/char/lrng/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_LRNG_KCAPI) += lrng_kcapi.o
obj-$(CONFIG_LRNG_JENT) += lrng_jent.o
obj-$(CONFIG_LRNG_HEALTH_TESTS) += lrng_health.o
obj-$(CONFIG_LRNG_TESTING) += lrng_testing.o
+obj-$(CONFIG_LRNG_SELFTEST) += lrng_selftest.o
diff --git a/drivers/char/lrng/lrng_selftest.c b/drivers/char/lrng/lrng_selftest.c
new file mode 100644
index 000000000000..bf69ba6bffa1
--- /dev/null
+++ b/drivers/char/lrng/lrng_selftest.c
@@ -0,0 +1,418 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * LRNG power-on and on-demand self-test
+ *
+ * Copyright (C) 2016 - 2020, Stephan Mueller <smueller@chronox.de>
+ */
+
+/*
+ * In addition to the self-tests below, the following LRNG components
+ * are covered with self-tests during regular operation:
+ *
+ * * power-on self-test: SP800-90A DRBG provided by the Linux kernel crypto API
+ * * power-on self-test: PRNG provided by the Linux kernel crypto API
+ * * runtime test: Raw noise source data testing including SP800-90B compliant
+ * tests when enabling CONFIG_LRNG_HEALTH_TESTS
+ *
+ * Additional developer tests present with LRNG code:
+ * * SP800-90B APT and RCT test enforcement validation when enabling
+ * CONFIG_LRNG_APT_BROKEN or CONFIG_LRNG_RCT_BROKEN.
+ * * Collection of raw entropy from the interrupt noise source when enabling
+ * CONFIG_LRNG_TESTING and pulling the data from the kernel with the provided
+ * interface.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/lrng.h>
+#include <linux/slab.h>
+
+#include "lrng_chacha20.h"
+#include "lrng_internal.h"
+#include "lrng_lfsr.h"
+#include "lrng_sw_noise.h"
+
+#define LRNG_SELFTEST_PASSED 0
+#define LRNG_SEFLTEST_ERROR_TIME (1 << 0)
+#define LRNG_SEFLTEST_ERROR_LFSR (1 << 1)
+#define LRNG_SEFLTEST_ERROR_CHACHA20 (1 << 2)
+#define LRNG_SEFLTEST_ERROR_HASHDF (1 << 3)
+#define LRNG_SELFTEST_NOT_EXECUTED 0xffffffff
+
+static u32 lrng_time_selftest[LRNG_TIME_ARRAY_SIZE];
+
+static unsigned int lrng_selftest_status = LRNG_SELFTEST_NOT_EXECUTED;
+
+static inline void lrng_time_process_selftest_insert(u32 time)
+{
+ static u32 lrng_time_selftest_ptr = 0;
+ u32 ptr = lrng_time_selftest_ptr++ & LRNG_TIME_WORD_MASK;
+
+ lrng_time_selftest[lrng_time_idx2array(ptr)] |=
+ lrng_time_slot_val(time & LRNG_TIME_SLOTSIZE_MASK,
+ lrng_time_idx2slot(ptr));
+}
+
+static unsigned int lrng_time_process_selftest(void)
+{
+ u32 time;
+ u32 idx_zero_compare = (0 << 0) | (1 << 8) | (2 << 16) | (3 << 24);
+ u32 idx_one_compare = (4 << 0) | (5 << 8) | (6 << 16) | (7 << 24);
+ u32 idx_last_compare = ((LRNG_TIME_NUM_VALUES - 4) << 0) |
+ ((LRNG_TIME_NUM_VALUES - 3) << 8) |
+ ((LRNG_TIME_NUM_VALUES - 2) << 16) |
+ ((LRNG_TIME_NUM_VALUES - 1) << 24);
+
+ (void)idx_one_compare;
+
+ for (time = 0; time < LRNG_TIME_NUM_VALUES; time++)
+ lrng_time_process_selftest_insert(time);
+
+ if ((lrng_time_selftest[0] != idx_zero_compare) ||
+#if (LRNG_TIME_ARRAY_SIZE > 1)
+ (lrng_time_selftest[1] != idx_one_compare) ||
+#endif
+ (lrng_time_selftest[LRNG_TIME_ARRAY_SIZE - 1] != idx_last_compare))
+ {
+ pr_err("LRNG time array self-test FAILED\n");
+ return LRNG_SEFLTEST_ERROR_TIME;
+ }
+
+ return LRNG_SELFTEST_PASSED;
+}
+
+/*
+ * The test vectors are generated with the lfsr_testvector_generation tool
+ * provided as part of the test tool set of the LRNG.
+ */
+static unsigned int lrng_pool_lfsr_selftest(void)
+{
+ /*
+ * First, 67th and last entry of entropy pool.
+ *
+ * The 67th entry is picked because this one is the first to receive
+ * an entry. As we start with 1 to inject into the LFSR, the
+ * 67th entry should be equal to rol(1, 7) >> 3 considering that
+ * all other values of the LFSR are zero and the the twist value of 0
+ * is applied.
+ */
+ static u32 const lrng_lfsr_selftest_result[][3] = {
+ { 0xf56df24a, 0x00000010, 0x0e014939 },
+ { 0x4b130726, 0x00000010, 0x2802f509 },
+ { 0x87279152, 0x00000010, 0x00150000 },
+ { 0x0b67f997, 0x00000010, 0x00150000 },
+ { 0x4fea174f, 0x00000010, 0xcbf4a6ae },
+ { 0x77149108, 0x00000010, 0x77bfadf2 },
+ { 0x1e96037e, 0x00000010, 0x18017e79 },
+ { 0xc84acef2, 0x00000010, 0x6345f7a8 },
+ { 0x6a2eb6df, 0x00000010, 0x03950000 },
+ };
+ struct lrng_pool *lrng_pool, *lrng_pool_aligned;
+ u32 i, ret = LRNG_SELFTEST_PASSED;
+
+ BUILD_BUG_ON(ARRAY_SIZE(lrng_lfsr_selftest_result) <
+ CONFIG_LRNG_POOL_SIZE);
+
+ lrng_pool = kzalloc(sizeof(struct lrng_pool) + LRNG_KCAPI_ALIGN,
+ GFP_KERNEL);
+ if (!lrng_pool)
+ return LRNG_SEFLTEST_ERROR_LFSR;
+ lrng_pool_aligned = PTR_ALIGN(lrng_pool, sizeof(u32));
+
+ for (i = 1; i <= LRNG_POOL_SIZE; i++)
+ _lrng_pool_lfsr_u32(lrng_pool_aligned, i);
+
+ if ((atomic_read_u32(&lrng_pool_aligned->pool[0]) !=
+ lrng_lfsr_selftest_result[CONFIG_LRNG_POOL_SIZE][0]) ||
+ (atomic_read_u32(&lrng_pool_aligned->pool[67 &
+ (LRNG_POOL_SIZE - 1)]) !=
+ lrng_lfsr_selftest_result[CONFIG_LRNG_POOL_SIZE][1]) ||
+ (atomic_read_u32(&lrng_pool_aligned->pool[LRNG_POOL_SIZE - 1]) !=
+ lrng_lfsr_selftest_result[CONFIG_LRNG_POOL_SIZE][2])) {
+ pr_err("LRNG LFSR self-test FAILED\n");
+ ret = LRNG_SEFLTEST_ERROR_LFSR;
+ }
+
+ kfree(lrng_pool);
+ return ret;
+}
+
+/*
+ * The test vectors are generated with the hash_df_testvector_generation tool
+ * provided as part of the test tool set of the LRNG.
+ */
+static unsigned int lrng_hash_df_selftest(void)
+{
+ const struct lrng_crypto_cb *crypto_cb = &lrng_cc20_crypto_cb;
+
+ /*
+ * The size of 45 bytes is chosen arbitrarily. Yet, this size should
+ * ensure that we have at least two hash blocks plus some fraction
+ * of a hash block generated.
+ */
+ static u8 const lrng_hash_df_selftest_result[][45] = {
+ {
+ 0x3b, 0xbe, 0x7a, 0xbd, 0x2b, 0x16, 0x02, 0x4c,
+ 0xfc, 0xd3, 0x02, 0x15, 0xf0, 0x86, 0xd4, 0xdb,
+ 0x49, 0xec, 0x26, 0x53, 0xd6, 0xc9, 0x6d, 0xad,
+ 0x24, 0xca, 0x72, 0x89, 0x2c, 0xfa, 0x48, 0x18,
+ 0xf7, 0x47, 0xb5, 0x2f, 0x92, 0xa2, 0x1b, 0xd9,
+ 0x24, 0xa7, 0x2f, 0xa2, 0x0b,
+ }, {
+ 0xd2, 0xaa, 0xf9, 0x76, 0x26, 0xc6, 0x13, 0xea,
+ 0xb8, 0xde, 0xe6, 0x88, 0x8f, 0xc4, 0x7a, 0x7d,
+ 0x9c, 0xb4, 0x1b, 0xd1, 0xd1, 0x8a, 0x40, 0xc9,
+ 0xaa, 0x45, 0xa6, 0xb6, 0xb5, 0x6f, 0xf6, 0xbc,
+ 0xbb, 0x77, 0x37, 0xbc, 0x5a, 0x2d, 0xcc, 0x84,
+ 0x25, 0x68, 0x5e, 0xba, 0x16,
+ }, {
+ 0x58, 0x66, 0x82, 0x88, 0x29, 0x19, 0xa4, 0xbb,
+ 0x33, 0x42, 0xc9, 0x72, 0x0d, 0x68, 0x6e, 0xb9,
+ 0xc6, 0xe0, 0x7a, 0xf9, 0x20, 0xca, 0x6d, 0x18,
+ 0x35, 0xec, 0xfa, 0x9e, 0xf6, 0x3a, 0xa7, 0xb6,
+ 0x92, 0x7a, 0xe5, 0xcd, 0xc5, 0x13, 0x9f, 0x65,
+ 0x6a, 0xe1, 0xe4, 0x3f, 0xb9,
+ }, {
+ 0xdd, 0xf1, 0x34, 0xca, 0x08, 0xe3, 0xce, 0x8a,
+ 0x26, 0x6b, 0xce, 0x99, 0x8a, 0x84, 0xd2, 0x21,
+ 0x98, 0x10, 0x95, 0x5f, 0x9f, 0xc3, 0xf2, 0xe4,
+ 0x79, 0x75, 0xb5, 0x15, 0xa7, 0xa2, 0xf1, 0xc4,
+ 0xdc, 0x67, 0xcb, 0x67, 0x8c, 0xb2, 0x1b, 0xd5,
+ 0xd6, 0x8b, 0xc2, 0x34, 0xd6,
+ }, {
+ 0xc3, 0x16, 0x9d, 0xf0, 0x78, 0x15, 0xab, 0xf2,
+ 0x2f, 0xc9, 0x2e, 0xe1, 0xc6, 0x5e, 0xfa, 0x03,
+ 0xaf, 0xd4, 0xd5, 0x47, 0x2a, 0xe8, 0x06, 0xe8,
+ 0x7e, 0x0a, 0x71, 0xc7, 0x0d, 0x39, 0xb1, 0xa9,
+ 0x5a, 0x49, 0xee, 0x8b, 0x2f, 0xcd, 0xea, 0x96,
+ 0xcc, 0x08, 0x71, 0xef, 0x9c,
+ }, {
+ 0x1a, 0x3d, 0x70, 0x39, 0xc2, 0x02, 0x4d, 0x3a,
+ 0xaa, 0x14, 0x20, 0x88, 0x96, 0x4c, 0x7c, 0xe4,
+ 0xaa, 0x49, 0x89, 0x30, 0x50, 0x96, 0xb6, 0xa7,
+ 0x55, 0x0a, 0xf8, 0xd2, 0x4e, 0x83, 0x9d, 0x1f,
+ 0x56, 0x49, 0x13, 0xc6, 0x46, 0x55, 0x73, 0x0d,
+ 0x74, 0xcd, 0x81, 0xe0, 0x65,
+ }, {
+ 0x4b, 0xf6, 0x49, 0x89, 0x2a, 0x9f, 0x67, 0xd7,
+ 0xb8, 0x1d, 0xbb, 0x5d, 0xf0, 0x1b, 0x60, 0xb6,
+ 0xb7, 0xf3, 0x86, 0x6d, 0xe0, 0x04, 0xa1, 0xbc,
+ 0x3b, 0xb0, 0x10, 0x91, 0xe8, 0x22, 0x67, 0x5b,
+ 0xe8, 0xf0, 0x4f, 0x82, 0x70, 0xc7, 0xe1, 0xc8,
+ 0xd8, 0xad, 0x70, 0xcf, 0xf6,
+ }, {
+ 0x60, 0x1f, 0x71, 0x07, 0x92, 0xae, 0xa0, 0x24,
+ 0xb6, 0xa4, 0x10, 0x70, 0x1f, 0x94, 0x51, 0x9a,
+ 0x5a, 0x81, 0xc4, 0x46, 0x78, 0x56, 0x71, 0xdd,
+ 0x45, 0x63, 0x01, 0x34, 0x87, 0x79, 0xb4, 0xd5,
+ 0x91, 0x79, 0xb9, 0x93, 0x11, 0x44, 0x50, 0xad,
+ 0x64, 0x7e, 0x5c, 0xec, 0x16,
+ }, {
+ 0x49, 0x2f, 0xa0, 0x45, 0xf8, 0xb0, 0x80, 0x88,
+ 0x79, 0xeb, 0xb6, 0x82, 0x1c, 0xf3, 0x67, 0xc4,
+ 0x88, 0x88, 0xe9, 0x75, 0x20, 0x54, 0x78, 0xc6,
+ 0x5c, 0x59, 0xcf, 0xd9, 0x73, 0x12, 0x17, 0xf4,
+ 0x30, 0x9c, 0xb7, 0x21, 0x45, 0xe2, 0xb6, 0x0c,
+ 0x0c, 0xeb, 0x1b, 0xdc, 0xdc,
+ }
+ };
+ struct lrng_pool *lrng_pool, *lrng_pool_aligned;
+ u8 hash_df[sizeof(lrng_hash_df_selftest_result[0])]
+ __aligned(sizeof(u32));
+ u32 generated;
+ int ret = 0;
+
+ BUILD_BUG_ON(ARRAY_SIZE(lrng_hash_df_selftest_result) <
+ CONFIG_LRNG_POOL_SIZE);
+
+ lrng_pool = kzalloc(sizeof(struct lrng_pool) + LRNG_KCAPI_ALIGN,
+ GFP_KERNEL);
+ if (!lrng_pool)
+ return LRNG_SEFLTEST_ERROR_HASHDF;
+ lrng_pool_aligned = PTR_ALIGN(lrng_pool, sizeof(u32));
+
+ generated = __lrng_pool_hash_df(crypto_cb, NULL, lrng_pool_aligned,
+ hash_df, sizeof(hash_df) << 3);
+
+ if ((generated >> 3) != sizeof(hash_df) ||
+ memcmp(hash_df, lrng_hash_df_selftest_result[CONFIG_LRNG_POOL_SIZE],
+ sizeof(hash_df))) {
+ pr_err("LRNG Hash DF self-test FAILED\n");
+ ret = LRNG_SEFLTEST_ERROR_HASHDF;
+ }
+
+ kfree(lrng_pool);
+ return ret;
+}
+
+/*
+ * The test vectors were generated using the ChaCha20 DRNG from
+ * https://www.chronox.de/chacha20.html
+ */
+static unsigned int lrng_chacha20_drng_selftest(void)
+{
+ const struct lrng_crypto_cb *crypto_cb = &lrng_cc20_crypto_cb;
+ static u8 const seed[CHACHA_KEY_SIZE * 2] = {
+ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+ 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+ 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+ 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+ 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
+ 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f,
+ 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f,
+ };
+ struct chacha20_block chacha20;
+ int ret;
+ u8 outbuf[CHACHA_KEY_SIZE * 2] __aligned(sizeof(u32));
+
+ /*
+ * Expected result when ChaCha20 DRNG state is zero:
+ * * constants are set to "expand 32-byte k"
+ * * remaining state is 0
+ * and pulling one half ChaCha20 DRNG block.
+ */
+ static u8 const expected_halfblock[CHACHA_KEY_SIZE] = {
+ 0x76, 0xb8, 0xe0, 0xad, 0xa0, 0xf1, 0x3d, 0x90,
+ 0x40, 0x5d, 0x6a, 0xe5, 0x53, 0x86, 0xbd, 0x28,
+ 0xbd, 0xd2, 0x19, 0xb8, 0xa0, 0x8d, 0xed, 0x1a,
+ 0xa8, 0x36, 0xef, 0xcc, 0x8b, 0x77, 0x0d, 0xc7 };
+
+ /*
+ * Expected result when ChaCha20 DRNG state is zero:
+ * * constants are set to "expand 32-byte k"
+ * * remaining state is 0
+ * followed by a reseed with two keyblocks
+ * 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+ * 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+ * 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+ * 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+ * 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
+ * 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f,
+ * 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ * 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f
+ * and pulling one ChaCha20 DRNG block.
+ */
+ static u8 const expected_oneblock[CHACHA_KEY_SIZE * 2] = {
+ 0xf5, 0xb4, 0xb6, 0x5a, 0xec, 0xcd, 0x5a, 0x65,
+ 0x87, 0x56, 0xe3, 0x86, 0x51, 0x54, 0xfc, 0x90,
+ 0x56, 0xff, 0x5e, 0xae, 0x58, 0xf2, 0x01, 0x88,
+ 0xb1, 0x7e, 0xb8, 0x2e, 0x17, 0x9a, 0x27, 0xe6,
+ 0x86, 0xb3, 0xed, 0x33, 0xf7, 0xb9, 0x06, 0x05,
+ 0x8a, 0x2d, 0x1a, 0x93, 0xc9, 0x0b, 0x80, 0x04,
+ 0x03, 0xaa, 0x60, 0xaf, 0xd5, 0x36, 0x40, 0x11,
+ 0x67, 0x89, 0xb1, 0x66, 0xd5, 0x88, 0x62, 0x6d };
+
+ /*
+ * Expected result when ChaCha20 DRNG state is zero:
+ * * constants are set to "expand 32-byte k"
+ * * remaining state is 0
+ * followed by a reseed with one key block plus one byte
+ * 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+ * 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+ * 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+ * 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+ * 0x20
+ * and pulling less than one ChaCha20 DRNG block.
+ */
+ static u8 const expected_block_nonalinged[CHACHA_KEY_SIZE + 1] = {
+ 0x3d, 0x13, 0x47, 0x1e, 0x7f, 0x7c, 0x99, 0x33,
+ 0xfc, 0x44, 0xa4, 0xdd, 0xf9, 0x3d, 0xe1, 0x9a,
+ 0xd4, 0xe8, 0x7a, 0x7d, 0x42, 0xac, 0xd1, 0xcd,
+ 0x10, 0x69, 0xe7, 0xbf, 0xd4, 0xfd, 0x69, 0x4b,
+ 0xa7 };
+
+ memset(&chacha20, 0, sizeof(chacha20));
+ lrng_cc20_init_rfc7539(&chacha20);
+
+ /* Generate with zero state */
+ ret = crypto_cb->lrng_drng_generate_helper(&chacha20, outbuf,
+ sizeof(expected_halfblock));
+ if (ret != sizeof(expected_halfblock))
+ goto err;
+ if (memcmp(outbuf, expected_halfblock, sizeof(expected_halfblock)))
+ goto err;
+
+ /* Clear state of DRNG */
+ memset(&chacha20.key.u[0], 0, 48);
+
+ /* Reseed with 2 key blocks */
+ ret = crypto_cb->lrng_drng_seed_helper(&chacha20, seed,
+ sizeof(expected_oneblock));
+ if (ret < 0)
+ goto err;
+ ret = crypto_cb->lrng_drng_generate_helper(&chacha20, outbuf,
+ sizeof(expected_oneblock));
+ if (ret != sizeof(expected_oneblock))
+ goto err;
+ if (memcmp(outbuf, expected_oneblock, sizeof(expected_oneblock)))
+ goto err;
+
+ /* Clear state of DRNG */
+ memset(&chacha20.key.u[0], 0, 48);
+
+ /* Reseed with 1 key block and one byte */
+ ret = crypto_cb->lrng_drng_seed_helper(&chacha20, seed,
+ sizeof(expected_block_nonalinged));
+ if (ret < 0)
+ goto err;
+ ret = crypto_cb->lrng_drng_generate_helper(&chacha20, outbuf,
+ sizeof(expected_block_nonalinged));
+ if (ret != sizeof(expected_block_nonalinged))
+ goto err;
+ if (memcmp(outbuf, expected_block_nonalinged,
+ sizeof(expected_block_nonalinged)))
+ goto err;
+
+ return LRNG_SELFTEST_PASSED;
+
+err:
+ pr_err("LRNG ChaCha20 DRNG self-test FAILED\n");
+ return LRNG_SEFLTEST_ERROR_CHACHA20;
+}
+
+static int lrng_selftest(void)
+{
+ unsigned int ret = lrng_time_process_selftest();
+
+ ret |= lrng_pool_lfsr_selftest();
+ ret |= lrng_chacha20_drng_selftest();
+ ret |= lrng_hash_df_selftest();
+
+ if (ret) {
+ if (IS_ENABLED(CONFIG_LRNG_SELFTEST_PANIC))
+ panic("LRNG self-tests failed: %u\n", ret);
+ } else {
+ pr_info("LRNG self-tests passed\n");
+ }
+
+ lrng_selftest_status = ret;
+
+ if (lrng_selftest_status)
+ return -EFAULT;
+ return 0;
+}
+
+#ifdef CONFIG_SYSFS
+/* Re-perform self-test when any value is written to the sysfs file. */
+static int lrng_selftest_sysfs_set(const char *val,
+ const struct kernel_param *kp)
+{
+ return lrng_selftest();
+}
+
+static const struct kernel_param_ops lrng_selftest_sysfs = {
+ .set = lrng_selftest_sysfs_set,
+ .get = param_get_uint,
+};
+module_param_cb(selftest_status, &lrng_selftest_sysfs, &lrng_selftest_status,
+ 0644);
+#endif /* CONFIG_SYSFS */
+
+static int __init lrng_selftest_init(void)
+{
+ return lrng_selftest();
+}
+
+module_init(lrng_selftest_init);
--
2.24.1
^ permalink raw reply related
* Re: KASAN: slab-out-of-bounds Read in bitmap_ip_ext_cleanup
From: syzbot @ 2020-01-20 1:35 UTC (permalink / raw)
To: a, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
allison-wcfNWazeolPR7s880joybQ, arnd-r2nGTMty4D4,
axboe-tSWWG44O7X1aa/9Udqfwiw,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
bp-Gina5bIWoIWzQB+pC5nmwQ, catalin.marinas-5wv7dgnIgG8,
chris-YvXeqwSYzG2sTnJN9+BGXg, christian-STijNZzMWpgWenYVfaLwtA,
coreteam-Cap9r6Oaw4JrovVCs/uTlw, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
elena.reshetova-ral2JQCrhuEAvxtiuMwx3w,
florent.fourcot-vJuzhzuFitlGWvitb5QawA, fw-HFFVJYpyMKqzQB+pC5nmwQ,
geert-Td1EMuHUCqxL1ZNQvxDV9g, hare-IBi9RG/b67k,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA, hpa-YMNOUZJC4hwAvxtiuMwx3w,
info-EcKl7qYKIbxeoWH0uzbU5w, jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
jeremy-7nq0j9FNiFGsTnJN9+BGXg,
johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
kadlec-Cap9r6Oaw4JrovVCs/uTlw, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
mareklindner-rVWd3aGhH2z5bpWLKbzFeg, mingo-H+wXaHxf7aLQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
pablo-Cap9r6Oaw4JrovVCs/uTlw, peterz
In-Reply-To: <000000000000bdb5b2059c865f5c-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
syzbot has bisected this bug to:
commit d68dbb0c9ac8b1ff52eb09aa58ce6358400fa939
Author: Christian Brauner <christian-STijNZzMWpgWenYVfaLwtA@public.gmane.org>
Date: Thu Jun 20 23:26:35 2019 +0000
arch: handle arches who do not yet define clone3
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1456fed1e00000
start commit: 09d4f10a net: sched: act_ctinfo: fix memory leak
git tree: net
final crash: https://syzkaller.appspot.com/x/report.txt?x=1656fed1e00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1256fed1e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=7e89bd00623fe71e
dashboard link: https://syzkaller.appspot.com/bug?extid=6491ea8f6dddbf04930e
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=141af959e00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1067fa85e00000
Reported-by: syzbot+6491ea8f6dddbf04930e-Pl5Pbv+GP7P466ipTTIvnc23WoclnBCfAL8bYrjMMd8@public.gmane.org
Fixes: d68dbb0c9ac8 ("arch: handle arches who do not yet define clone3")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Michal Hocko @ 2020-01-20 7:58 UTC (permalink / raw)
To: sspatil
Cc: kirill, minchan, akpm, linux-kernel, linux-mm, linux-api,
oleksandr, surenb, timmurray, dancol, sonnyrao, bgeffon, hannes,
shakeelb, joaodias, ktkhai, christian.brauner, sjpark
In-Reply-To: <20200119161431.GA94410@google.com>
On Sun 19-01-20 08:14:31, sspatil@google.com wrote:
> On Sat, Jan 18, 2020 at 12:26:53AM +0300, Kirill A. Shutemov wrote:
> > On Fri, Jan 17, 2020 at 09:32:39AM -0800, Minchan Kim wrote:
> > > On Fri, Jan 17, 2020 at 06:58:37PM +0300, Kirill A. Shutemov wrote:
> > > > On Fri, Jan 17, 2020 at 12:52:25PM +0100, Michal Hocko wrote:
> > > > > On Thu 16-01-20 15:59:50, Minchan Kim wrote:
> > > > > > There is usecase that System Management Software(SMS) want to give
> > > > > > a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
> > > > > > in the case of Android, it is the ActivityManagerService.
> > > > > >
> > > > > > It's similar in spirit to madvise(MADV_WONTNEED), but the information
> > > > > > required to make the reclaim decision is not known to the app. Instead,
> > > > > > it is known to the centralized userspace daemon(ActivityManagerService),
> > > > > > and that daemon must be able to initiate reclaim on its own without
> > > > > > any app involvement.
> > > > > >
> > > > > > To solve the issue, this patch introduces new syscall process_madvise(2).
> > > > > > It uses pidfd of an external processs to give the hint.
> > > > > >
> > > > > > int process_madvise(int pidfd, void *addr, size_t length, int advise,
> > > > > > unsigned long flag);
> > > > > >
> > > > > > Since it could affect other process's address range, only privileged
> > > > > > process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
> > > > > > gives it the right to ptrace the process could use it successfully.
> > > > > > The flag argument is reserved for future use if we need to extend the
> > > > > > API.
> > > > > >
> > > > > > I think supporting all hints madvise has/will supported/support to
> > > > > > process_madvise is rather risky. Because we are not sure all hints make
> > > > > > sense from external process and implementation for the hint may rely on
> > > > > > the caller being in the current context so it could be error-prone.
> > > > > > Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
> > > > > >
> > > > > > If someone want to add other hints, we could hear hear the usecase and
> > > > > > review it for each hint. It's more safe for maintainace rather than
> > > > > > introducing a buggy syscall but hard to fix it later.
> > > > >
> > > > > I have brought this up when we discussed this in the past but there is
> > > > > no reflection on that here so let me bring that up again.
> > > > >
> > > > > I believe that the interface has an inherent problem that it is racy.
> > > > > The external entity needs to know the address space layout of the target
> > > > > process to do anyhing useful on it. The address space is however under
> > > > > the full control of the target process though and the external entity
> > > > > has no means to find out that the layout has changed. So
> > > > > time-to-check-time-to-act is an inherent problem.
> > > > >
> > > > > This is a serious design flaw and it should be explained why it doesn't
> > > > > matter or how to use the interface properly to prevent that problem.
> > > >
> > > > I agree, it looks flawed.
> > > >
> > > > Also I don't see what System Management Software can generically do on
> > > > sub-process level. I mean how can it decide which part of address space is
> > > > less important than other.
> > > >
> > > > I see how a manager can indicate that this process (or a group of
> > > > processes) is less important than other, but on per-addres-range basis?
> > >
> > > For example, memory ranges shared by several processes or critical for the
> > > latency, we could avoid those ranges to be cold/pageout to prevent
> > > unncecessary CPU burning/paging.
> >
> > Hmm.. I still don't see why any external entity has a better (or any)
> > knowledge about the matter. The process has to do this, no?
>
> FWIW, I totally agree with the time-to-check-time-to-react problem. However,
> I'd like to clarify the ActivityManager/SystemServer case (I'll call it
> SystemServer from now on)
>
> For Android, every application (including the special SystemServer) are forked
> from Zygote. The reason ofcourse is to share as many libraries and classes between
> the two as possible to benefit from the preloading during boot.
>
> After applications start, (almost) all of the APIs end up calling into this
> SystemServer process over IPC (binder) and back to the application.
>
> In a fully running system, the SystemServer monitors every single process
> periodically to calculate their PSS / RSS and also decides which process is
> "important" to the user for interactivity.
>
> So, because of how these processes start _and_ the fact that the SystemServer
> is looping to monitor each process, it does tend to *know* which address
> range of the application is not used / useful.
>
> Besides, we can never rely on applications to clean things up themselves.
> We've had the "hey app1, the system is low on memory, please trim your
> memory usage down" notifications for a long time[1]. They rely on
> applications honoring the broadcasts and very few do.
>
> So, if we want to avoid the inevitable killing of the application and
> restarting it, some way to be able to tell the OS about unimportant memory in
> these applications will be useful.
This is a useful information that should be a part of the changelog. I
do see how the current form of the API might fit into Android model
without many problems. But we are not designing an API for a single
usecase, right? In a highly cooperative environments you can use ptrace
code injection as mentioned by Kirill. Or is there any fundamental
problem about that?
The interface really has to be robust to future potential usecases.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Michal Hocko @ 2020-01-20 8:03 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, LKML, linux-mm, linux-api-u79uwXL29TY76Z2rM5mHXA,
oleksandr-H+wXaHxf7aLQT0dZR+AlfA, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias,
ktkhai-5HdwGun5lf+gSpxsJD1C4w,
christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
sjpark-ebkRAfMGSJGzQB+pC5nmwQ
In-Reply-To: <20200117172542.GA140922-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On Fri 17-01-20 09:25:42, Minchan Kim wrote:
> On Fri, Jan 17, 2020 at 12:52:25PM +0100, Michal Hocko wrote:
> > On Thu 16-01-20 15:59:50, Minchan Kim wrote:
> > > There is usecase that System Management Software(SMS) want to give
> > > a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
> > > in the case of Android, it is the ActivityManagerService.
> > >
> > > It's similar in spirit to madvise(MADV_WONTNEED), but the information
> > > required to make the reclaim decision is not known to the app. Instead,
> > > it is known to the centralized userspace daemon(ActivityManagerService),
> > > and that daemon must be able to initiate reclaim on its own without
> > > any app involvement.
> > >
> > > To solve the issue, this patch introduces new syscall process_madvise(2).
> > > It uses pidfd of an external processs to give the hint.
> > >
> > > int process_madvise(int pidfd, void *addr, size_t length, int advise,
> > > unsigned long flag);
> > >
> > > Since it could affect other process's address range, only privileged
> > > process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
> > > gives it the right to ptrace the process could use it successfully.
> > > The flag argument is reserved for future use if we need to extend the
> > > API.
> > >
> > > I think supporting all hints madvise has/will supported/support to
> > > process_madvise is rather risky. Because we are not sure all hints make
> > > sense from external process and implementation for the hint may rely on
> > > the caller being in the current context so it could be error-prone.
> > > Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
> > >
> > > If someone want to add other hints, we could hear hear the usecase and
> > > review it for each hint. It's more safe for maintainace rather than
> > > introducing a buggy syscall but hard to fix it later.
> >
> > I have brought this up when we discussed this in the past but there is
> > no reflection on that here so let me bring that up again.
> >
> > I believe that the interface has an inherent problem that it is racy.
> > The external entity needs to know the address space layout of the target
> > process to do anyhing useful on it. The address space is however under
> > the full control of the target process though and the external entity
> > has no means to find out that the layout has changed. So
> > time-to-check-time-to-act is an inherent problem.
> >
> > This is a serious design flaw and it should be explained why it doesn't
> > matter or how to use the interface properly to prevent that problem.
>
> Sorry for the missing that part.
>
> It's not a particular problem of this API because other APIs already have
> done with that(e.g., move_pages, process_vm_writev).
I am sorry but this is not really an argument.
> Point is userspace has several ways for the control of target process
> like SIGSTOP, cgroup freezer or even no need to control since platform
> is already aware of that the process will never run until he grant it
> or it's resilient even though the race happens.
If you have that level of control then you can simply inject the code
via ptrace and you do not need a new syscall in the first place.
> In future, if we want to support more fine-grained consistency model
> like memory layout, we could provide some API to get cookie(e.g.,
> seq count which is updated whenever vma of the process changes). And then
> we could feed the cookie to process_madvise's last argument so that
> it can fail if founds it's not matched.
> For that API, Daniel already posted RFC - process_getinfo[1].
> https://lore.kernel.org/lkml/20190520035254.57579-1-minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org/T/#m7694416fd179b2066a2c62b5b139b14e3894e224
So why do not we start with a clean API since the beginning? I do agree
that a remote madvise is an interesting feature and it opens gates to
all sorts of userspace memory management which is not possible this
days. But the syscall has to have a reasonable semantic to allow that.
We cannot simply start with a half proken symantic first based on an
Android usecase and then hit the wall as soon as others with a different
user space model want to use it as well.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Kirill Tkhai @ 2020-01-20 10:24 UTC (permalink / raw)
To: Michal Hocko, Minchan Kim
Cc: Andrew Morton, LKML, linux-mm, linux-api-u79uwXL29TY76Z2rM5mHXA,
oleksandr-H+wXaHxf7aLQT0dZR+AlfA, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias,
christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
sjpark-ebkRAfMGSJGzQB+pC5nmwQ
In-Reply-To: <20200117115225.GV19428-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
On 17.01.2020 14:52, Michal Hocko wrote:
> On Thu 16-01-20 15:59:50, Minchan Kim wrote:
>> There is usecase that System Management Software(SMS) want to give
>> a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
>> in the case of Android, it is the ActivityManagerService.
>>
>> It's similar in spirit to madvise(MADV_WONTNEED), but the information
>> required to make the reclaim decision is not known to the app. Instead,
>> it is known to the centralized userspace daemon(ActivityManagerService),
>> and that daemon must be able to initiate reclaim on its own without
>> any app involvement.
>>
>> To solve the issue, this patch introduces new syscall process_madvise(2).
>> It uses pidfd of an external processs to give the hint.
>>
>> int process_madvise(int pidfd, void *addr, size_t length, int advise,
>> unsigned long flag);
>>
>> Since it could affect other process's address range, only privileged
>> process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
>> gives it the right to ptrace the process could use it successfully.
>> The flag argument is reserved for future use if we need to extend the
>> API.
>>
>> I think supporting all hints madvise has/will supported/support to
>> process_madvise is rather risky. Because we are not sure all hints make
>> sense from external process and implementation for the hint may rely on
>> the caller being in the current context so it could be error-prone.
>> Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
>>
>> If someone want to add other hints, we could hear hear the usecase and
>> review it for each hint. It's more safe for maintainace rather than
>> introducing a buggy syscall but hard to fix it later.
>
> I have brought this up when we discussed this in the past but there is
> no reflection on that here so let me bring that up again.
>
> I believe that the interface has an inherent problem that it is racy.
> The external entity needs to know the address space layout of the target
> process to do anyhing useful on it. The address space is however under
> the full control of the target process though and the external entity
> has no means to find out that the layout has changed. So
> time-to-check-time-to-act is an inherent problem.
>
> This is a serious design flaw and it should be explained why it doesn't
> matter or how to use the interface properly to prevent that problem.
Really, any address space manipulation, where more than one process is
involved, is racy. Even two threads on common memory need a synchronization
to manage mappings in a sane way. Managing memory from two processes
is the same in principle, and the only difference is that another level
of synchronization is required.
I'm not fan and user for this feature (at least for now, nobody knows
what will be in the future), but design flaw argument does not look
fully valid.
Regards,
Kirill
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Kirill Tkhai @ 2020-01-20 10:39 UTC (permalink / raw)
To: Michal Hocko, sspatil-hpIqsD4AKlfQT0dZR+AlfA
Cc: kirill-oKw7cIdHH8eLwutG50LtGA, minchan-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA,
oleksandr-H+wXaHxf7aLQT0dZR+AlfA, surenb-hpIqsD4AKlfQT0dZR+AlfA,
timmurray-hpIqsD4AKlfQT0dZR+AlfA, dancol-hpIqsD4AKlfQT0dZR+AlfA,
sonnyrao-hpIqsD4AKlfQT0dZR+AlfA, bgeffon-hpIqsD4AKlfQT0dZR+AlfA,
hannes-druUgvl0LCNAfugRpC6u6w, shakeelb-hpIqsD4AKlfQT0dZR+AlfA,
joaodias-hpIqsD4AKlfQT0dZR+AlfA,
christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
sjpark-ebkRAfMGSJGzQB+pC5nmwQ
In-Reply-To: <20200120075825.GH18451-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
On 20.01.2020 10:58, Michal Hocko wrote:
> On Sun 19-01-20 08:14:31, sspatil-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org wrote:
>> On Sat, Jan 18, 2020 at 12:26:53AM +0300, Kirill A. Shutemov wrote:
>>> On Fri, Jan 17, 2020 at 09:32:39AM -0800, Minchan Kim wrote:
>>>> On Fri, Jan 17, 2020 at 06:58:37PM +0300, Kirill A. Shutemov wrote:
>>>>> On Fri, Jan 17, 2020 at 12:52:25PM +0100, Michal Hocko wrote:
>>>>>> On Thu 16-01-20 15:59:50, Minchan Kim wrote:
>>>>>>> There is usecase that System Management Software(SMS) want to give
>>>>>>> a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
>>>>>>> in the case of Android, it is the ActivityManagerService.
>>>>>>>
>>>>>>> It's similar in spirit to madvise(MADV_WONTNEED), but the information
>>>>>>> required to make the reclaim decision is not known to the app. Instead,
>>>>>>> it is known to the centralized userspace daemon(ActivityManagerService),
>>>>>>> and that daemon must be able to initiate reclaim on its own without
>>>>>>> any app involvement.
>>>>>>>
>>>>>>> To solve the issue, this patch introduces new syscall process_madvise(2).
>>>>>>> It uses pidfd of an external processs to give the hint.
>>>>>>>
>>>>>>> int process_madvise(int pidfd, void *addr, size_t length, int advise,
>>>>>>> unsigned long flag);
>>>>>>>
>>>>>>> Since it could affect other process's address range, only privileged
>>>>>>> process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
>>>>>>> gives it the right to ptrace the process could use it successfully.
>>>>>>> The flag argument is reserved for future use if we need to extend the
>>>>>>> API.
>>>>>>>
>>>>>>> I think supporting all hints madvise has/will supported/support to
>>>>>>> process_madvise is rather risky. Because we are not sure all hints make
>>>>>>> sense from external process and implementation for the hint may rely on
>>>>>>> the caller being in the current context so it could be error-prone.
>>>>>>> Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
>>>>>>>
>>>>>>> If someone want to add other hints, we could hear hear the usecase and
>>>>>>> review it for each hint. It's more safe for maintainace rather than
>>>>>>> introducing a buggy syscall but hard to fix it later.
>>>>>>
>>>>>> I have brought this up when we discussed this in the past but there is
>>>>>> no reflection on that here so let me bring that up again.
>>>>>>
>>>>>> I believe that the interface has an inherent problem that it is racy.
>>>>>> The external entity needs to know the address space layout of the target
>>>>>> process to do anyhing useful on it. The address space is however under
>>>>>> the full control of the target process though and the external entity
>>>>>> has no means to find out that the layout has changed. So
>>>>>> time-to-check-time-to-act is an inherent problem.
>>>>>>
>>>>>> This is a serious design flaw and it should be explained why it doesn't
>>>>>> matter or how to use the interface properly to prevent that problem.
>>>>>
>>>>> I agree, it looks flawed.
>>>>>
>>>>> Also I don't see what System Management Software can generically do on
>>>>> sub-process level. I mean how can it decide which part of address space is
>>>>> less important than other.
>>>>>
>>>>> I see how a manager can indicate that this process (or a group of
>>>>> processes) is less important than other, but on per-addres-range basis?
>>>>
>>>> For example, memory ranges shared by several processes or critical for the
>>>> latency, we could avoid those ranges to be cold/pageout to prevent
>>>> unncecessary CPU burning/paging.
>>>
>>> Hmm.. I still don't see why any external entity has a better (or any)
>>> knowledge about the matter. The process has to do this, no?
>>
>> FWIW, I totally agree with the time-to-check-time-to-react problem. However,
>> I'd like to clarify the ActivityManager/SystemServer case (I'll call it
>> SystemServer from now on)
>>
>> For Android, every application (including the special SystemServer) are forked
>> from Zygote. The reason ofcourse is to share as many libraries and classes between
>> the two as possible to benefit from the preloading during boot.
>>
>> After applications start, (almost) all of the APIs end up calling into this
>> SystemServer process over IPC (binder) and back to the application.
>>
>> In a fully running system, the SystemServer monitors every single process
>> periodically to calculate their PSS / RSS and also decides which process is
>> "important" to the user for interactivity.
>>
>> So, because of how these processes start _and_ the fact that the SystemServer
>> is looping to monitor each process, it does tend to *know* which address
>> range of the application is not used / useful.
>>
>> Besides, we can never rely on applications to clean things up themselves.
>> We've had the "hey app1, the system is low on memory, please trim your
>> memory usage down" notifications for a long time[1]. They rely on
>> applications honoring the broadcasts and very few do.
>>
>> So, if we want to avoid the inevitable killing of the application and
>> restarting it, some way to be able to tell the OS about unimportant memory in
>> these applications will be useful.
>
> This is a useful information that should be a part of the changelog. I
> do see how the current form of the API might fit into Android model
> without many problems. But we are not designing an API for a single
> usecase, right? In a highly cooperative environments you can use ptrace
> code injection as mentioned by Kirill. Or is there any fundamental
> problem about that?
There could be only problems with multi-threads applications, which
poll the state of another threads (and exit with error, when they
found that someone is traced). But this may be workarounded by freezer
cgroup making all the thread group is frozen. It should guarantee
consistent state of the whole process after attaching.
Kirill
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Michal Hocko @ 2020-01-20 11:27 UTC (permalink / raw)
To: Kirill Tkhai
Cc: Minchan Kim, Andrew Morton, LKML, linux-mm,
linux-api-u79uwXL29TY76Z2rM5mHXA,
oleksandr-H+wXaHxf7aLQT0dZR+AlfA, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias,
christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
sjpark-ebkRAfMGSJGzQB+pC5nmwQ
In-Reply-To: <f57fb198-4070-d3b4-b6bd-43b29ff40a2c-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>
On Mon 20-01-20 13:24:35, Kirill Tkhai wrote:
> On 17.01.2020 14:52, Michal Hocko wrote:
> > On Thu 16-01-20 15:59:50, Minchan Kim wrote:
> >> There is usecase that System Management Software(SMS) want to give
> >> a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
> >> in the case of Android, it is the ActivityManagerService.
> >>
> >> It's similar in spirit to madvise(MADV_WONTNEED), but the information
> >> required to make the reclaim decision is not known to the app. Instead,
> >> it is known to the centralized userspace daemon(ActivityManagerService),
> >> and that daemon must be able to initiate reclaim on its own without
> >> any app involvement.
> >>
> >> To solve the issue, this patch introduces new syscall process_madvise(2).
> >> It uses pidfd of an external processs to give the hint.
> >>
> >> int process_madvise(int pidfd, void *addr, size_t length, int advise,
> >> unsigned long flag);
> >>
> >> Since it could affect other process's address range, only privileged
> >> process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
> >> gives it the right to ptrace the process could use it successfully.
> >> The flag argument is reserved for future use if we need to extend the
> >> API.
> >>
> >> I think supporting all hints madvise has/will supported/support to
> >> process_madvise is rather risky. Because we are not sure all hints make
> >> sense from external process and implementation for the hint may rely on
> >> the caller being in the current context so it could be error-prone.
> >> Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
> >>
> >> If someone want to add other hints, we could hear hear the usecase and
> >> review it for each hint. It's more safe for maintainace rather than
> >> introducing a buggy syscall but hard to fix it later.
> >
> > I have brought this up when we discussed this in the past but there is
> > no reflection on that here so let me bring that up again.
> >
> > I believe that the interface has an inherent problem that it is racy.
> > The external entity needs to know the address space layout of the target
> > process to do anyhing useful on it. The address space is however under
> > the full control of the target process though and the external entity
> > has no means to find out that the layout has changed. So
> > time-to-check-time-to-act is an inherent problem.
> >
> > This is a serious design flaw and it should be explained why it doesn't
> > matter or how to use the interface properly to prevent that problem.
>
> Really, any address space manipulation, where more than one process is
> involved, is racy.
They are, indeed. But that is not the point I wanted to make.
> Even two threads on common memory need a synchronization
> to manage mappings in a sane way. Managing memory from two processes
> is the same in principle, and the only difference is that another level
> of synchronization is required.
Well, not really. The operation might simply attempt to perform an
operation on a specific memory area and get a failure if it doesn't
reference the same object anymore. What I think we need is some form of
a handle to operate on. In the past we have discussed several
directions. I was proposing /proc/self/map_anon/ (analogous to
map_files) where you could inspect anonymous memory and get a file
handle for it. madvise would then operate on the fd and then there
shouldn't be a real problem to revalidate that the object is still
valid. But there was no general enthusiasm about that approach. There
are likely some land mines on the way.
There was another approach mentioned by Minchan in other email in this
thread.
What I want to say is that I believe a remove madvise can have a
sensible semantic even without a strong synchronization between the
monitor and the target task. We just have to make sure that the monitor
never operates on a different object then it believes it acts on.
On the other hand, there will never be any way to make this interface
reasonable for destructive operations though because the content of the
memory needs a strong synchronization IMHO.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Kirill A. Shutemov @ 2020-01-20 12:39 UTC (permalink / raw)
To: Michal Hocko
Cc: Kirill Tkhai, Minchan Kim, Andrew Morton, LKML, linux-mm,
linux-api, oleksandr, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias, christian.brauner,
sjpark
In-Reply-To: <20200120112722.GY18451@dhcp22.suse.cz>
On Mon, Jan 20, 2020 at 12:27:22PM +0100, Michal Hocko wrote:
> On Mon 20-01-20 13:24:35, Kirill Tkhai wrote:
> > On 17.01.2020 14:52, Michal Hocko wrote:
> > > On Thu 16-01-20 15:59:50, Minchan Kim wrote:
> > >> There is usecase that System Management Software(SMS) want to give
> > >> a memory hint like MADV_[COLD|PAGEEOUT] to other processes and
> > >> in the case of Android, it is the ActivityManagerService.
> > >>
> > >> It's similar in spirit to madvise(MADV_WONTNEED), but the information
> > >> required to make the reclaim decision is not known to the app. Instead,
> > >> it is known to the centralized userspace daemon(ActivityManagerService),
> > >> and that daemon must be able to initiate reclaim on its own without
> > >> any app involvement.
> > >>
> > >> To solve the issue, this patch introduces new syscall process_madvise(2).
> > >> It uses pidfd of an external processs to give the hint.
> > >>
> > >> int process_madvise(int pidfd, void *addr, size_t length, int advise,
> > >> unsigned long flag);
> > >>
> > >> Since it could affect other process's address range, only privileged
> > >> process(CAP_SYS_PTRACE) or something else(e.g., being the same UID)
> > >> gives it the right to ptrace the process could use it successfully.
> > >> The flag argument is reserved for future use if we need to extend the
> > >> API.
> > >>
> > >> I think supporting all hints madvise has/will supported/support to
> > >> process_madvise is rather risky. Because we are not sure all hints make
> > >> sense from external process and implementation for the hint may rely on
> > >> the caller being in the current context so it could be error-prone.
> > >> Thus, I just limited hints as MADV_[COLD|PAGEOUT] in this patch.
> > >>
> > >> If someone want to add other hints, we could hear hear the usecase and
> > >> review it for each hint. It's more safe for maintainace rather than
> > >> introducing a buggy syscall but hard to fix it later.
> > >
> > > I have brought this up when we discussed this in the past but there is
> > > no reflection on that here so let me bring that up again.
> > >
> > > I believe that the interface has an inherent problem that it is racy.
> > > The external entity needs to know the address space layout of the target
> > > process to do anyhing useful on it. The address space is however under
> > > the full control of the target process though and the external entity
> > > has no means to find out that the layout has changed. So
> > > time-to-check-time-to-act is an inherent problem.
> > >
> > > This is a serious design flaw and it should be explained why it doesn't
> > > matter or how to use the interface properly to prevent that problem.
> >
> > Really, any address space manipulation, where more than one process is
> > involved, is racy.
>
> They are, indeed. But that is not the point I wanted to make.
>
> > Even two threads on common memory need a synchronization
> > to manage mappings in a sane way. Managing memory from two processes
> > is the same in principle, and the only difference is that another level
> > of synchronization is required.
>
> Well, not really. The operation might simply attempt to perform an
> operation on a specific memory area and get a failure if it doesn't
> reference the same object anymore. What I think we need is some form of
> a handle to operate on. In the past we have discussed several
> directions. I was proposing /proc/self/map_anon/ (analogous to
> map_files) where you could inspect anonymous memory and get a file
> handle for it. madvise would then operate on the fd and then there
> shouldn't be a real problem to revalidate that the object is still
> valid. But there was no general enthusiasm about that approach. There
> are likely some land mines on the way.
Converting anon memory to file-backed is bad idea and going to backfire.
It will break many things around rmap (for THP in particular). We have
assumption that an anon page cannot be mapped twice in the same process.
Breaking rules around memory inheritance is not helpful too.
It is a major re-design of the subsystem and I don't see any real need for
this.
--
Kirill A. Shutemov
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Read in bitmap_ip_ext_cleanup
From: Christian Brauner @ 2020-01-20 13:19 UTC (permalink / raw)
To: syzbot
Cc: a, akpm, allison, arnd, axboe, b.a.t.m.a.n, bp, catalin.marinas,
chris, christian, coreteam, davem, elena.reshetova,
florent.fourcot, fw, geert, hare, heiko.carstens, hpa, info,
jcmvbkbc, jeremy, johannes.berg, kadlec, linux-api,
linux-arm-kernel, linux-kernel, linux-xtensa, linux, mareklindner,
mingo, netdev, netfilter-devel, pablo, peterz
In-Reply-To: <000000000000c795fa059c884c21@google.com>
On Sun, Jan 19, 2020 at 05:35:01PM -0800, syzbot wrote:
> syzbot has bisected this bug to:
>
> commit d68dbb0c9ac8b1ff52eb09aa58ce6358400fa939
> Author: Christian Brauner <christian@brauner.io>
> Date: Thu Jun 20 23:26:35 2019 +0000
>
> arch: handle arches who do not yet define clone3
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1456fed1e00000
> start commit: 09d4f10a net: sched: act_ctinfo: fix memory leak
> git tree: net
> final crash: https://syzkaller.appspot.com/x/report.txt?x=1656fed1e00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=1256fed1e00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=7e89bd00623fe71e
> dashboard link: https://syzkaller.appspot.com/bug?extid=6491ea8f6dddbf04930e
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=141af959e00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1067fa85e00000
>
> Reported-by: syzbot+6491ea8f6dddbf04930e@syzkaller.appspotmail.com
> Fixes: d68dbb0c9ac8 ("arch: handle arches who do not yet define clone3")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
This bisect seems bogus.
Christian
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Michal Hocko @ 2020-01-20 13:24 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill Tkhai, Minchan Kim, Andrew Morton, LKML, linux-mm,
linux-api-u79uwXL29TY76Z2rM5mHXA,
oleksandr-H+wXaHxf7aLQT0dZR+AlfA, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias,
christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
sjpark-ebkRAfMGSJGzQB+pC5nmwQ
In-Reply-To: <20200120123935.onlls7enjtzenbvt@box>
On Mon 20-01-20 15:39:35, Kirill A. Shutemov wrote:
> On Mon, Jan 20, 2020 at 12:27:22PM +0100, Michal Hocko wrote:
> > On Mon 20-01-20 13:24:35, Kirill Tkhai wrote:
[...]
> > > Even two threads on common memory need a synchronization
> > > to manage mappings in a sane way. Managing memory from two processes
> > > is the same in principle, and the only difference is that another level
> > > of synchronization is required.
> >
> > Well, not really. The operation might simply attempt to perform an
> > operation on a specific memory area and get a failure if it doesn't
> > reference the same object anymore. What I think we need is some form of
> > a handle to operate on. In the past we have discussed several
> > directions. I was proposing /proc/self/map_anon/ (analogous to
> > map_files) where you could inspect anonymous memory and get a file
> > handle for it. madvise would then operate on the fd and then there
> > shouldn't be a real problem to revalidate that the object is still
> > valid. But there was no general enthusiasm about that approach. There
> > are likely some land mines on the way.
>
> Converting anon memory to file-backed is bad idea and going to backfire.
I didn't mean to convert. I meant to expose that information via proc
the same way we do for file backed mappings. That shouldn't really
require to re-design the way how anonymous vma work IMO. But I haven't
tried that so there might be many gotchas there.
There are obvious things to think about though. Such fd cannot be sent
to other processes (SCM stuff), mmap of the file would have to be
disallowed and many others I am not aware of. I am not even pushing this
direction because I am not convinced about how viable it is myself. But
it would sound like a nice extension of the existing mechanism we have
and a file based madvise sounds attractive to me as well because we
already have that.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 3/6] cgroup: refactor fork helpers
From: Oleg Nesterov @ 2020-01-20 14:00 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-api, linux-kernel, Tejun Heo, Johannes Weiner, Li Zefan,
cgroups
In-Reply-To: <20200117181219.14542-4-christian.brauner@ubuntu.com>
This is probably the only patch in series I can understand ;)
To me it looks like a good cleanup regardless, but
On 01/17, Christian Brauner wrote:
>
> The patch just passes in the parent task_struct
For what? "parent" is always "current", no?
Oleg.
^ permalink raw reply
* Re: [PATCH v4 3/6] cgroup: refactor fork helpers
From: Christian Brauner @ 2020-01-20 14:04 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-api, linux-kernel, Tejun Heo, Johannes Weiner, Li Zefan,
cgroups
In-Reply-To: <20200120140029.GB30403@redhat.com>
On Mon, Jan 20, 2020 at 03:00:30PM +0100, Oleg Nesterov wrote:
> This is probably the only patch in series I can understand ;)
>
> To me it looks like a good cleanup regardless, but
>
> On 01/17, Christian Brauner wrote:
> >
> > The patch just passes in the parent task_struct
>
> For what? "parent" is always "current", no?
Yes. What exactly are you hinting at? :) Would you prefer that the
commit message speaks of "current" instead of "parent"?
Christian
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Kirill A. Shutemov @ 2020-01-20 14:21 UTC (permalink / raw)
To: Michal Hocko
Cc: Kirill Tkhai, Minchan Kim, Andrew Morton, LKML, linux-mm,
linux-api, oleksandr, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias, christian.brauner,
sjpark
In-Reply-To: <20200120132405.GF18451@dhcp22.suse.cz>
On Mon, Jan 20, 2020 at 02:24:05PM +0100, Michal Hocko wrote:
> On Mon 20-01-20 15:39:35, Kirill A. Shutemov wrote:
> > On Mon, Jan 20, 2020 at 12:27:22PM +0100, Michal Hocko wrote:
> > > On Mon 20-01-20 13:24:35, Kirill Tkhai wrote:
> [...]
> > > > Even two threads on common memory need a synchronization
> > > > to manage mappings in a sane way. Managing memory from two processes
> > > > is the same in principle, and the only difference is that another level
> > > > of synchronization is required.
> > >
> > > Well, not really. The operation might simply attempt to perform an
> > > operation on a specific memory area and get a failure if it doesn't
> > > reference the same object anymore. What I think we need is some form of
> > > a handle to operate on. In the past we have discussed several
> > > directions. I was proposing /proc/self/map_anon/ (analogous to
> > > map_files) where you could inspect anonymous memory and get a file
> > > handle for it. madvise would then operate on the fd and then there
> > > shouldn't be a real problem to revalidate that the object is still
> > > valid. But there was no general enthusiasm about that approach. There
> > > are likely some land mines on the way.
> >
> > Converting anon memory to file-backed is bad idea and going to backfire.
>
> I didn't mean to convert. I meant to expose that information via proc
> the same way we do for file backed mappings. That shouldn't really
> require to re-design the way how anonymous vma work IMO. But I haven't
> tried that so there might be many gotchas there.
>
> There are obvious things to think about though. Such fd cannot be sent
> to other processes (SCM stuff), mmap of the file would have to be
> disallowed and many others I am not aware of. I am not even pushing this
> direction because I am not convinced about how viable it is myself. But
> it would sound like a nice extension of the existing mechanism we have
> and a file based madvise sounds attractive to me as well because we
> already have that.
If the fd cannot be passed around or mmaped what does it represent?
And how is it different from plain address?
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v4 3/6] cgroup: refactor fork helpers
From: Oleg Nesterov @ 2020-01-20 14:22 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-api, linux-kernel, Tejun Heo, Johannes Weiner, Li Zefan,
cgroups
In-Reply-To: <20200120140452.qyjogmmhyqc3gxon@wittgenstein>
On 01/20, Christian Brauner wrote:
>
> On Mon, Jan 20, 2020 at 03:00:30PM +0100, Oleg Nesterov wrote:
> > This is probably the only patch in series I can understand ;)
> >
> > To me it looks like a good cleanup regardless, but
> >
> > On 01/17, Christian Brauner wrote:
> > >
> > > The patch just passes in the parent task_struct
> >
> > For what? "parent" is always "current", no?
>
> Yes. What exactly are you hinting at? :) Would you prefer that the
> commit message speaks of "current" instead of "parent"?
I meant, I don't understand why did you add the new "parent" arg,
cgroup_xxx_fork() can simply use "current" ?
Oleg.
^ permalink raw reply
* Re: [PATCH v4 3/6] cgroup: refactor fork helpers
From: Christian Brauner @ 2020-01-20 14:33 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Johannes Weiner,
Li Zefan, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200120142202.GC30403-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Jan 20, 2020 at 03:22:03PM +0100, Oleg Nesterov wrote:
> On 01/20, Christian Brauner wrote:
> >
> > On Mon, Jan 20, 2020 at 03:00:30PM +0100, Oleg Nesterov wrote:
> > > This is probably the only patch in series I can understand ;)
> > >
> > > To me it looks like a good cleanup regardless, but
> > >
> > > On 01/17, Christian Brauner wrote:
> > > >
> > > > The patch just passes in the parent task_struct
> > >
> > > For what? "parent" is always "current", no?
> >
> > Yes. What exactly are you hinting at? :) Would you prefer that the
> > commit message speaks of "current" instead of "parent"?
>
> I meant, I don't understand why did you add the new "parent" arg,
> cgroup_xxx_fork() can simply use "current" ?
There's no specific reason behind it. I don't care much whether it's an
arg or we just grab current in each helper directly.
Christian
^ permalink raw reply
* Re: [PATCH v4 1/6] cgroup: unify attach permission checking
From: Oleg Nesterov @ 2020-01-20 14:42 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Li Zefan,
Johannes Weiner, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200117181219.14542-2-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
I guess I am totally confused, but...
On 01/17, Christian Brauner wrote:
>
> +static inline bool cgroup_same_domain(const struct cgroup *src_cgrp,
> + const struct cgroup *dst_cgrp)
> +{
> + return src_cgrp->dom_cgrp == dst_cgrp->dom_cgrp;
> +}
> +
> +static int cgroup_attach_permissions(struct cgroup *src_cgrp,
> + struct cgroup *dst_cgrp,
> + struct super_block *sb, bool thread)
> +{
> + int ret = 0;
> +
> + ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb);
> + if (ret)
> + return ret;
> +
> + ret = cgroup_migrate_vet_dst(dst_cgrp);
> + if (ret)
> + return ret;
> +
> + if (thread &&
> + !cgroup_same_domain(src_cgrp->dom_cgrp, dst_cgrp->dom_cgrp))
^^^^^^^^^^ ^^^^^^^^^^
cgroup_same_domain(src_cgrp, dst_cgrp)
no?
And given that cgroup_same_domain() has no other users, perhaps it can
simply check
src_cgrp->dom_cgrp != dst_cgrp->dom_cgrp
?
Oleg.
^ permalink raw reply
* Re: [PATCH v4 1/6] cgroup: unify attach permission checking
From: Christian Brauner @ 2020-01-20 14:46 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Li Zefan,
Johannes Weiner, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200120144244.GD30403-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Jan 20, 2020 at 03:42:45PM +0100, Oleg Nesterov wrote:
> I guess I am totally confused, but...
>
> On 01/17, Christian Brauner wrote:
> >
> > +static inline bool cgroup_same_domain(const struct cgroup *src_cgrp,
> > + const struct cgroup *dst_cgrp)
> > +{
> > + return src_cgrp->dom_cgrp == dst_cgrp->dom_cgrp;
> > +}
> > +
> > +static int cgroup_attach_permissions(struct cgroup *src_cgrp,
> > + struct cgroup *dst_cgrp,
> > + struct super_block *sb, bool thread)
> > +{
> > + int ret = 0;
> > +
> > + ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb);
> > + if (ret)
> > + return ret;
> > +
> > + ret = cgroup_migrate_vet_dst(dst_cgrp);
> > + if (ret)
> > + return ret;
> > +
> > + if (thread &&
> > + !cgroup_same_domain(src_cgrp->dom_cgrp, dst_cgrp->dom_cgrp))
> ^^^^^^^^^^ ^^^^^^^^^^
>
> cgroup_same_domain(src_cgrp, dst_cgrp)
>
> no?
>
> And given that cgroup_same_domain() has no other users, perhaps it can
> simply check
>
> src_cgrp->dom_cgrp != dst_cgrp->dom_cgrp
Yeah, I just added it because the helper is very descriptive given its
name. Maybe too descriptive given my braino.
I'll just remove it in favor of this check and give it a small comment.
Thanks!
Christian
^ permalink raw reply
* Re: [PATCH v4 5/6] clone3: allow spawning processes into cgroups
From: Oleg Nesterov @ 2020-01-20 15:39 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Ingo Molnar,
Johannes Weiner, Li Zefan, Peter Zijlstra,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200117181219.14542-6-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
On 01/17, Christian Brauner wrote:
>
> +static int cgroup_css_set_fork(struct task_struct *parent,
> + struct kernel_clone_args *kargs)
...
> + kargs->cset = find_css_set(cset, dst_cgrp);
> + if (!kargs->cset) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + if (cgroup_is_dead(dst_cgrp)) {
> + ret = -ENODEV;
> + goto err;
^^^^^^^^
this looks wrong... don't we need put_css_set(kargs->cset) before "goto err" ?
Oleg.
^ permalink raw reply
* Re: [PATCH v2 2/5] mm: introduce external memory hinting API
From: Michal Hocko @ 2020-01-20 15:44 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Kirill Tkhai, Minchan Kim, Andrew Morton, LKML, linux-mm,
linux-api-u79uwXL29TY76Z2rM5mHXA,
oleksandr-H+wXaHxf7aLQT0dZR+AlfA, Suren Baghdasaryan, Tim Murray,
Daniel Colascione, Sandeep Patil, Sonny Rao, Brian Geffon,
Johannes Weiner, Shakeel Butt, John Dias,
christian.brauner-GeWIH/nMZzLQT0dZR+AlfA,
sjpark-ebkRAfMGSJGzQB+pC5nmwQ
In-Reply-To: <20200120142132.srf4igph4zmecu7b@box>
On Mon 20-01-20 17:21:32, Kirill A. Shutemov wrote:
> On Mon, Jan 20, 2020 at 02:24:05PM +0100, Michal Hocko wrote:
> > On Mon 20-01-20 15:39:35, Kirill A. Shutemov wrote:
> > > On Mon, Jan 20, 2020 at 12:27:22PM +0100, Michal Hocko wrote:
> > > > On Mon 20-01-20 13:24:35, Kirill Tkhai wrote:
> > [...]
> > > > > Even two threads on common memory need a synchronization
> > > > > to manage mappings in a sane way. Managing memory from two processes
> > > > > is the same in principle, and the only difference is that another level
> > > > > of synchronization is required.
> > > >
> > > > Well, not really. The operation might simply attempt to perform an
> > > > operation on a specific memory area and get a failure if it doesn't
> > > > reference the same object anymore. What I think we need is some form of
> > > > a handle to operate on. In the past we have discussed several
> > > > directions. I was proposing /proc/self/map_anon/ (analogous to
> > > > map_files) where you could inspect anonymous memory and get a file
> > > > handle for it. madvise would then operate on the fd and then there
> > > > shouldn't be a real problem to revalidate that the object is still
> > > > valid. But there was no general enthusiasm about that approach. There
> > > > are likely some land mines on the way.
> > >
> > > Converting anon memory to file-backed is bad idea and going to backfire.
> >
> > I didn't mean to convert. I meant to expose that information via proc
> > the same way we do for file backed mappings. That shouldn't really
> > require to re-design the way how anonymous vma work IMO. But I haven't
> > tried that so there might be many gotchas there.
> >
> > There are obvious things to think about though. Such fd cannot be sent
> > to other processes (SCM stuff), mmap of the file would have to be
> > disallowed and many others I am not aware of. I am not even pushing this
> > direction because I am not convinced about how viable it is myself. But
> > it would sound like a nice extension of the existing mechanism we have
> > and a file based madvise sounds attractive to me as well because we
> > already have that.
>
> If the fd cannot be passed around or mmaped what does it represent?
Because it is a cookie maintained by the kernel.
> And how is it different from plain address?
Kernel can revalidate that the given fd is denoting the vma it was
created for and simply fail with ENOENT or whatever suits if somebody
did unmap and mmap to the same address.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Read in bitmap_ip_ext_cleanup
From: Dan Carpenter @ 2020-01-20 17:46 UTC (permalink / raw)
To: Christian Brauner
Cc: mareklindner, peterz, catalin.marinas, a, jcmvbkbc, syzbot, hpa,
will, elena.reshetova, hare, johannes.berg, florent.fourcot, x86,
linux, kadlec, coreteam, geert, allison, pablo, linux-arm-kernel,
linux-xtensa, arnd, heiko.carstens, syzkaller-bugs, bp, viro,
tglx, mingo, christian, axboe, chris, jeremy, sw, linux-api,
b.a.t.m.a.n, fw, linux-kernel
In-Reply-To: <20200120131930.pbhbsrm4bk4lq3d7@wittgenstein>
On Mon, Jan 20, 2020 at 02:19:31PM +0100, Christian Brauner wrote:
> On Sun, Jan 19, 2020 at 05:35:01PM -0800, syzbot wrote:
> > syzbot has bisected this bug to:
> >
> > commit d68dbb0c9ac8b1ff52eb09aa58ce6358400fa939
> > Author: Christian Brauner <christian@brauner.io>
> > Date: Thu Jun 20 23:26:35 2019 +0000
> >
> > arch: handle arches who do not yet define clone3
> >
> > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1456fed1e00000
> > start commit: 09d4f10a net: sched: act_ctinfo: fix memory leak
> > git tree: net
> > final crash: https://syzkaller.appspot.com/x/report.txt?x=1656fed1e00000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1256fed1e00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=7e89bd00623fe71e
> > dashboard link: https://syzkaller.appspot.com/bug?extid=6491ea8f6dddbf04930e
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=141af959e00000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1067fa85e00000
> >
> > Reported-by: syzbot+6491ea8f6dddbf04930e@syzkaller.appspotmail.com
> > Fixes: d68dbb0c9ac8 ("arch: handle arches who do not yet define clone3")
> >
> > For information about bisection process see: https://goo.gl/tpsmEJ#bisection
>
> This bisect seems bogus.
>
Yeah. József Kadlecsik already fixed the bug in a different thread. It
was reported as seven different bugs so there was a bunch of threads for
it.
regards,
dan carpenter
^ permalink raw reply
* Contact Diplomatic Agent, Mr. Mcclaine John to receive your ATM CARD valued the sum of $12.8Million United States Dollars
From: Prof, William Roberts @ 2020-01-20 19:31 UTC (permalink / raw)
Attn: Dear Beneficiary,
I wish to inform you that the diplomatic agent conveying your ATM CARD
valued the sum of $12.8Million United States Dollars has misplaced
your address and he is currently stranded at (George Bush
International Airport) Houston Texas USA now
We required you to reconfirm the following information's below to him
so that he can deliver your Payment CARD to you today or tomorrow
morning as information provided with open communications via email and
telephone for security reasons.
HERE IS THE DETAILS HE NEED FROM YOU URGENT
YOUR FULL NAME:========
ADDRESS:========
MOBILE NO:========
NAME OF YOUR NEAREST AIRPORT:========
A COPY OF YOUR IDENTIFICATION :========
Note; do contact the diplomatic agent immediately through the
information's listed below
Contact Person: Diplomatic Agent, Mr. Mcclaine John
EMAIL: mcclainejohn.13-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Tel:(223) 777-7518
Contact the diplomatic agent immediately
because he is waiting to hear from you today with the needed information's.
NOTE: The Diplomatic agent does not know that the content of the
consignment box is $12.800,000,00 Million United States Dollars and on
no circumstances should you let him know the content. The consignment
was moved from here as family treasures, so never allow him to open
the box. Please I have paid delivery fees for you but the only money
you must send to Mcclaine John is your ATM CARD delivery fee $25.00
only. text Him as you contact Him Immediately
Thanks,
with Regards.
Prof, William Roberts
Director DHL COURIER SERVICES-Benin
^ permalink raw reply
* Re: [PATCH v4 5/6] clone3: allow spawning processes into cgroups
From: Christian Brauner @ 2020-01-20 21:38 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-api, linux-kernel, Tejun Heo, Ingo Molnar, Johannes Weiner,
Li Zefan, Peter Zijlstra, cgroups
In-Reply-To: <20200120153930.GE30403@redhat.com>
On Mon, Jan 20, 2020 at 04:39:30PM +0100, Oleg Nesterov wrote:
> On 01/17, Christian Brauner wrote:
> >
> > +static int cgroup_css_set_fork(struct task_struct *parent,
> > + struct kernel_clone_args *kargs)
> ...
> > + kargs->cset = find_css_set(cset, dst_cgrp);
> > + if (!kargs->cset) {
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + if (cgroup_is_dead(dst_cgrp)) {
> > + ret = -ENODEV;
> > + goto err;
> ^^^^^^^^
>
> this looks wrong... don't we need put_css_set(kargs->cset) before "goto err" ?
Yeah, but we should rather do:
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 4d36255ef25f..482055d1e64a 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5994,6 +5994,8 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs)
if (dst_cgrp)
cgroup_put(dst_cgrp);
put_css_set(cset);
+ if (kargs->cset)
+ put_css_set(kargs->cset);
return ret;
}
Christian
^ permalink raw reply related
* [PATCH v5 0/6] clone3 & cgroups: allow spawning processes into cgroups
From: Christian Brauner @ 2020-01-21 15:48 UTC (permalink / raw)
To: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
Cc: Oleg Nesterov, Christian Brauner
Hey Tejun,
This is v5 of the promised series to enable spawning processes into a
target cgroup different from the parent's cgroup.
/* v1 */
Link: https://lore.kernel.org/r/20191218173516.7875-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org
/* v2 */
Link: https://lore.kernel.org/r/20191223061504.28716-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org
Rework locking and remove unneeded helper functions. Please see
individual patch changelogs for details.
With this I've been able to run the cgroup selftests and stress tests in
loops for a long time without any regressions or deadlocks; lockdep and
kasan did not complain either.
/* v3 */
Link: https://lore.kernel.org/r/20200117002143.15559-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org
Split preliminary work into separate patches.
See changelog of individual commits.
/* v4 */
Link: https://lore.kernel.org/r/20200117181219.14542-1-christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org
Verify that we have write access to the target cgroup. This is usually
done by the vfs but since we aren't going through the vfs with
CLONE_INTO_CGROUP we need to do it ourselves.
/* v5 */
Don't pass down the parent task_struct as argument, just use current
directly. Put kargs->cset on error.
With this cgroup migration will be a lot easier, and accounting will be
more exact. It also allows for nice features such as creating a frozen
process by spawning it into a frozen cgroup.
The code simplifies container creation and exec logic quite a bit as
well.
I've tried to contain all core changes for this features in
kernel/cgroup/* to avoid exposing cgroup internals. This has mostly
worked.
When a new process is supposed to be spawned in a cgroup different from
the parent's then we briefly acquire the cgroup mutex right before
fork()'s point of no return and drop it once the child process has been
attached to the tasklist and to its css_set. This is done to ensure that
the cgroup isn't removed behind our back. The cgroup mutex is _only_
held in this case; the usual case, where the child is created in the
same cgroup as the parent does not acquire it since the cgroup can't be
removed.
The series already comes with proper testing. Once we've decided that
this approach is good I'll expand the test-suite even more.
The branch can be found in the following locations:
[1]: kernel.org: https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=clone_into_cgroup
[2]: github.com: https://github.com/brauner/linux/tree/clone_into_cgroup
[3]: gitlab.com: https://gitlab.com/brauner/linux/commits/clone_into_cgroup
Thanks!
Christian
Christian Brauner (6):
cgroup: unify attach permission checking
cgroup: add cgroup_get_from_file() helper
cgroup: refactor fork helpers
cgroup: add cgroup_may_write() helper
clone3: allow spawning processes into cgroups
selftests/cgroup: add tests for cloning into cgroups
include/linux/cgroup-defs.h | 6 +-
include/linux/cgroup.h | 20 +-
include/linux/sched/task.h | 4 +
include/uapi/linux/sched.h | 5 +
kernel/cgroup/cgroup.c | 297 ++++++++++++++----
kernel/cgroup/pids.c | 15 +-
kernel/fork.c | 19 +-
tools/testing/selftests/cgroup/Makefile | 6 +-
tools/testing/selftests/cgroup/cgroup_util.c | 126 ++++++++
tools/testing/selftests/cgroup/cgroup_util.h | 4 +
tools/testing/selftests/cgroup/test_core.c | 64 ++++
.../selftests/clone3/clone3_selftests.h | 19 +-
12 files changed, 501 insertions(+), 84 deletions(-)
base-commit: b3a987b0264d3ddbb24293ebff10eddfc472f653
--
2.25.0
^ 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