[PATCH RFC v1] random: implement getrandom() in vDSO

[PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Two statements:

  1) Userspace wants faster cryptographically secure numbers of
     arbitrary size, big or small.

  2) Userspace is currently unable to safely roll its own RNG with the
     same security profile as getrandom().

Statement (1) has been debated for years, with arguments ranging from
"we need faster cryptographically secure card shuffling!" to "the only
things that actually need good randomness are keys, which are few and
far between" to "actually, TLS CBC nonces are frequent" and so on. I
don't intend to wade into that debate substantially, except to note that
recently glibc added arc4random(), whose goal is to return a
cryptographically secure uint32_t. So here we are.

Statement (2) is more interesting. The kernel is the nexus of all
entropic inputs that influence the RNG. It is in the best position, and
probably the only position, to decide anything at all about the current
state of the RNG and of its entropy. One of the things it uniquely knows
about is when reseeding is necessary.

For example, when a virtual machine is forked, restored, or duplicated,
it's imparative that the RNG doesn't generate the same outputs. For this
reason, there's a small protocol between hypervisors and the kernel that
indicates this has happened, alongside some ID, which the RNG uses to
immediately reseed, so as not to return the same numbers. Were userspace
to expand a getrandom() seed from time T1 for the next hour, and at some
point T2 < hour, the virtual machine forked, userspace would continue to
provide the same numbers to two (or more) different virtual machines,
resulting in potential cryptographic catastrophe. Something similar
happens on resuming from hibernation (or even suspend), with various
compromise scenarios there in mind.

There's a more general reason why userspace rolling its own RNG from a
getrandom() seed is fraught. There's a lot of attention paid to this
particular Linuxism we have of the RNG being initialized and thus
non-blocking or uninitialized and thus blocking until it is initialized.
These are our Two Big States that many hold to be the holy
differentiating factor between safe and not safe, between
cryptographically secure and garbage. The fact is, however, that the
distinction between these two states is a hand-wavy wishy-washy inexact
approximation. Outside of a few exceptional cases (e.g. a HW RNG is
available), we actually don't really ever know with any rigor at all
when the RNG is safe and ready (nor when it's compromised). We do the
best we can to "estimate" it, but entropy estimation is fundamentally
impossible in the general case. So really, we're just doing guess work,
and hoping it's good and conservative enough. Let's then assume that
there's always some potential error involved in this differentiator.

In fact, under the surface, the RNG is engineered around a different
principal, and that is trying to *use* new entropic inputs regularly and
at the right specific moments in time. For example, close to boot time,
the RNG reseeds itself more often than later. At certain events, like VM
fork, the RNG reseeds itself immediately. The various heuristics for
when the RNG will use new entropy and how often is really a core aspect
of what the RNG has some potential to do decently enough (and something
that will probably continue to improve in the future from random.c's
present set of algorithms). So in your mind, put away the metal
attachment to the Two Big States, which represent an approximation with
a potential margin of error. Instead keep in mind that the RNG's primary
operating heuristic is how often and exactly when it's going to reseed.

So, if userspace takes a seed from getrandom() at point T1, and uses it
for the next hour (or N megabytes or some other meaningless metric),
during that time, potential errors in the Two Big States approximation
are amplified. During that time potential reseeds are being lost,
forgotten, not reflected in the output stream. That's not good.

The simplest statement you could is that userspace RNGs that expand a
getrandom() seed at some point T1 are nearly always *worse*, in some
way, than just calling getrandom() every time a random number is
desired.

For those reasons, after some discussion on libc-alpha, glibc's
arc4random() now just calls getrandom() on each invocation. That's
trivially safe, and gives us latitude to then make the safe thing faster
without becoming unsafe at our leasure. Card shuffling isn't
particularly fast, however.

How do we rectify this? By putting a safe implementation of getrandom()
in the vDSO, which has access to whatever information a
particular iteration of random.c is using to make its decisions. I use
that careful language of "particular iteration of random.c", because the
set of things that a vDSO getrandom() implementation might need for making
decisions as good as the kernel's will likely change over time. This
isn't just a matter of exporting certain *data* to userspace. We're not
going to commit to a "data API" where the various heuristics used are
exposed, locking in how the kernel works for decades to come, and then
leave it to various userspaces to roll something on top and shoot
themselves in the foot and have all sorts of complexity disasters.
Rather, vDSO getrandom() is supposed to be the *same exact algorithm*
that runs in the kernel, except it's been hoisted into userspace as
much as possible. And so vDSO getrandom() and kernel getrandom() will
always mirror each other hermetically.

API-wise, vDSO getrandom has this signature:

  ssize_t getrandom(void **state, void *buffer, size_t len, unsigned long flags);

The return value and the latter 3 arguments are the same as ordinary
getrandom(). The first argument is a double pointer to some state that
vDSO allocates and manages. Call it first with *&my_state==NULL, and
subsequently with the same &my_state, and only that first call will
allocate. We very intentionally do *not* leave state memory management
up to the caller. There are too many weird things that can go wrong, and
it's important that vDSO does not provide too generic of a mechanism.
It's not going to store its state in just any old memory address. It'll
do it only in ones it allocates.

Right now this means it's a mlock'd page with WIPEONFORK set. In the
future maybe there will be other interesting page flags or
anti-heartbleed measures, or other platform-specific kernel-specific
things that can be set. Again, it's important that the vDSO has a say in
how this works rather than agreeing to operate on any old address;
memory isn't neutral.

Because WIPEONFORK implies a whole page, vDSO getrandom() itself uses
vDSO getcpu() in order to shard into various buckets, so that this
remains fast from multiple threads.

The interesting meat of the implementation is in lib/vdso/getrandom.c,
as generic C code, and it aims to mainly follow random.c's buffered fast
key erasure logic. Before the RNG is initialized, it falls back to the
syscall. Right now it uses a simple generation counter to make its
decisions on reseeding; this covers many cases, but not all, so this RFC
still has a little bit of improvement work to do. But it should give you
the general idea.

The actual place that has the most work to do is in all of the other
files. Most of the vDSO shared page infrastructure is centered around
gettimeofday, and so the main structs are all in arrays for different
timestamp types, and attached to time namespaces, and so forth. I've
done the best I could to add onto this in an unintrusive way, but you'll
notice almost immediately from glancing at the code that it still needs
some untangling work. This also only works on x86 at the moment. I could
certainly use a hand with this part.

So far in my test results, performance is pretty stellar, and it seems
to be working. But this is very, very young, immature code, suitable for
an RFC and no more, so expect dragons.

Cc: linux-crypto@vger.kernel.org
Cc: x86@kernel.org
Cc: Nadia Heninger <nadiah@cs.ucsd.edu>
Cc: Thomas Ristenpart <ristenpart@cornell.edu>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/x86/entry/vdso/Makefile          |   2 +-
 arch/x86/entry/vdso/vdso.lds.S        |   2 +
 arch/x86/entry/vdso/vgetrandom.c      |  16 +++
 arch/x86/include/asm/vdso/getrandom.h |  74 +++++++++++
 arch/x86/include/asm/vvar.h           |  16 +++
 drivers/char/random.c                 |   4 +
 include/vdso/datapage.h               |   6 +
 lib/vdso/getrandom.c                  | 176 ++++++++++++++++++++++++++
 8 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/entry/vdso/vgetrandom.c
 create mode 100644 arch/x86/include/asm/vdso/getrandom.h
 create mode 100644 lib/vdso/getrandom.c

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 76cd790ed0bd..a60d4771d500 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -24,7 +24,7 @@ VDSO32-$(CONFIG_X86_32)		:= y
 VDSO32-$(CONFIG_IA32_EMULATION)	:= y
 
 # files to link into the vdso
-vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
+vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vgetrandom.o
 vobjs32-y := vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o
 vobjs32-y += vdso32/vclock_gettime.o
 vobjs-$(CONFIG_X86_SGX)	+= vsgx.o
diff --git a/arch/x86/entry/vdso/vdso.lds.S b/arch/x86/entry/vdso/vdso.lds.S
index 4bf48462fca7..1919cc39277e 100644
--- a/arch/x86/entry/vdso/vdso.lds.S
+++ b/arch/x86/entry/vdso/vdso.lds.S
@@ -28,6 +28,8 @@ VERSION {
 		clock_getres;
 		__vdso_clock_getres;
 		__vdso_sgx_enter_enclave;
+		getrandom;
+		__vdso_getrandom;
 	local: *;
 	};
 }
diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c
new file mode 100644
index 000000000000..40389c399c6a
--- /dev/null
+++ b/arch/x86/entry/vdso/vgetrandom.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+
+#include "../../../../lib/vdso/getrandom.c"
+
+ssize_t __vdso_getrandom(void **state, void *buffer, size_t len, unsigned long flags)
+{
+	return __cvdso_getrandom(state, buffer, len, flags);
+}
+
+ssize_t getrandom(void **, void *, size_t, unsigned long)
+	__attribute__((weak, alias("__vdso_getrandom")));
diff --git a/arch/x86/include/asm/vdso/getrandom.h b/arch/x86/include/asm/vdso/getrandom.h
new file mode 100644
index 000000000000..83cb483aab74
--- /dev/null
+++ b/arch/x86/include/asm/vdso/getrandom.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+#ifndef __ASM_VDSO_GETRANDOM_H
+#define __ASM_VDSO_GETRANDOM_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/unistd.h>
+#include <asm/vvar.h>
+
+static __always_inline ssize_t
+getrandom_syscall(void *buffer, size_t len, unsigned long flags)
+{
+	long ret;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_getrandom), "D" (buffer), "S" (len), "d" (flags) :
+	     "rcx", "r11", "memory");
+
+	return ret;
+}
+
+static __always_inline void *
+mmap_syscall(void *addr, size_t len, int prot, int flags, int fd, off_t offset)
+{
+	long ret;
+	register long r10 asm("r10") = flags;
+	register long r8 asm("r8") = fd;
+	register long r9 asm("r9") = offset;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_mmap), "D" (addr), "S" (len), "d" (prot),
+	     "r" (r10), "r" (r8), "r" (r9) :
+	     "rcx", "r11");
+
+	return (void *)ret;
+}
+
+static __always_inline int
+munmap_syscall(void *addr, size_t len)
+{
+	long ret;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_munmap), "D" (addr), "S" (len) :
+	     "rcx", "r11");
+
+	return ret;
+}
+
+static __always_inline int
+madvise_syscall(void *addr, size_t len, int advice)
+{
+	long ret;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_madvise), "D" (addr), "S" (len), "d" (advice) :
+	     "rcx", "r11");
+
+	return ret;
+}
+
+#define __vdso_rng_data (VVAR(_vdso_rng_data))
+
+static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
+{
+	return &__vdso_rng_data;
+}
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_GETRANDOM_H */
diff --git a/arch/x86/include/asm/vvar.h b/arch/x86/include/asm/vvar.h
index 183e98e49ab9..9d9af37f7cab 100644
--- a/arch/x86/include/asm/vvar.h
+++ b/arch/x86/include/asm/vvar.h
@@ -26,6 +26,8 @@
  */
 #define DECLARE_VVAR(offset, type, name) \
 	EMIT_VVAR(name, offset)
+#define DECLARE_VVAR_SINGLE(offset, type, name) \
+	EMIT_VVAR(name, offset)
 
 #else
 
@@ -37,6 +39,10 @@ extern char __vvar_page;
 	extern type timens_ ## name[CS_BASES]				\
 	__attribute__((visibility("hidden")));				\
 
+#define DECLARE_VVAR_SINGLE(offset, type, name)				\
+	extern type vvar_ ## name					\
+	__attribute__((visibility("hidden")));				\
+
 #define VVAR(name) (vvar_ ## name)
 #define TIMENS(name) (timens_ ## name)
 
@@ -44,12 +50,22 @@ extern char __vvar_page;
 	type name[CS_BASES]						\
 	__attribute__((section(".vvar_" #name), aligned(16))) __visible
 
+#define DEFINE_VVAR_SINGLE(type, name)					\
+	type name							\
+	__attribute__((section(".vvar_" #name), aligned(16))) __visible
+
 #endif
 
 /* DECLARE_VVAR(offset, type, name) */
 
 DECLARE_VVAR(128, struct vdso_data, _vdso_data)
 
+#if !defined(_SINGLE_DATA)
+#define _SINGLE_DATA
+DECLARE_VVAR_SINGLE(640, struct vdso_rng_data, _vdso_rng_data)
+#endif
+
 #undef DECLARE_VVAR
+#undef DECLARE_VVAR_SINGLE
 
 #endif
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 7bf11fa66265..a18ff41713d7 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -59,6 +59,7 @@
 #include <asm/irq.h>
 #include <asm/irq_regs.h>
 #include <asm/io.h>
+#include <vdso/datapage.h>
 
 /*********************************************************************
  *
@@ -84,6 +85,7 @@ static DEFINE_STATIC_KEY_FALSE(crng_is_ready);
 /* Various types of waiters for crng_init->CRNG_READY transition. */
 static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);
 static struct fasync_struct *fasync;
+DEFINE_VVAR_SINGLE(struct vdso_rng_data, _vdso_rng_data);
 
 /* Control how we warn userspace. */
 static struct ratelimit_state urandom_warning =
@@ -221,6 +223,7 @@ static void crng_reseed(void)
 		++next_gen;
 	WRITE_ONCE(base_crng.generation, next_gen);
 	WRITE_ONCE(base_crng.birth, jiffies);
+	WRITE_ONCE(_vdso_rng_data.generation, next_gen + 1);
 	if (!static_branch_likely(&crng_is_ready))
 		crng_init = CRNG_READY;
 	spin_unlock_irqrestore(&base_crng.lock, flags);
@@ -660,6 +663,7 @@ static void __cold _credit_init_bits(size_t bits)
 		crng_reseed(); /* Sets crng_init to CRNG_READY under base_crng.lock. */
 		if (static_key_initialized)
 			execute_in_process_context(crng_set_ready, &set_ready);
+		_vdso_rng_data.is_ready = true;
 		wake_up_interruptible(&crng_init_wait);
 		kill_fasync(&fasync, SIGIO, POLL_IN);
 		pr_notice("crng init done\n");
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 73eb622e7663..cbacfd923a5c 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -109,6 +109,11 @@ struct vdso_data {
 	struct arch_vdso_data	arch_data;
 };
 
+struct vdso_rng_data {
+	unsigned long generation;
+	bool is_ready;
+};
+
 /*
  * We use the hidden visibility to prevent the compiler from generating a GOT
  * relocation. Not only is going through a GOT useless (the entry couldn't and
@@ -120,6 +125,7 @@ struct vdso_data {
  */
 extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
 extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
+extern struct vdso_rng_data _vdso_rng_data __attribute__((visibility("hidden")));
 
 /*
  * The generic vDSO implementation requires that gettimeofday.h
diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
new file mode 100644
index 000000000000..3ffc900f31ff
--- /dev/null
+++ b/lib/vdso/getrandom.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/atomic.h>
+#include <linux/fs.h>
+#include <vdso/datapage.h>
+#include <asm/vdso/getrandom.h>
+#include <asm/vdso/vsyscall.h>
+#include <asm/page.h>
+#include <uapi/linux/mman.h>
+#include "../crypto/chacha.c"
+
+#define NEG_NULL ((void *)~0UL)
+
+struct getrandom_state {
+	unsigned long generation;
+	union {
+		struct {
+			u8 key[CHACHA_KEY_SIZE];
+			u8 batch[CHACHA_BLOCK_SIZE * 3 / 2];
+		};
+		u8 key_batch[CHACHA_BLOCK_SIZE * 2];
+	};
+	u8 pos;
+	bool in_use;
+};
+
+enum { NUM_BUCKETS = PAGE_SIZE / sizeof(struct getrandom_state) };
+
+static bool allocate_new_state(void **state)
+{
+	void *new_state;
+
+	if (cmpxchg(state, NULL, NEG_NULL) != NULL)
+		return false;
+
+	new_state = mmap_syscall(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
+				 MAP_PRIVATE | MAP_ANONYMOUS | MAP_LOCKED, -1, 0);
+	if (new_state == NEG_NULL)
+		goto err_unlock;
+
+	if (madvise_syscall(new_state, PAGE_SIZE, MADV_WIPEONFORK))
+		goto err_unmap;
+
+	WRITE_ONCE(*state, new_state);
+	return true;
+
+err_unmap:
+	munmap_syscall(new_state, PAGE_SIZE);
+err_unlock:
+	WRITE_ONCE(*state, NULL);
+	return false;
+}
+
+extern long getcpu(unsigned int *cpu, unsigned int *node, void *unused);
+
+static struct getrandom_state *find_free_bucket(struct getrandom_state *buckets)
+{
+	unsigned int start = 0, i;
+
+	if (getcpu(&start, NULL, NULL) == 0)
+		start %= NUM_BUCKETS;
+
+	for (i = start;;) {
+		struct getrandom_state *state = &buckets[i];
+
+		if (cmpxchg(&state->in_use, false, true) == false)
+			return state;
+
+		i = i == NUM_BUCKETS - 1 ? 0 : i + 1;
+		if (i == start)
+			break;
+	}
+
+	return NULL;
+}
+
+static void memcpy_and_zero(void *dst, void *src, size_t len)
+{
+#define CASCADE(type) \
+	while (len >= sizeof(type)) { \
+		*(type *)dst = *(type *)src; \
+		*(type *)src = 0; \
+		dst += sizeof(type); \
+		src += sizeof(type); \
+		len -= sizeof(type); \
+	}
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+#if BITS_PER_LONG == 64
+	CASCADE(u64);
+#endif
+	CASCADE(u32);
+	CASCADE(u16);
+#endif
+	CASCADE(u8);
+#undef CASCADE
+}
+
+static ssize_t __always_inline
+__cvdso_getrandom(void **state, void *buffer, size_t len, unsigned long flags)
+{
+	const struct vdso_rng_data *v = __arch_get_vdso_rng_data();
+	struct getrandom_state *buckets = *state, *s;
+	u32 chacha_state[CHACHA_STATE_WORDS];
+	unsigned long generation;
+	ssize_t ret = min_t(size_t, MAX_RW_COUNT, len);
+	size_t batch_len;
+
+	if (unlikely(!v->is_ready))
+		return getrandom_syscall(buffer, len, flags);
+
+	if (unlikely(!len))
+		return 0;
+
+	if (unlikely(!buckets)) {
+		if (!allocate_new_state(state))
+			return getrandom_syscall(buffer, len, flags);
+		buckets = *state;
+	} else if (unlikely(buckets == NEG_NULL)) {
+		return getrandom_syscall(buffer, len, flags);
+	}
+
+	s = find_free_bucket(buckets);
+	if (unlikely(!s))
+		return getrandom_syscall(buffer, len, flags);
+
+retry_generation:
+	generation = READ_ONCE(v->generation);
+	if (unlikely(s->generation != generation)) {
+		if (getrandom_syscall(s->key, sizeof(s->key), 0) != sizeof(s->key)) {
+			WRITE_ONCE(s->in_use, false);
+			return getrandom_syscall(buffer, len, flags);
+		}
+		s->pos = sizeof(s->batch);
+		s->generation = generation;
+		if (generation != READ_ONCE(v->generation))
+			goto retry_generation;
+	}
+
+	len = ret;
+more_batch:
+	batch_len = min_t(size_t, sizeof(s->batch) - s->pos, len);
+	if (batch_len) {
+		memcpy_and_zero(buffer, s->batch, batch_len);
+		s->pos += batch_len;
+		buffer += batch_len;
+		len -= batch_len;
+		if (!len) {
+			WRITE_ONCE(s->in_use, false);
+			return ret;
+		}
+	}
+
+	chacha_init_consts(chacha_state);
+	memcpy(&chacha_state[4], s->key, CHACHA_KEY_SIZE);
+	memset(&chacha_state[12], 0, sizeof(u32) * 4);
+
+	while (len >= CHACHA_BLOCK_SIZE) {
+		chacha20_block(chacha_state, buffer);
+		if (unlikely(chacha_state[12] == 0))
+			++chacha_state[13];
+		buffer += CHACHA_BLOCK_SIZE;
+		len -= CHACHA_BLOCK_SIZE;
+	}
+
+	chacha20_block(chacha_state, s->key_batch);
+	if (unlikely(chacha_state[12] == 0))
+		++chacha_state[13];
+	chacha20_block(chacha_state, s->key_batch + CHACHA_BLOCK_SIZE);
+	s->pos = 0;
+	memzero_explicit(chacha_state, sizeof(chacha_state));
+	goto more_batch;
+}
-- 
2.35.1

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Florian Weimer]

* Jason A. Donenfeld:

> diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
> new file mode 100644
> index 000000000000..3ffc900f31ff
> --- /dev/null
> +++ b/lib/vdso/getrandom.c

> +static struct getrandom_state *find_free_bucket(struct getrandom_state *buckets)
> +{
> +	unsigned int start = 0, i;
> +
> +	if (getcpu(&start, NULL, NULL) == 0)
> +		start %= NUM_BUCKETS;

getcpu is not available everywhere.  Userspace/libc should probably
provide a CPU number hint as an additional argument during the vDSO
call.  We can load that easily enough from rseq.  That's going to be
faster on x86, too (the LSL instruction is quite slow).  The only
advantage of using getcpu like this is that it's compatible with a libc
that isn't rseq-enabled.

> +	for (i = start;;) {
> +		struct getrandom_state *state = &buckets[i];
> +
> +		if (cmpxchg(&state->in_use, false, true) == false)
> +			return state;
> +
> +		i = i == NUM_BUCKETS - 1 ? 0 : i + 1;
> +		if (i == start)
> +			break;
> +	}

Surely this scales very badly once the number of buckets is smaller than
the system processor count?

> +static ssize_t __always_inline
> +__cvdso_getrandom(void **state, void *buffer, size_t len, unsigned long flags)

> +more_batch:
> +	batch_len = min_t(size_t, sizeof(s->batch) - s->pos, len);
> +	if (batch_len) {
> +		memcpy_and_zero(buffer, s->batch, batch_len);
> +		s->pos += batch_len;
> +		buffer += batch_len;
> +		len -= batch_len;
> +		if (!len) {
> +			WRITE_ONCE(s->in_use, false);
> +			return ret;
> +		}
> +	}

I expect the main performance benefit comes from not doing any ChaCha20
work except on batch boundaries, not the vDSO acceleration as such.
Maybe that's something that should be tried first within the system call
implementation.  (Even for getrandom with a small buffer, it's not the
system call overhead that dominates, but something related to ChaCha20.)

Thanks,
Florian

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hey Florian,

Thanks for the feedback.

On Fri, Jul 29, 2022 at 10:19:05PM +0200, Florian Weimer wrote:
> > +	if (getcpu(&start, NULL, NULL) == 0)
> > +		start %= NUM_BUCKETS;
> 
> getcpu is not available everywhere.  Userspace/libc should probably
> provide a CPU number hint as an additional argument during the vDSO
> call.  We can load that easily enough from rseq.  That's going to be
> faster on x86, too (the LSL instruction is quite slow).  The only
> advantage of using getcpu like this is that it's compatible with a libc
> that isn't rseq-enabled.

Actually, the only requirement is that it's somewhat stable and somehow
separates threads most of the time. So a per-thread ID or even a
per-thread address would work fine too. Adhemerval suggested on IRC this
afternoon that there's a thread pointer register value that would be
usable for this purpose. I think what I'll do for v2 is abstract this
out to a __arch_get_bucket_hint() function, or similar, which the
different archs can fill in.

> > +	for (i = start;;) {
> > +		struct getrandom_state *state = &buckets[i];
> > +
> > +		if (cmpxchg(&state->in_use, false, true) == false)
> > +			return state;
> > +
> > +		i = i == NUM_BUCKETS - 1 ? 0 : i + 1;
> > +		if (i == start)
> > +			break;
> > +	}
> 
> Surely this scales very badly once the number of buckets is smaller than
> the system processor count?

Right, and there are a few ways that observation can go:

1) It doesn't matter, because who has > 28 threads all churning at once
   here? Is that something real?

2) The state variable is controllable by userspace, so in theory
   different ones could be passed. I don't like this idea though - hard
   to manage and not enough information to do it well.

3) Since we know when this kind of contention is hit, it should be
   possible to just expand the map size. Seems a bit complicated.

4) Simply allocate a number of pages relative to the number of CPUs, so
   that this isn't actually a problem. This seems like the simplest
   approach and seems fine.

Jason 

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Linus Torvalds]

On Fri, Jul 29, 2022 at 3:32 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Two statements:
>
>   1) Userspace wants faster cryptographically secure numbers of
>      arbitrary size, big or small.
>
>   2) Userspace is currently unable to safely roll its own RNG with the
>      same security profile as getrandom().

So I'm really not convinced that this kind of thing is something the
kernel should work that hard to help.

And that state allocation in particular looks very random in all the
wrong ways, with various "if I run out of resources I'll just do a
system call" things.

Which just makes me go "just do the system call".

People who are _that_ sensitive to performance can't use this anyway,
unless they know the exact rules of "ok, I only need X state buffers"
(ok, the buckets are probably the more real "sometimes it might fail"
thing). So they basically need to know about the implementation
details - even if it's only about that kind of "only a limited number
of states" thing.

Not to mention that I don't think your patch can work anyway, with
things like "cmpxchg()" not being something you can do in the vdso
because it might have the kernel instrumentation in it.

So this all smells really fragile to me, and honestly, unlike the vdso
things we _do_ have, I don't think I've ever seen getrandom() be a
huge deal performance-wise.

It's just too specialized, and the people who care about performance
can - and do - do special things anyway.

Your patch fundamentally seems to be about "make it easy to not have
to care, and still get high performance", but that's _such_ a small
use-case (the intersection between "don't care" and "really really
need high performance" would seem to be basically NIL).

              Linus

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hey Linus,

Thanks a bunch for chiming in. Indeed this whole thing is kind of crazy,
so your input is particularly useful here.

On Sat, Jul 30, 2022 at 08:48:42AM -0700, Linus Torvalds wrote:
> It's just too specialized, and the people who care about performance
> can - and do - do special things anyway.

I followed most of your email, but I just wanted to point out that the
"can" part of this isn't quite right, though the "do" part is.
Specifically, I don't think there's currently a good way for userspace
to do this kind of thing and get the same kind of security guarantees
that the syscall has. They "do" it anyway, though (openssl, libgcrypt,
glibc's arc4random() implementation before I tamed it last week, etc),
and this is somewhat concerning.

So my larger intent is, assuming that people will continue to attempt
such things, to just nip the issue in the bud by providing an actually
safe way for it to be done.

To be clear, I really would rather not do this. I'm not really looking
for more stuff to do, and I don't tend to write (public) code "just
'cuz". My worry is that by /not/ doing it, footguns will proliferate.
The glibc thing was what finally motivated me to want to at least sketch
out a potential action to make this kind of (apparently common) urge of
writing a userspace RNG safer.

(Actually coding it up didn't really take much time, which perhaps
shows: that `if (!len)` check needs to be hoisted out of the inner
block!)

> So I'm really not convinced that this kind of thing is something the
> kernel should work that hard to help.
> 
> Your patch fundamentally seems to be about "make it easy to not have
> to care, and still get high performance", but that's _such_ a small
> use-case (the intersection between "don't care" and "really really
> need high performance" would seem to be basically NIL).

So this is "statement (1)" stuff. Namely, userspace apparently wants
faster random numbers. Is this desire justified? Has anybody aside from
Phoronix even benchmarked getrandom() since I did the neat lockless
stuff to it? Is this just for some artificial card shuffling unit tests,
or is generating TLS CBC nonces at scale using getrandom() a real
bottleneck for a real use case?

I'm honestly not quite sure. But I do know that people are building
these userspace RNGs anyway, and will keep building them, and that kind
of worries me.

So either this is a useful thing to have, and people are building it
anyway, so maybe the kernel should get involved. Or it's not a useful
thing to have, BUT people are building it anyway, so maybe the kernel
should [not?] get involved? The latter case is a bit decisionally
hairier.

Anyway, onto the technical feedback:

> And that state allocation in particular looks very random in all the
> wrong ways, with various "if I run out of resources I'll just do a
> system call" things.
> 
> Not to mention that I don't think your patch can work anyway, with
> things like "cmpxchg()" not being something you can do in the vdso
> because it might have the kernel instrumentation in it.

Yea this sharding thing is somewhat faulty. In its current inception, it
also falls over during fork, since the cmpxchg pseudo trylock is
dropped, among other problems Andy and I discussed on IRC. Andy also
suggested not doing the allocation inside of the same function. Florian
brought up the difficulty of even determining the CPU number on arm64.
And also that's a good point about instrumentation on cmpxchg.

So, anyway, if I do muster a v2 of this (perhaps just to see the idea
through), the API might split in two to something like:

  void *getrandom_allocate_states([inout] size_t *number_of_states, [out] size_t *length_per_state);
  ssize_t getrandom(void *state, void *buffer, size_t len, unsigned long flags);

User code will call getrandom_allocate_state(), which will allocate
enough pages to hold *number_of_states, and return the size of each one
in length_per_state and the number actually allocated back in
number_of_states. The result can then be sliced up by that size, and
passed to getrandom(). So glibc or whatever would presumably allocate
one per thread, and handle any reentrancy/locking around it.

Or some other variation on that. I'm sure you hate those function
signatures. Everybody loves to bikeshed APIs, right? There's plenty to
be tweaked. But that's anyhow about where my thinking is for a potential
v2.

Jason

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

On Sun, Jul 31, 2022 at 01:45:43AM +0200, Jason A. Donenfeld wrote:
> So, anyway, if I do muster a v2 of this (perhaps just to see the idea
> through), the API might split in two to something like:
> 
>   void *getrandom_allocate_states([inout] size_t *number_of_states, [out] size_t *length_per_state);
>   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned long flags);
> 
> User code will call getrandom_allocate_state(), which will allocate
> enough pages to hold *number_of_states, and return the size of each one
> in length_per_state and the number actually allocated back in
> number_of_states. The result can then be sliced up by that size, and
> passed to getrandom(). So glibc or whatever would presumably allocate
> one per thread, and handle any reentrancy/locking around it.
> 
> Or some other variation on that. I'm sure you hate those function
> signatures. Everybody loves to bikeshed APIs, right? There's plenty to
> be tweaked. But that's anyhow about where my thinking is for a potential
> v2.

Doing this also doubled performance, perhaps unsurprisingly, as that
getcpu() operation wasn't free.

For uint32_t generation:

   vdso: 25000000 times in 0.289876265 seconds
syscall: 25000000 times in 4.296636025 seconds

[PATCH RFC v2] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Two statements:

  1) Userspace wants faster cryptographically secure random numbers of
     arbitrary size, big or small.

  2) Userspace is currently unable to safely roll its own RNG with the
     same security profile as getrandom().

Statement (1) has been debated for years, with arguments ranging from
"we need faster cryptographically secure card shuffling!" to "the only
things that actually need good randomness are keys, which are few and
far between" to "actually, TLS CBC nonces are frequent" and so on. I
don't intend to wade into that debate substantially, except to note that
recently glibc added arc4random(), whose goal is to return a
cryptographically secure uint32_t. So here we are.

Statement (2) is more interesting. The kernel is the nexus of all
entropic inputs that influence the RNG. It is in the best position, and
probably the only position, to decide anything at all about the current
state of the RNG and of its entropy. One of the things it uniquely knows
about is when reseeding is necessary.

For example, when a virtual machine is forked, restored, or duplicated,
it's imparative that the RNG doesn't generate the same outputs. For this
reason, there's a small protocol between hypervisors and the kernel that
indicates this has happened, alongside some ID, which the RNG uses to
immediately reseed, so as not to return the same numbers. Were userspace
to expand a getrandom() seed from time T1 for the next hour, and at some
point T2 < hour, the virtual machine forked, userspace would continue to
provide the same numbers to two (or more) different virtual machines,
resulting in potential cryptographic catastrophe. Something similar
happens on resuming from hibernation (or even suspend), with various
compromise scenarios there in mind.

There's a more general reason why userspace rolling its own RNG from a
getrandom() seed is fraught. There's a lot of attention paid to this
particular Linuxism we have of the RNG being initialized and thus
non-blocking or uninitialized and thus blocking until it is initialized.
These are our Two Big States that many hold to be the holy
differentiating factor between safe and not safe, between
cryptographically secure and garbage. The fact is, however, that the
distinction between these two states is a hand-wavy wishy-washy inexact
approximation. Outside of a few exceptional cases (e.g. a HW RNG is
available), we actually don't really ever know with any rigor at all
when the RNG is safe and ready (nor when it's compromised). We do the
best we can to "estimate" it, but entropy estimation is fundamentally
impossible in the general case. So really, we're just doing guess work,
and hoping it's good and conservative enough. Let's then assume that
there's always some potential error involved in this differentiator.

In fact, under the surface, the RNG is engineered around a different
principal, and that is trying to *use* new entropic inputs regularly and
at the right specific moments in time. For example, close to boot time,
the RNG reseeds itself more often than later. At certain events, like VM
fork, the RNG reseeds itself immediately. The various heuristics for
when the RNG will use new entropy and how often is really a core aspect
of what the RNG has some potential to do decently enough (and something
that will probably continue to improve in the future from random.c's
present set of algorithms). So in your mind, put away the metal
attachment to the Two Big States, which represent an approximation with
a potential margin of error. Instead keep in mind that the RNG's primary
operating heuristic is how often and exactly when it's going to reseed.

So, if userspace takes a seed from getrandom() at point T1, and uses it
for the next hour (or N megabytes or some other meaningless metric),
during that time, potential errors in the Two Big States approximation
are amplified. During that time potential reseeds are being lost,
forgotten, not reflected in the output stream. That's not good.

The simplest statement you could make is that userspace RNGs that expand
a getrandom() seed at some point T1 are nearly always *worse*, in some
way, than just calling getrandom() every time a random number is
desired.

For those reasons, after some discussion on libc-alpha, glibc's
arc4random() now just calls getrandom() on each invocation. That's
trivially safe, and gives us latitude to then make the safe thing faster
without becoming unsafe at our leasure. Card shuffling isn't
particularly fast, however.

How do we rectify this? By putting a safe implementation of getrandom()
in the vDSO, which has access to whatever information a
particular iteration of random.c is using to make its decisions. I use
that careful language of "particular iteration of random.c", because the
set of things that a vDSO getrandom() implementation might need for making
decisions as good as the kernel's will likely change over time. This
isn't just a matter of exporting certain *data* to userspace. We're not
going to commit to a "data API" where the various heuristics used are
exposed, locking in how the kernel works for decades to come, and then
leave it to various userspaces to roll something on top and shoot
themselves in the foot and have all sorts of complexity disasters.
Rather, vDSO getrandom() is supposed to be the *same exact algorithm*
that runs in the kernel, except it's been hoisted into userspace as
much as possible. And so vDSO getrandom() and kernel getrandom() will
always mirror each other hermetically.

API-wise, vDSO getrandom has a pair of functions:

  ssize_t getrandom(void *state, void *buffer, size_t len, unsigned int flags);
  void *getrandom_alloc([inout] size_t *num, [out] size_t *size_per_each);

In the first function, the return value and the latter 3 arguments are
the same as ordinary getrandom(), while the first argument is a pointer
to some state allocated with getrandom_alloc(). getrandom_alloc() takes
the desired number of states, and returns an array of states, the number
actually allocated, and the size in bytes of each one, enabling a libc
to use one per thread. We very intentionally do *not* leave state
allocation up to the caller. There are too many weird things that can go
wrong, and it's important that vDSO does not provide too generic of a
mechanism. It's not going to store its state in just any old memory
address. It'll do it only in ones it allocates.

Right now this means it's a mlock'd page with WIPEONFORK set. In the
future maybe there will be other interesting page flags or
anti-heartbleed measures, or other platform-specific kernel-specific
things that can be set. Again, it's important that the vDSO has a say in
how this works rather than agreeing to operate on any old address;
memory isn't neutral.

The interesting meat of the implementation is in lib/vdso/getrandom.c,
as generic C code, and it aims to mainly follow random.c's buffered fast
key erasure logic. Before the RNG is initialized, it falls back to the
syscall. Right now it uses a simple generation counter to make its
decisions on reseeding; this covers many cases, but not all, so this RFC
still has a little bit of improvement work to do. But it should give you
the general idea.

The actual place that has the most work to do is in all of the other
files. Most of the vDSO shared page infrastructure is centered around
gettimeofday, and so the main structs are all in arrays for different
timestamp types, and attached to time namespaces, and so forth. I've
done the best I could to add onto this in an unintrusive way, but you'll
notice almost immediately from glancing at the code that it still needs
some untangling work. This also only works on x86 at the moment. I could
certainly use a hand with this part.

So far in my test results, performance is pretty stellar (around 15x for
uint32_t generation), and it seems to be working. But this is very, very
young, immature code, suitable for an RFC and no more, so expect
dragons.

Cc: linux-crypto@vger.kernel.org
Cc: x86@kernel.org
Cc: Nadia Heninger <nadiah@cs.ucsd.edu>
Cc: Thomas Ristenpart <ristenpart@cornell.edu>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
I'm not in a hurry to make this patch happen, but I know some people
wanted to tinker around with this, and given that this v2 drops v1's
sharding, I figure I should post this sooner so there's something to
play with.

Changes v1->v2:
- Hoist !len check out of batch_len check.
- Use smp_store_release instead of WRITE_ONCE.
- Account for fork() during execution.
- Simplify API so that libc passes a state per thread, allocated with a
  separate function.
- Attempt to approximate crng_has_old_seed(). This needs work still, but is a
  step in the right direction.
- Document generation counter quirks in comment.
- getrandom() takes `unsigned int flags`, not `unsigned long flags`.

 MAINTAINERS                           |   1 +
 arch/x86/entry/vdso/Makefile          |   2 +-
 arch/x86/entry/vdso/vdso.lds.S        |   4 +
 arch/x86/entry/vdso/vgetrandom.c      |  24 ++++
 arch/x86/include/asm/vdso/getrandom.h |  74 +++++++++++++
 arch/x86/include/asm/vvar.h           |  16 +++
 drivers/char/random.c                 |   4 +
 include/vdso/datapage.h               |   6 +
 lib/vdso/getrandom.c                  | 151 ++++++++++++++++++++++++++
 9 files changed, 281 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/entry/vdso/vgetrandom.c
 create mode 100644 arch/x86/include/asm/vdso/getrandom.h
 create mode 100644 lib/vdso/getrandom.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 651616ed8ae2..b0d2dce4855a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16856,6 +16856,7 @@ T:	git https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git
 S:	Maintained
 F:	drivers/char/random.c
 F:	drivers/virt/vmgenid.c
+F:	lib/vdso/getrandom.c
 
 RAPIDIO SUBSYSTEM
 M:	Matt Porter <mporter@kernel.crashing.org>
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 76cd790ed0bd..a60d4771d500 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -24,7 +24,7 @@ VDSO32-$(CONFIG_X86_32)		:= y
 VDSO32-$(CONFIG_IA32_EMULATION)	:= y
 
 # files to link into the vdso
-vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
+vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vgetrandom.o
 vobjs32-y := vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o
 vobjs32-y += vdso32/vclock_gettime.o
 vobjs-$(CONFIG_X86_SGX)	+= vsgx.o
diff --git a/arch/x86/entry/vdso/vdso.lds.S b/arch/x86/entry/vdso/vdso.lds.S
index 4bf48462fca7..faf38a5e3d82 100644
--- a/arch/x86/entry/vdso/vdso.lds.S
+++ b/arch/x86/entry/vdso/vdso.lds.S
@@ -28,6 +28,10 @@ VERSION {
 		clock_getres;
 		__vdso_clock_getres;
 		__vdso_sgx_enter_enclave;
+		getrandom;
+		__vdso_getrandom;
+		getrandom_alloc;
+		__vdso_getrandom_alloc;
 	local: *;
 	};
 }
diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c
new file mode 100644
index 000000000000..2766b0c8e6fd
--- /dev/null
+++ b/arch/x86/entry/vdso/vgetrandom.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+
+#include "../../../../lib/vdso/getrandom.c"
+
+ssize_t __vdso_getrandom(void *state, void *buffer, size_t len, unsigned int flags)
+{
+	return __cvdso_getrandom(state, buffer, len, flags);
+}
+
+ssize_t getrandom(void *, void *, size_t, unsigned int)
+	__attribute__((weak, alias("__vdso_getrandom")));
+
+void *__vdso_getrandom_alloc(size_t *num, size_t *size_per_each)
+{
+	return __cvdso_getrandom_alloc(num, size_per_each);
+}
+
+void *getrandom_alloc(size_t *, size_t *)
+	__attribute__((weak, alias("__vdso_getrandom_alloc")));
diff --git a/arch/x86/include/asm/vdso/getrandom.h b/arch/x86/include/asm/vdso/getrandom.h
new file mode 100644
index 000000000000..00ca03e56ea9
--- /dev/null
+++ b/arch/x86/include/asm/vdso/getrandom.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+#ifndef __ASM_VDSO_GETRANDOM_H
+#define __ASM_VDSO_GETRANDOM_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/unistd.h>
+#include <asm/vvar.h>
+
+static __always_inline ssize_t
+getrandom_syscall(void *buffer, size_t len, unsigned int flags)
+{
+	long ret;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_getrandom), "D" (buffer), "S" (len), "d" (flags) :
+	     "rcx", "r11", "memory");
+
+	return ret;
+}
+
+static __always_inline void *
+mmap_syscall(void *addr, size_t len, int prot, int flags, int fd, off_t offset)
+{
+	long ret;
+	register long r10 asm("r10") = flags;
+	register long r8 asm("r8") = fd;
+	register long r9 asm("r9") = offset;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_mmap), "D" (addr), "S" (len), "d" (prot),
+	     "r" (r10), "r" (r8), "r" (r9) :
+	     "rcx", "r11");
+
+	return (void *)ret;
+}
+
+static __always_inline int
+munmap_syscall(void *addr, size_t len)
+{
+	long ret;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_munmap), "D" (addr), "S" (len) :
+	     "rcx", "r11");
+
+	return ret;
+}
+
+static __always_inline int
+madvise_syscall(void *addr, size_t len, int advice)
+{
+	long ret;
+
+	asm ("syscall" : "=a" (ret) :
+	     "0" (__NR_madvise), "D" (addr), "S" (len), "d" (advice) :
+	     "rcx", "r11");
+
+	return ret;
+}
+
+#define __vdso_rng_data (VVAR(_vdso_rng_data))
+
+static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
+{
+	return &__vdso_rng_data;
+}
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_GETRANDOM_H */
diff --git a/arch/x86/include/asm/vvar.h b/arch/x86/include/asm/vvar.h
index 183e98e49ab9..9d9af37f7cab 100644
--- a/arch/x86/include/asm/vvar.h
+++ b/arch/x86/include/asm/vvar.h
@@ -26,6 +26,8 @@
  */
 #define DECLARE_VVAR(offset, type, name) \
 	EMIT_VVAR(name, offset)
+#define DECLARE_VVAR_SINGLE(offset, type, name) \
+	EMIT_VVAR(name, offset)
 
 #else
 
@@ -37,6 +39,10 @@ extern char __vvar_page;
 	extern type timens_ ## name[CS_BASES]				\
 	__attribute__((visibility("hidden")));				\
 
+#define DECLARE_VVAR_SINGLE(offset, type, name)				\
+	extern type vvar_ ## name					\
+	__attribute__((visibility("hidden")));				\
+
 #define VVAR(name) (vvar_ ## name)
 #define TIMENS(name) (timens_ ## name)
 
@@ -44,12 +50,22 @@ extern char __vvar_page;
 	type name[CS_BASES]						\
 	__attribute__((section(".vvar_" #name), aligned(16))) __visible
 
+#define DEFINE_VVAR_SINGLE(type, name)					\
+	type name							\
+	__attribute__((section(".vvar_" #name), aligned(16))) __visible
+
 #endif
 
 /* DECLARE_VVAR(offset, type, name) */
 
 DECLARE_VVAR(128, struct vdso_data, _vdso_data)
 
+#if !defined(_SINGLE_DATA)
+#define _SINGLE_DATA
+DECLARE_VVAR_SINGLE(640, struct vdso_rng_data, _vdso_rng_data)
+#endif
+
 #undef DECLARE_VVAR
+#undef DECLARE_VVAR_SINGLE
 
 #endif
diff --git a/drivers/char/random.c b/drivers/char/random.c
index d44832e9e709..69694bb3209a 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -59,6 +59,7 @@
 #include <asm/irq.h>
 #include <asm/irq_regs.h>
 #include <asm/io.h>
+#include <vdso/datapage.h>
 
 /*********************************************************************
  *
@@ -84,6 +85,7 @@ static DEFINE_STATIC_KEY_FALSE(crng_is_ready);
 /* Various types of waiters for crng_init->CRNG_READY transition. */
 static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);
 static struct fasync_struct *fasync;
+DEFINE_VVAR_SINGLE(struct vdso_rng_data, _vdso_rng_data);
 
 /* Control how we warn userspace. */
 static struct ratelimit_state urandom_warning =
@@ -221,6 +223,7 @@ static void crng_reseed(void)
 		++next_gen;
 	WRITE_ONCE(base_crng.generation, next_gen);
 	WRITE_ONCE(base_crng.birth, jiffies);
+	smp_store_release(&_vdso_rng_data.generation, next_gen + 1);
 	if (!static_branch_likely(&crng_is_ready))
 		crng_init = CRNG_READY;
 	spin_unlock_irqrestore(&base_crng.lock, flags);
@@ -660,6 +663,7 @@ static void __cold _credit_init_bits(size_t bits)
 		crng_reseed(); /* Sets crng_init to CRNG_READY under base_crng.lock. */
 		if (static_key_initialized)
 			execute_in_process_context(crng_set_ready, &set_ready);
+		smp_store_release(&_vdso_rng_data.is_ready, true);
 		wake_up_interruptible(&crng_init_wait);
 		kill_fasync(&fasync, SIGIO, POLL_IN);
 		pr_notice("crng init done\n");
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 73eb622e7663..cbacfd923a5c 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -109,6 +109,11 @@ struct vdso_data {
 	struct arch_vdso_data	arch_data;
 };
 
+struct vdso_rng_data {
+	unsigned long generation;
+	bool is_ready;
+};
+
 /*
  * We use the hidden visibility to prevent the compiler from generating a GOT
  * relocation. Not only is going through a GOT useless (the entry couldn't and
@@ -120,6 +125,7 @@ struct vdso_data {
  */
 extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
 extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
+extern struct vdso_rng_data _vdso_rng_data __attribute__((visibility("hidden")));
 
 /*
  * The generic vDSO implementation requires that gettimeofday.h
diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
new file mode 100644
index 000000000000..d6368e05443b
--- /dev/null
+++ b/lib/vdso/getrandom.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/atomic.h>
+#include <linux/fs.h>
+#include <vdso/datapage.h>
+#include <asm/vdso/getrandom.h>
+#include <asm/vdso/vsyscall.h>
+#include <asm/page.h>
+#include <uapi/linux/mman.h>
+#include "../crypto/chacha.c"
+
+struct getrandom_state {
+	uint64_t last_reseed;
+	unsigned long generation;
+	union {
+		struct {
+			u8 key[CHACHA_KEY_SIZE];
+			u8 batch[CHACHA_BLOCK_SIZE * 3 / 2];
+		};
+		u8 key_batch[CHACHA_BLOCK_SIZE * 2];
+	};
+	u8 pos;
+	bool not_forked;
+};
+
+static void memcpy_and_zero(void *dst, void *src, size_t len)
+{
+#define CASCADE(type) \
+	while (len >= sizeof(type)) { \
+		*(type *)dst = *(type *)src; \
+		*(type *)src = 0; \
+		dst += sizeof(type); \
+		src += sizeof(type); \
+		len -= sizeof(type); \
+	}
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+#if BITS_PER_LONG == 64
+	CASCADE(u64);
+#endif
+	CASCADE(u32);
+	CASCADE(u16);
+#endif
+	CASCADE(u8);
+#undef CASCADE
+}
+
+static __always_inline ssize_t
+__cvdso_getrandom(void *opaque_state, void *buffer, size_t len, unsigned int flags)
+{
+	struct getrandom_state *state = opaque_state;
+	const struct vdso_rng_data *rng_info = __arch_get_vdso_rng_data();
+	const struct vdso_data *timebase = &__arch_get_vdso_data()[CS_HRES_COARSE];
+	const struct vdso_timestamp *course_mono = &timebase->basetime[CLOCK_MONOTONIC_COARSE];
+	u32 chacha_state[CHACHA_STATE_WORDS];
+	ssize_t ret = min_t(size_t, MAX_RW_COUNT, len);
+	size_t batch_len;
+
+	if (unlikely(!rng_info->is_ready))
+		return getrandom_syscall(buffer, len, flags);
+
+	if (unlikely(!len))
+		return 0;
+
+	if (unlikely(!READ_ONCE(state->not_forked)))
+		state->not_forked = true;
+
+retry_generation:
+	if (unlikely(state->generation != READ_ONCE(rng_info->generation) ||
+		     /* 15 sec is crude approximation of crng_has_old_seed(). In the future,
+		      * export this in rng_info->expiration, or similar. Needs improvement. */
+		     READ_ONCE(course_mono->sec) - state->last_reseed > 15)) {
+		if (getrandom_syscall(state->key, sizeof(state->key), 0) != sizeof(state->key))
+			return getrandom_syscall(buffer, len, flags);
+		/* We shouldn't be reading rng_info->generation afterwards, as technically it could
+		 * be bumped in between these two lines. Instead this should be set to the value
+		 * read in the `if ()` above. But in fact, the lazy semantics of generation bumping
+		 * always make this happen. So live with this for now. Needs improvement. */
+		state->generation = READ_ONCE(rng_info->generation);
+		state->last_reseed = READ_ONCE(course_mono->sec);
+		state->pos = sizeof(state->batch);
+	}
+
+	len = ret;
+more_batch:
+	batch_len = min_t(size_t, sizeof(state->batch) - state->pos, len);
+	if (batch_len) {
+		memcpy_and_zero(buffer, state->batch, batch_len);
+		state->pos += batch_len;
+		buffer += batch_len;
+		len -= batch_len;
+	}
+	if (!len) {
+		if (unlikely(state->generation != READ_ONCE(rng_info->generation)))
+			goto retry_generation;
+		if (unlikely(!READ_ONCE(state->not_forked))) {
+			state->not_forked = true;
+			goto retry_generation;
+		}
+		return ret;
+	}
+
+	chacha_init_consts(chacha_state);
+	memcpy(&chacha_state[4], state->key, CHACHA_KEY_SIZE);
+	memset(&chacha_state[12], 0, sizeof(u32) * 4);
+
+	while (len >= CHACHA_BLOCK_SIZE) {
+		chacha20_block(chacha_state, buffer);
+		if (unlikely(chacha_state[12] == 0))
+			++chacha_state[13];
+		buffer += CHACHA_BLOCK_SIZE;
+		len -= CHACHA_BLOCK_SIZE;
+	}
+
+	chacha20_block(chacha_state, state->key_batch);
+	if (unlikely(chacha_state[12] == 0))
+		++chacha_state[13];
+	chacha20_block(chacha_state, state->key_batch + CHACHA_BLOCK_SIZE);
+	state->pos = 0;
+	memzero_explicit(chacha_state, sizeof(chacha_state));
+	goto more_batch;
+}
+
+static __always_inline void *
+__cvdso_getrandom_alloc(size_t *num, size_t *size_per_each)
+{
+	void *state;
+	size_t alloc_size;
+
+	alloc_size = size_mul(*num, sizeof(struct getrandom_state));
+	if (alloc_size == SIZE_MAX)
+		return NULL;
+	alloc_size = roundup(alloc_size, PAGE_SIZE);
+
+	state = mmap_syscall(NULL, alloc_size, PROT_READ | PROT_WRITE,
+			     MAP_PRIVATE | MAP_ANONYMOUS | MAP_LOCKED, -1, 0);
+	if (state == (void *)~0UL)
+		return NULL;
+
+	if (madvise_syscall(state, alloc_size, MADV_WIPEONFORK)) {
+		munmap_syscall(state, alloc_size);
+		return NULL;
+	}
+
+	*num = alloc_size / sizeof(struct getrandom_state);
+	*size_per_each = sizeof(struct getrandom_state);
+	return state;
+}
-- 
2.35.1

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Florian Weimer]

* Jason A. Donenfeld:

> API-wise, vDSO getrandom has a pair of functions:
>
>   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned int flags);
>   void *getrandom_alloc([inout] size_t *num, [out] size_t *size_per_each);
>
> In the first function, the return value and the latter 3 arguments are
> the same as ordinary getrandom(), while the first argument is a pointer
> to some state allocated with getrandom_alloc(). getrandom_alloc() takes
> the desired number of states, and returns an array of states, the number
> actually allocated, and the size in bytes of each one, enabling a libc
> to use one per thread. We very intentionally do *not* leave state
> allocation up to the caller. There are too many weird things that can go
> wrong, and it's important that vDSO does not provide too generic of a
> mechanism. It's not going to store its state in just any old memory
> address. It'll do it only in ones it allocates.

I still don't see why this couldn't be per-thread state (if you handle
fork generations somehow).

I also think it makes sense to introduce batching for the system call
implementation first, and tie that to the vDSO acceleration.  I expect a
large part of the benefit comes from the batching, not the system call
avoidance.

Thanks,
Florian

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hey Florian,

On Mon, Aug 01, 2022 at 10:48:01AM +0200, Florian Weimer wrote:
> * Jason A. Donenfeld:
> 
> > API-wise, vDSO getrandom has a pair of functions:
> >
> >   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned int flags);
> >   void *getrandom_alloc([inout] size_t *num, [out] size_t *size_per_each);
> >
> > In the first function, the return value and the latter 3 arguments are
> > the same as ordinary getrandom(), while the first argument is a pointer
> > to some state allocated with getrandom_alloc(). getrandom_alloc() takes
> > the desired number of states, and returns an array of states, the number
> > actually allocated, and the size in bytes of each one, enabling a libc
> > to use one per thread. We very intentionally do *not* leave state
> > allocation up to the caller. There are too many weird things that can go
> > wrong, and it's important that vDSO does not provide too generic of a
> > mechanism. It's not going to store its state in just any old memory
> > address. It'll do it only in ones it allocates.
> 
> I still don't see why this couldn't be per-thread state (if you handle
> fork generations somehow).

That actually *is* the intent of this v2. Specifically, you call
getrandom_alloc and you get an *array* of states, which you can then
pass off to various threads. Since we have to allocate in page sizes, we
can't do this piecemeal, so this is a mechanism for giving out chunks of
them (~28 at a time), which you'd then give to threads as they're
created, making more as needed.

> I also think it makes sense to introduce batching for the system call
> implementation first, and tie that to the vDSO acceleration.  I expect a
> large part of the benefit comes from the batching, not the system call
> avoidance.

What I understand you to mean is that *instead of* doing vDSO, we could
just batch in the kernel, and reap most of the performance benefits. If
that turns out to be true, and then we don't even need this vDSO stuff,
I'd be really happy. So I'll give this a try.

One question is where to store that batch. On the surface, per-cpu seems
appealing, like what we do for get_random_u32() and such for kernel
callers. But per-cpu means disabling preemption, which then becomes a
problem when copying into userspace, where the copies can fault. So
maybe something more sensible is, like above, just doing this per-task.
I'll give it a stab and will let you know what it looks like.

Jason

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

On Sun, Jul 31, 2022 at 03:31:25AM +0200, Jason A. Donenfeld wrote:
> +	batch_len = min_t(size_t, sizeof(state->batch) - state->pos, len);
> +	if (batch_len) {
> +		memcpy_and_zero(buffer, state->batch, batch_len);

Should be state->batch + state->pos, of course.

> +		state->pos += batch_len;
> +		buffer += batch_len;
> +		len -= batch_len;
> +	}

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hi Florian,

On Mon, Aug 01, 2022 at 02:49:17PM +0200, Jason A. Donenfeld wrote:
> What I understand you to mean is that *instead of* doing vDSO, we could
> just batch in the kernel, and reap most of the performance benefits. If
> that turns out to be true, and then we don't even need this vDSO stuff,
> I'd be really happy. So I'll give this a try.
> 
> One question is where to store that batch. On the surface, per-cpu seems
> appealing, like what we do for get_random_u32() and such for kernel
> callers. But per-cpu means disabling preemption, which then becomes a
> problem when copying into userspace, where the copies can fault. So
> maybe something more sensible is, like above, just doing this per-task.
> I'll give it a stab and will let you know what it looks like.

So doing the batching in the kernel gives roughly a 2x performance boost
for the u32 case. Below is a little hacky patch you can play with. This
isn't the face melting 15x of the vDSO approach, but it is something, I
guess.

Jason

From 99a314f603c9cd173e6db2e3776eb76477283e1a Mon Sep 17 00:00:00 2001
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Mon, 1 Aug 2022 15:19:33 +0200
Subject: [PATCH] random: batch getrandom() output per-task

bla bla just a test

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/char/random.c | 68 +++++++++++++++++++++++++++----------------
 include/linux/sched.h |  6 ++++
 2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d44832e9e709..1be0fea81cea 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -400,49 +400,67 @@ EXPORT_SYMBOL(get_random_bytes);
 static ssize_t get_random_bytes_user(struct iov_iter *iter)
 {
 	u32 chacha_state[CHACHA_STATE_WORDS];
-	u8 block[CHACHA_BLOCK_SIZE];
-	size_t ret = 0, copied;
+	unsigned long next_gen;
+	size_t ret, copied, batch_len;
+	bool do_more = true;

 	if (unlikely(!iov_iter_count(iter)))
 		return 0;

-	/*
-	 * Immediately overwrite the ChaCha key at index 4 with random
-	 * bytes, in case userspace causes copy_to_iter() below to sleep
-	 * forever, so that we still retain forward secrecy in that case.
-	 */
-	crng_make_state(chacha_state, (u8 *)&chacha_state[4], CHACHA_KEY_SIZE);
-	/*
-	 * However, if we're doing a read of len <= 32, we don't need to
-	 * use chacha_state after, so we can simply return those bytes to
-	 * the user directly.
-	 */
-	if (iov_iter_count(iter) <= CHACHA_KEY_SIZE) {
-		ret = copy_to_iter(&chacha_state[4], CHACHA_KEY_SIZE, iter);
-		goto out_zero_chacha;
+retry_generation:
+	ret = 0;
+	next_gen = READ_ONCE(base_crng.generation);
+	if (unlikely(next_gen != current->rng_batch.generation || crng_has_old_seed())) {
+		current->rng_batch.position = sizeof(current->rng_batch.buf);
+		current->rng_batch.generation = next_gen;
 	}

-	for (;;) {
-		chacha20_block(chacha_state, block);
+more_batch:
+	batch_len = min_t(size_t, iov_iter_count(iter),
+			  sizeof(current->rng_batch.buf) - current->rng_batch.position);
+	if (batch_len) {
+		copied = copy_to_iter(current->rng_batch.buf + current->rng_batch.position,
+				      batch_len, iter);
+		ret += copied;
+		memset(current->rng_batch.buf + current->rng_batch.position, 0, batch_len);
+		current->rng_batch.position += batch_len;
+		if (!iov_iter_count(iter) || copied != batch_len)
+			goto out;
+	}
+
+	crng_make_state(chacha_state, current->rng_batch.buf + CHACHA_BLOCK_SIZE, 32);
+	while (iov_iter_count(iter) >= CHACHA_BLOCK_SIZE) {
+		chacha20_block(chacha_state, current->rng_batch.buf);
 		if (unlikely(chacha_state[12] == 0))
 			++chacha_state[13];

-		copied = copy_to_iter(block, sizeof(block), iter);
+		copied = copy_to_iter(current->rng_batch.buf, CHACHA_BLOCK_SIZE, iter);
 		ret += copied;
-		if (!iov_iter_count(iter) || copied != sizeof(block))
+		if (!iov_iter_count(iter) || copied != CHACHA_BLOCK_SIZE) {
+			do_more = false;
 			break;
+		}

-		BUILD_BUG_ON(PAGE_SIZE % sizeof(block) != 0);
+		BUILD_BUG_ON(PAGE_SIZE % CHACHA_BLOCK_SIZE != 0);
 		if (ret % PAGE_SIZE == 0) {
-			if (signal_pending(current))
+			if (signal_pending(current)) {
+				do_more = false;
 				break;
+			}
 			cond_resched();
 		}
 	}
-
-	memzero_explicit(block, sizeof(block));
-out_zero_chacha:
+	chacha20_block(chacha_state, current->rng_batch.buf);
+	current->rng_batch.position = 0;
 	memzero_explicit(chacha_state, sizeof(chacha_state));
+	if (do_more)
+		goto more_batch;
+
+out:
+	if (unlikely(ret && current->rng_batch.generation != READ_ONCE(base_crng.generation))) {
+		iov_iter_revert(iter, ret);
+		goto retry_generation;
+	}
 	return ret ? ret : -EFAULT;
 }

diff --git a/include/linux/sched.h b/include/linux/sched.h
index c46f3a63b758..6df125a43bb1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1500,6 +1500,12 @@ struct task_struct {
 	struct callback_head		l1d_flush_kill;
 #endif

+	struct {
+		unsigned long generation;
+		u8 buf[96];
+		u8 position;
+	} rng_batch;
+
 	/*
 	 * New fields for task_struct should be added above here, so that
 	 * they are included in the randomized portion of task_struct.
--
2.35.1

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Thomas Gleixner]

Jason!

On Sun, Jul 31 2022 at 01:45, Jason A. Donenfeld wrote:
> Thanks a bunch for chiming in. Indeed this whole thing is kind of crazy,
> so your input is particularly useful here.
>
> On Sat, Jul 30, 2022 at 08:48:42AM -0700, Linus Torvalds wrote:
>> It's just too specialized, and the people who care about performance
>> can - and do - do special things anyway.
>
> To be clear, I really would rather not do this. I'm not really looking
> for more stuff to do, and I don't tend to write (public) code "just
> 'cuz". My worry is that by /not/ doing it, footguns will proliferate.
> The glibc thing was what finally motivated me to want to at least sketch
> out a potential action to make this kind of (apparently common) urge of
> writing a userspace RNG safer.

But the user space tinkering will continue no matter what. They might
then just use the vdso to get access to the ready/generation bits. I've
seen "better" VDSO implementations to access time. :)

> So, anyway, if I do muster a v2 of this (perhaps just to see the idea
> through), the API might split in two to something like:
>
>   void *getrandom_allocate_states([inout] size_t *number_of_states, [out] size_t *length_per_state);
>   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned long flags);

I'm not seeing any reason to have those functions at all.

The only thing which would be VDSO worthy here is the access to
random_state->ready and random_state->generation as that's the
information which is otherwise not available to userspace.

So you can just have:

   int random_check_and_update_generation(u64 *generation);

Everything else is library material, really.

Thanks,

        tglx

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Thomas Gleixner]

On Sun, Jul 31 2022 at 03:31, Jason A. Donenfeld wrote:

> +vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vgetrandom.o

You clearly forgot to tell people that they need a special config to
make this compile.

I don't even have to try to see that this cannot build with a defconfig:

Lacks -pg for that file and the included chacha.c contains
EXPORT_SYMBOL() which is not really working in the VDSO.

> +DECLARE_VVAR_SINGLE(640, struct vdso_rng_data, _vdso_rng_data)
...
> +#define __vdso_rng_data (VVAR(_vdso_rng_data))
> +
> +static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
> +{
> +	return &__vdso_rng_data;
> +}

That's not working with time name spaces.

> +static __always_inline ssize_t
> +__cvdso_getrandom(void *opaque_state, void *buffer, size_t len, unsigned int flags)
> +{
> +	struct getrandom_state *state = opaque_state;
> +	const struct vdso_rng_data *rng_info = __arch_get_vdso_rng_data();

This gives you vvar__vdso_rng_data and that points to the VVAR page at
offset 640. That works up to the point where a task is part of a
non-root time name space.

The kernel side mapping (the one which is updated) looks like this:

    VVAR_PAGE
    VIRT_CLOCK_PAGE[S]
    TIMENS_PAGE

If time namespaces are disabled or the task is in the root time
namespace then the user mapping is in the same order.

If the task is in the non-root time namespace, then the user mapping is:

    TIMENS_PAGE
    VIRT_CLOCK_PAGE[S]
    VVAR_PAGE

So your user space looks at offset 640 in the TIMENS_PAGE, which has
rand_data->ready and rand_data->generation == 0 forever.

See the comment above timens_setup_vdso_data() and look at the way how
e.g. __cvdso_time_data() deals with that.

VDSO hacking is special and not a sunday evening project. :)

Thanks,

        tglx

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

On Mon, Aug 01, 2022 at 09:30:20PM +0200, Thomas Gleixner wrote:
> Jason!

Thomas!

> > So, anyway, if I do muster a v2 of this (perhaps just to see the idea
> > through), the API might split in two to something like:
> >
> >   void *getrandom_allocate_states([inout] size_t *number_of_states, [out] size_t *length_per_state);
> >   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned long flags);
> 
> I'm not seeing any reason to have those functions at all.
> 
> The only thing which would be VDSO worthy here is the access to
> random_state->ready and random_state->generation as that's the
> information which is otherwise not available to userspace.

I think you might have missed the part of the patch message where I
discuss this. I'm happy to talk about that more, but it might help the
discussion to refer to the parts already addressed. Reproduced here:

| How do we rectify this? By putting a safe implementation of getrandom()
| in the vDSO, which has access to whatever information a
| particular iteration of random.c is using to make its decisions. I use
| that careful language of "particular iteration of random.c", because the
| set of things that a vDSO getrandom() implementation might need for making
| decisions as good as the kernel's will likely change over time. This
| isn't just a matter of exporting certain *data* to userspace. We're not
| going to commit to a "data API" where the various heuristics used are
| exposed, locking in how the kernel works for decades to come, and then
| leave it to various userspaces to roll something on top and shoot
| themselves in the foot and have all sorts of complexity disasters.
| Rather, vDSO getrandom() is supposed to be the *same exact algorithm*
| that runs in the kernel, except it's been hoisted into userspace as
| much as possible. And so vDSO getrandom() and kernel getrandom() will
| always mirror each other hermetically.

To reiterate, I don't want to commit to a particular data API, or even
to an ideal interplay between kernel random and user random. I'd like to
retain the latitude to change the semantics there considerably, so that
Linux isn't locked into one RNG design forever. I think that kind of
lock in would be a mistake. For example, just the generation counter
alone won't do it (as I mentioned later on in the message; the RFC patch
is somewhat incomplete). Rather, the interface I'm fine committing to
would be the higher level getrandom(), with maybe an added state
parameter, which doesn't expose any guts about what it's actually doing.

Comex (CC'd) described in a forum comment the idea (and perhaps vDSO in
general?) as a little more akin to system libraries on Windows or macOS,
which represent the OS barrier, rather than the raw system call. Such
libraries then can operate on private data as necessary. So in that
sense, this patch here isn't very Linuxy (which Comex described as a
potentially positive thing, but I assume you disagree).

Anyway, I guess it in large part isn't so dissimilar to decisions you
made around other vDSO functions, where to draw the barrier, etc. Why
not just have an accessor for each vvar struct member and leave it to
userspaces to implement? Well, that'd probably be a terrible idea for
various reasons, and I feel the same way about exposing too many
getrandom() guts.

> So you can just have:
> 
>    int random_check_and_update_generation(u64 *generation);
> 
> Everything else is library material, really.

Not very appealing for the reasons mentioned above, but also for the
record, I may like this idea for a closely related thing, vmgenid, but
that's a different conversation I'll get back to another time.

Jason

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hi Thomas,

On Mon, Aug 01, 2022 at 10:48:56PM +0200, Thomas Gleixner wrote:
> On Sun, Jul 31 2022 at 03:31, Jason A. Donenfeld wrote:
> You clearly forgot to tell people that they need a special config to
> make this compile.

As I wrote in my patch body:

| The actual place that has the most work to do is in all of the other
| files. Most of the vDSO shared page infrastructure is centered around
| gettimeofday, and so the main structs are all in arrays for different
| timestamp types, and attached to time namespaces, and so forth. I've
| done the best I could to add onto this in an unintrusive way, but you'll
| notice almost immediately from glancing at the code that it still needs
| some untangling work. This also only works on x86 at the moment. I could
| certainly use a hand with this part.

So I'm not surprised other things are screwed up. This works well in my
test harness, indeed, but I imagine there are lots of fiddly bits like
that to work out. I wanted to send an RFC to elicit comments on the idea
and API before moving forward, as I have a strong sense this is one of
those "90% 10%" things, where 10% of the details take 90% of the time.

Also, I haven't hooked up vdso32 yet.

> > +vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vgetrandom.o
> 
> I don't even have to try to see that this cannot build with a defconfig:
> 
> Lacks -pg for that file and the included chacha.c contains
> EXPORT_SYMBOL() which is not really working in the VDSO.

Thanks, I'll address this if I do a v3. You meant the removal of -pg,
right? For the EXPORT_SYMBOL() stuff (and other symbols), I'm not sure
whether I'll add an #ifdef maze, hoist a static function into a .h file,
or just make another minier implementation of the necessary functions.
Each approach has a pitfall.

> > +DECLARE_VVAR_SINGLE(640, struct vdso_rng_data, _vdso_rng_data)
> ...
> > +#define __vdso_rng_data (VVAR(_vdso_rng_data))
> > +
> > +static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
> > +{
> > +	return &__vdso_rng_data;
> > +}
> 
> That's not working with time name spaces.

> > +static __always_inline ssize_t
> > +__cvdso_getrandom(void *opaque_state, void *buffer, size_t len, unsigned int flags)
> > +{
> > +	struct getrandom_state *state = opaque_state;
> > +	const struct vdso_rng_data *rng_info = __arch_get_vdso_rng_data();
> 
> This gives you vvar__vdso_rng_data and that points to the VVAR page at
> offset 640. That works up to the point where a task is part of a
> non-root time name space.
> 
> The kernel side mapping (the one which is updated) looks like this:
> 
>     VVAR_PAGE
>     VIRT_CLOCK_PAGE[S]
>     TIMENS_PAGE
> 
> If time namespaces are disabled or the task is in the root time
> namespace then the user mapping is in the same order.
> 
> If the task is in the non-root time namespace, then the user mapping is:
> 
>     TIMENS_PAGE
>     VIRT_CLOCK_PAGE[S]
>     VVAR_PAGE
> 
> So your user space looks at offset 640 in the TIMENS_PAGE, which has
> rand_data->ready and rand_data->generation == 0 forever.
> 
> See the comment above timens_setup_vdso_data() and look at the way how
> e.g. __cvdso_time_data() deals with that.

Ahhh, bingo! Thanks a lot for that. I couldn't quite grok before what
was happening with the timens stuff, but I think I get it now. When a
process is made in a timens, these pages are mapped differently, so that
the timens is in the same place as the init ns page would be. That's
clever. So I need to figure out some way to make __arch_get_vdso_rng_
data() always return the address of VVAR_PAGE, even when it's been
scooted down... I guess this means checking a bit in what's normally in
the vvar slot, and if it's a timens one, then loading the one that it's
in the timens slot, since that'll be the vvar one. Maybe that'll do it.

> VDSO hacking is special and not a sunday evening project. :)

While initially a somewhat bewildering maze, it's a rather fun puzzle.

Jason

Re: [PATCH RFC v2] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

On Tue, Aug 02, 2022 at 01:41:58AM +0200, Jason A. Donenfeld wrote:
> Hi Thomas,
> 
> On Mon, Aug 01, 2022 at 10:48:56PM +0200, Thomas Gleixner wrote:
> > On Sun, Jul 31 2022 at 03:31, Jason A. Donenfeld wrote:
> > You clearly forgot to tell people that they need a special config to
> > make this compile.
> 
> As I wrote in my patch body:
> 
> | The actual place that has the most work to do is in all of the other
> | files. Most of the vDSO shared page infrastructure is centered around
> | gettimeofday, and so the main structs are all in arrays for different
> | timestamp types, and attached to time namespaces, and so forth. I've
> | done the best I could to add onto this in an unintrusive way, but you'll
> | notice almost immediately from glancing at the code that it still needs
> | some untangling work. This also only works on x86 at the moment. I could
> | certainly use a hand with this part.
> 
> So I'm not surprised other things are screwed up. This works well in my
> test harness, indeed, but I imagine there are lots of fiddly bits like
> that to work out. I wanted to send an RFC to elicit comments on the idea
> and API before moving forward, as I have a strong sense this is one of
> those "90% 10%" things, where 10% of the details take 90% of the time.
> 
> Also, I haven't hooked up vdso32 yet.

Just to add, your "special config", "defconfig" remarks and such don't
strike me as very accurate. I had forgotten until a second ago that I'm
actually running this on my laptop's kernel, which is a fairly normal
"desktop distro" kernel with modules and lots of bloat turned on. So
c'mon now... the thing does work.

With that said, I've taken a stab at the various build, symbol, and
timens things you pointed out (thanks for that), and that's sitting now
in a branch, and will be part of a v3 if I ever post a v3:

https://git.zx2c4.com/linux-rng/commit/?h=jd/vdso

That pointer arithmetic around the timens stuff might not be super duper
appealing; I could add a symbol and have the linker resolve its location
and such. But also I wasn't keen on adding a symbol to a piece of data
that's not really there. So instead I calculate that offset from the
vdso_data and timens_vdso_data symbols.

By the way, in writing a test for that, I thought it was interesting
that CLONE_NEWTIME requires a new process, rather than just an execve.
You don't want to do it immediately, because that'd involve a
potentially racy remapping of the vvar pages. But at the very least it
could have been done on execve, right? Since all the maps are thrown out
then.

Jason

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Thomas Gleixner]

Jason!

On Tue, Aug 02 2022 at 01:16, Jason A. Donenfeld wrote:
> On Mon, Aug 01, 2022 at 09:30:20PM +0200, Thomas Gleixner wrote:
>> > So, anyway, if I do muster a v2 of this (perhaps just to see the idea
>> > through), the API might split in two to something like:
>> >
>> >   void *getrandom_allocate_states([inout] size_t *number_of_states, [out] size_t *length_per_state);
>> >   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned long flags);
>> 
>> I'm not seeing any reason to have those functions at all.
>> 
>> The only thing which would be VDSO worthy here is the access to
>> random_state->ready and random_state->generation as that's the
>> information which is otherwise not available to userspace.
>
> I think you might have missed the part of the patch message where I
> discuss this. I'm happy to talk about that more, but it might help the
> discussion to refer to the parts already addressed. Reproduced here:

I did not miss this. I carefully read it.

> To reiterate, I don't want to commit to a particular data API, or even
> to an ideal interplay between kernel random and user random. I'd like to
> retain the latitude to change the semantics there considerably, so that
> Linux isn't locked into one RNG design forever. I think that kind of
> lock in would be a mistake. For example, just the generation counter
> alone won't do it (as I mentioned later on in the message; the RFC patch
> is somewhat incomplete). Rather, the interface I'm fine committing to
> would be the higher level getrandom(), with maybe an added state
> parameter, which doesn't expose any guts about what it's actually doing.
>
> Comex (CC'd) described in a forum comment the idea (and perhaps vDSO in
> general?) as a little more akin to system libraries on Windows or macOS,
> which represent the OS barrier, rather than the raw system call. Such
> libraries then can operate on private data as necessary. So in that
> sense, this patch here isn't very Linuxy (which Comex described as a
> potentially positive thing, but I assume you disagree).
>
> Anyway, I guess it in large part isn't so dissimilar to decisions you
> made around other vDSO functions, where to draw the barrier, etc. Why
> not just have an accessor for each vvar struct member and leave it to
> userspaces to implement? Well, that'd probably be a terrible idea for
> various reasons, and I feel the same way about exposing too many
> getrandom() guts.

I surely understand your goal, but the real question is where we draw
the line and what kind of functionality should go into such a library
and what's the required justification for it.

The concept of system libraries on Windows NT was to provide different
APIs for application programmers: Win32, OS/2, Posix. That allowed to
change the actual syscalls without breaking existing applications. IOW,
it's just a glue layer which translates between application API and
syscall API.

Right now the Linux VDSO functions are 1:1 replacements for system calls
and not adding a magic pile of functionality which is otherwise not
available.

What you are proposing is to have an implementation which is not
available via a regular syscall. Which means you are creating a VDSO
only syscall which still has the same problem as any other syscall in
terms of API design and functionality which needs to be supported
forever.

Thanks,

        tglx

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hi Thomas,

On Tue, Aug 02, 2022 at 03:46:27PM +0200, Thomas Gleixner wrote:
> Right now the Linux VDSO functions are 1:1 replacements for system calls
> and not adding a magic pile of functionality which is otherwise not
> available.
> 
> What you are proposing is to have an implementation which is not
> available via a regular syscall. Which means you are creating a VDSO
> only syscall which still has the same problem as any other syscall in
> terms of API design and functionality which needs to be supported
> forever.

Wait, what? That's not correct. The WHOLE point is that vdso getrandom()
will generate bytes in the same way as the ordinary syscall, without
differences. Same function name, same algorithm. But just faster,
because vDSO. I explicitly don't want to dip into introducing something
different. That's the big selling point: that vDSO getrandom() and
syscall getrandom() are the same thing. If you trust one, you can trust
the other. If you expect properties of one, you get that from the other.
If you know the API of one, you can use the other.

There might be other valid objections to this whole thing, but "this is
different from the syscall" really isn't one of them. It's the same
ideaspace that motivated gettimeofday() and such.

Jason

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Thomas Gleixner]

On Tue, Aug 02 2022 at 15:59, Jason A. Donenfeld wrote:
> On Tue, Aug 02, 2022 at 03:46:27PM +0200, Thomas Gleixner wrote:
>> Right now the Linux VDSO functions are 1:1 replacements for system calls
>> and not adding a magic pile of functionality which is otherwise not
>> available.
>> 
>> What you are proposing is to have an implementation which is not
>> available via a regular syscall. Which means you are creating a VDSO
>> only syscall which still has the same problem as any other syscall in
>> terms of API design and functionality which needs to be supported
>> forever.
>
> Wait, what? That's not correct. The WHOLE point is that vdso getrandom()
> will generate bytes in the same way as the ordinary syscall, without
> differences. Same function name, same algorithm. But just faster,
> because vDSO. I explicitly don't want to dip into introducing something
> different. That's the big selling point: that vDSO getrandom() and
> syscall getrandom() are the same thing. If you trust one, you can trust
> the other. If you expect properties of one, you get that from the other.
> If you know the API of one, you can use the other.

Seriously no. All existing VDSO functions have exactly the same function
signature and semantics as their syscall counterparts. So they are drop
in equivalent.

But:

  ssize_t getrandom(void *, void *, size_t, unsigned int);

is very much different than

  ssize_t getrandom(void *, size_t, unsigned int);

Different signature and different semantics.

So you have to go through the whole process of a new ABI whether you
like it or not.

It does not matter whether they both produce random numbers. If your
argument would hold true, then you can also claim that openat(2) and
openat2(2) are the same thing because they both open a file.

Thanks,

        tglx

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

Hi Thomas,

On Tue, Aug 2, 2022 at 5:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> Seriously no.

Why so serious all at once? :-)

> All existing VDSO functions have exactly the same function
> signature and semantics as their syscall counterparts. So they are drop
> in equivalent.
>
> But:
>
>   ssize_t getrandom(void *, void *, size_t, unsigned int);
>
> is very much different than
>
>   ssize_t getrandom(void *, size_t, unsigned int);
>
> Different signature and different semantics.

Different signature, but basically the same semantics.

> So you have to go through the whole process of a new ABI whether you
> like it or not.

Ahh, in that sense. Yea, I'd rather not have to do that too, with the
additional opaque handle passed as the first argument. It'd be nice if
there were some private place where I could store the necessary state,
but I'm not really sure where that might be at the moment. If you have
any ideas, please let me know.

Jason

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Thomas Gleixner]

Jason!

On Tue, Aug 02 2022 at 17:26, Jason A. Donenfeld wrote:
> On Tue, Aug 2, 2022 at 5:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> Seriously no.
>
> Why so serious all at once? :-)

Because you triggered the 'now it gets serious' button with your "it's
the same" sentiment.

>> All existing VDSO functions have exactly the same function
>> signature and semantics as their syscall counterparts. So they are drop
>> in equivalent.
>>
>> But:
>>
>>   ssize_t getrandom(void *, void *, size_t, unsigned int);
>>
>> is very much different than
>>
>>   ssize_t getrandom(void *, size_t, unsigned int);
>>
>> Different signature and different semantics.
>
> Different signature, but basically the same semantics.

Not at all. The concept of 'basically same semantics' is a delusion. It
does not exist. Either it's the same or it's not.

I really want to see your reaction on a claim that some RNG
implementation is basically the same as the existing one. I'm sure you
buy that without complaints.

>> So you have to go through the whole process of a new ABI whether you
>> like it or not.
>
> Ahh, in that sense. Yea, I'd rather not have to do that too, with the
> additional opaque handle passed as the first argument. It'd be nice if
> there were some private place where I could store the necessary state,
> but I'm not really sure where that might be at the moment. If you have
> any ideas, please let me know.

That's exactly the problem. VDSO is a stateless syscall wrapper which
has to be self contained for obvious reasons.

My previous statement:

    Everything else is library material, really.

is based on that fact and not on the unwillingness to add magic muck to
the VDSO.

The unwillingness part is just the question:

    Is there a sensible usecase?

Assumed that there is a sensible usecase, there is a way out and that's
exactly the library part. You can make that VDSO interface versioned and
provide a library in tools/random/ which goes in lockstep with the VDSO
changes.

If the RNG tinkerers abuse that, then so be it. You can't do anything
about it whatever you try. They can abuse your magic vdso functionality
too.

That's very much the same as we have with e.g. perf. The old perf binary
still works, but it does not have access to the latest and greatest
features.

You can do very much the same in a kernel supplied helper library which
either can cope with the version change or falls back to
sys_getrandom().

Vs. the storage problem. That yells TLS, but that makes your process
wide sharing moot, which might not be the worst of all things IMO.

Thanks,

        tglx

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jason A. Donenfeld]

On Wed, Aug 03, 2022 at 12:27:43AM +0200, Thomas Gleixner wrote:
> Jason!

Thomas!!

> Not at all. The concept of 'basically same semantics' is a delusion. It
> does not exist. Either it's the same or it's not.
> 
> I really want to see your reaction on a claim that some RNG
> implementation is basically the same as the existing one. I'm sure you
> buy that without complaints.

I mean there are some undeniable similarities here. We might be arguing
over semantics of semantics at this point, but... Barring the additional
`void *state` argument, they take the same inputs and produce the same
outputs. Then, most importantly, the way they each produce randomness is
the same, using the same algorithms and same lifetime rules. For all
measures that are meaningful to me, they're the "same". Yes yes
technically that extra `void *state` means something is slightly
different.  But does that matter? I guess your point is that somehow it
does matter, and maybe that's where we disagree.

> >> So you have to go through the whole process of a new ABI whether you
> >> like it or not.
> >
> > Ahh, in that sense. Yea, I'd rather not have to do that too, with the
> > additional opaque handle passed as the first argument. It'd be nice if
> > there were some private place where I could store the necessary state,
> > but I'm not really sure where that might be at the moment. If you have
> > any ideas, please let me know.
> 
> That's exactly the problem. VDSO is a stateless syscall wrapper which
> has to be self contained for obvious reasons.

Unless each call can import and export any potential state. That is,
unless there's a `void *state` argument like what I added. Then the code
itself doesn't refer to any global state, but can still behave in a
stateful manner.

We could even establish this as a "convention" if necessary, and
document it as such. That way upgrading a syscall ABI to a vDSO stateful
ABI always follows some rule, and then this doesn't feel as ad-hoc.
Heck, it could even be an opaque type, `vdso_handle_t state`, which
would just be a unsigned long.

> My previous statement:
> 
>     Everything else is library material, really.
> 
> is based on that fact and not on the unwillingness to add magic muck to
> the VDSO.
> 
> The unwillingness part is just the question:
> 
>     Is there a sensible usecase?

In the use case department, by the way, apparently there really is.
arc4random() is too slow for chronyd:
https://sourceware.org/bugzilla/show_bug.cgi?id=29437
And that's on a kernel even with the "newer faster getrandom()".

> Assumed that there is a sensible usecase, there is a way out and that's
> exactly the library part. You can make that VDSO interface versioned and
> provide a library in tools/random/ which goes in lockstep with the VDSO
> changes.

That sounds absolutely dreadful; no way jose. Then we wind up having to
maintain the data in the vDSO as a particular version, and keep that
working into the future. That's not going to fly. As I wrote before, I
don't want to commit the RNG to preserving certain internal semantics
over time. That's not something I feel comfortable committing to. And
even if we can "add" new ones with a new version in your hypothetical
scheme, we'd still have to keep the old ones working, and that could
prove prohibitive of improvements. So that's not going to work.

And again, I don't see how this is any different than gettimeofday() and
such. Why didn't you just make versioned accessor functions for each one
of the various struct fields, and then stick some library code into
tools/timekeeping/ for glibc to copy and paste once and never update
again? Why isn't this a nightmare you chose to have each time the moon
is full? Clearly because doing so would be a maintenance disaster and
would impede future meaningful progress, not to mention proliferating
wrong means of reading the time.

Just as gettimeofday benefits from being an actual function in the vDSO,
so too does getrandom().

> Vs. the storage problem. That yells TLS, but that makes your process
> wide sharing moot, which might not be the worst of all things IMO.

Yea, TLS is what we want here. The `void *state` argument thing is meant
for this. You allocate an array of states using that alloc function, and
then you divvy them up per-thread.

Jason

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Jeffrey Walton]

On Thu, Aug 4, 2022 at 11:40 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Wed, Aug 03, 2022 at 12:27:43AM +0200, Thomas Gleixner wrote:
> > Jason!
>
>  ...
> > Vs. the storage problem. That yells TLS, but that makes your process
> > wide sharing moot, which might not be the worst of all things IMO.
>
> Yea, TLS is what we want here. The `void *state` argument thing is meant
> for this. You allocate an array of states using that alloc function, and
> then you divvy them up per-thread.

I think it would be wise to give each thread its own state. It will
simplify locking and help avoid contention.

Jeff

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Thomas Gleixner]

Jason!

On Thu, Aug 04 2022 at 17:23, Jason A. Donenfeld wrote:
> On Wed, Aug 03, 2022 at 12:27:43AM +0200, Thomas Gleixner wrote:
>> Not at all. The concept of 'basically same semantics' is a delusion. It
>> does not exist. Either it's the same or it's not.
>> 
>> I really want to see your reaction on a claim that some RNG
>> implementation is basically the same as the existing one. I'm sure you
>> buy that without complaints.
>
> I mean there are some undeniable similarities here.

Similarity yes. Semantically in the sense of an API definitely not.

> We might be arguing over semantics of semantics at this point,
> but... Barring the additional `void *state` argument, they take the
> same inputs and produce the same outputs. Then, most importantly, the
> way they each produce randomness is the same, using the same
> algorithms and same lifetime rules. For all measures that are
> meaningful to me, they're the "same". Yes yes technically that extra
> `void *state` means something is slightly different.  But does that
> matter? I guess your point is that somehow it does matter, and maybe
> that's where we disagree.

The point is that the API is different and you can't provide a API
compatible variant in the VDSO ever.

This matters because it requires auxilliary code outside of the VDSO or
even application changes.

>> >> So you have to go through the whole process of a new ABI whether you
>> >> like it or not.
>> >
>> > Ahh, in that sense. Yea, I'd rather not have to do that too, with the
>> > additional opaque handle passed as the first argument. It'd be nice if
>> > there were some private place where I could store the necessary state,
>> > but I'm not really sure where that might be at the moment. If you have
>> > any ideas, please let me know.
>> 
>> That's exactly the problem. VDSO is a stateless syscall wrapper which
>> has to be self contained for obvious reasons.
>
> Unless each call can import and export any potential state. That is,
> unless there's a `void *state` argument like what I added. Then the code
> itself doesn't refer to any global state, but can still behave in a
> stateful manner.

Which makes it an incompatible API with an untyped pointer which is
sloppy at best.

>> The unwillingness part is just the question:
>> 
>>     Is there a sensible usecase?
>
> In the use case department, by the way, apparently there really is.
> arc4random() is too slow for chronyd:
> https://sourceware.org/bugzilla/show_bug.cgi?id=29437
> And that's on a kernel even with the "newer faster getrandom()".

For a problem which does not require cryptographically secure random
numbers. Truly convincing.

>> Assumed that there is a sensible usecase, there is a way out and that's
>> exactly the library part. You can make that VDSO interface versioned and
>> provide a library in tools/random/ which goes in lockstep with the VDSO
>> changes.
>
> That sounds absolutely dreadful; no way jose. Then we wind up having to
> maintain the data in the vDSO as a particular version, and keep that
> working into the future. That's not going to fly. As I wrote before, I
> don't want to commit the RNG to preserving certain internal semantics
> over time. That's not something I feel comfortable committing to. And
> even if we can "add" new ones with a new version in your hypothetical
> scheme, we'd still have to keep the old ones working, and that could
> prove prohibitive of improvements. So that's not going to work.

That's a complete nonsensical argumentation. If you install a new
kernel, then the new library which exposes the application interface is
installed too. So where is the problem?

That needs obviously some infrastructure so that the kernel install
handles such libraries similar to modules, but that would be a
worthwhile exercise if we want to expose more information through VDSO.

Such a library exposes the API and the interface between the library and
the VDSO is kernel dependent. They are shipped in lockstep.

That way you avoid to implement a ton of syscall fallbacks for this muck
and just treat it as any other regular library. You even can hide the
whole allocation completely and make it a drop in replacement:

  getrandom(....)
        if (!tls->random_state)
           alloc_tls_random_state()
        ....

A real library can do that, VDSO not.

The only version information in that data interface would be:

struct vdso_random_data {
       unsigned int	version;
       ....
};

All other fields are version dependent and you can change them as you
want.

If some tinkerer uses the data interface directly and ignores the
version check, then he can keep the pieces. That's not any different
than the timekeeping data interface of the VDSO.

I've seen code which accesses the timekeeping VDSO data directly just
because some performance expert decided that rdtsc_ordered() is too slow
and a trivial rdtsc() is good enough. That exploded in their face when
we overhauled the VDSO.

Can you prevent that? No.

Does it matter? No.

The data interface is documented to be for the kernel supplied library
only and the version field is only for paranoia reasons to validate that
kernel and library agree.

That means if some tinkerer uses the data interface directly then he
either does a version check and bails out when the version is different
from the expected version or he falls flat on his face.

That's a very simple contract. Versioned interfaces are not required to
be backwards compatible. That's a matter of definition and documentation.

> And again, I don't see how this is any different than gettimeofday() and
> such. Why didn't you just make versioned accessor functions for each one
> of the various struct fields, and then stick some library code into
> tools/timekeeping/ for glibc to copy and paste once and never update
> again? Why isn't this a nightmare you chose to have each time the moon
> is full? Clearly because doing so would be a maintenance disaster and
> would impede future meaningful progress, not to mention proliferating
> wrong means of reading the time.

The time accessors have been proven to be performance critical and the
implementation is a drop in replacement for the syscall so that an extra
library is pointless and less performant.

But you are neither providing a convincing use case nor a drop in
replacement simply because it's technically impossible in the VDSO to
provide that with the features you want to add.

> Just as gettimeofday benefits from being an actual function in the vDSO,
> so too does getrandom().

time*() == vsdo_time*() but getrandom() != vdso_magic_getrandom()

Can you spot the difference?

>> Vs. the storage problem. That yells TLS, but that makes your process
>> wide sharing moot, which might not be the worst of all things IMO.
>
> Yea, TLS is what we want here. The `void *state` argument thing is meant
> for this. You allocate an array of states using that alloc function, and
> then you divvy them up per-thread.

How does the first caller know how many threads are going to use this
and how is the pointer handed over to another thread?. TLS == Thread
Local Storage which unsurprisingly means that such a pointer is thread
private.

Which part of the code is going to do that pointer sharing? Are you
going to hide that in the VDSO library too? You can't.

If you want to use TLS which is the only sensible solution then you have
to do the allocation per thread which avoids all the sharding issues and
screams even more for a lib_getrandom_$kernel_version.so solution.

But before we go there, please provide a sensible use case.

Thanks,

        tglx

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Florian Weimer]

* Thomas Gleixner:

> On Tue, Aug 02 2022 at 15:59, Jason A. Donenfeld wrote:
>> On Tue, Aug 02, 2022 at 03:46:27PM +0200, Thomas Gleixner wrote:
>>> Right now the Linux VDSO functions are 1:1 replacements for system calls
>>> and not adding a magic pile of functionality which is otherwise not
>>> available.
>>> 
>>> What you are proposing is to have an implementation which is not
>>> available via a regular syscall. Which means you are creating a VDSO
>>> only syscall which still has the same problem as any other syscall in
>>> terms of API design and functionality which needs to be supported
>>> forever.
>>
>> Wait, what? That's not correct. The WHOLE point is that vdso getrandom()
>> will generate bytes in the same way as the ordinary syscall, without
>> differences. Same function name, same algorithm. But just faster,
>> because vDSO. I explicitly don't want to dip into introducing something
>> different. That's the big selling point: that vDSO getrandom() and
>> syscall getrandom() are the same thing. If you trust one, you can trust
>> the other. If you expect properties of one, you get that from the other.
>> If you know the API of one, you can use the other.
>
> Seriously no. All existing VDSO functions have exactly the same function
> signature and semantics as their syscall counterparts. So they are drop
> in equivalent.
>
> But:
>
>   ssize_t getrandom(void *, void *, size_t, unsigned int);
>
> is very much different than
>
>   ssize_t getrandom(void *, size_t, unsigned int);
>
> Different signature and different semantics.

Just use

   ssize_t getrandom(size_t, unsigned int, void *);

then and have the system call ignore the argument.  There is recent
precedent for adding additional arguments to system calls, see
membarrier.  If we want to be super-conservative, we could add a new
flag and have the vDSO version always call into the kernel if the flag
isn't set.

*This* part is far less problematic compared to the approach to
per-thread memory allocation.

We now have:

* Explicit donation of memory areas to the kernel (set_robust_list,
  rseq).

* This getrandom_alloc vDSO call which does something unspecified and
  may return pointers which are or are not abstract.  (How is CRIU
  expected to handle this?)

* There's also userspace shadow stack coming.  I think the kernel moved
  away from implicit allocation, to something mmap-based.  It's not
  clear to me why that would be okay here, but not for shadow stacks.

Does io_uring have to handle a similar problem, too?

As long as the vDSO doesn't use private system calls, I don't expect any
practical problems, but this optimization doesn't really look to me like
something that intrinsically benefits from a completely new way of
allocating userspace memory for use by the kernel.

Thanks,
Florian

Re: [PATCH RFC v1] random: implement getrandom() in vDSO

[Peter Zijlstra]

On Wed, Aug 03, 2022 at 12:27:43AM +0200, Thomas Gleixner wrote:
> Assumed that there is a sensible usecase, there is a way out and that's
> exactly the library part. You can make that VDSO interface versioned and
> provide a library in tools/random/ which goes in lockstep with the VDSO
> changes.

Well.... there is still the old idea of making a genuine libkernel.so
which is part of the kernel and provided as a virtual file (say in
/sys/lib/).

Since that's perfectly in lock-step with the kerne image, it can be
used to avoid the vdso data layout compat issues. Only the actual
symbols provided by the library are ABI constrained, not their
implementation.