| Message ID | 20221121152909.3414096-3-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | implement getrandom() in vDSO | expand |
On 21/11/2022 16.29, Jason A. Donenfeld wrote: Cc += linux-api > > if (!new_block) > goto out; > new_cap = grnd_allocator.cap + num; > new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states)); > if (!new_states) { > munmap(new_block, num * size_per_each); Hm. This does leak an implementation detail of vgetrandom_alloc(), namely that it is based on mmap() of that size rounded up to page size. Do we want to commit to this being the proper way of disposing of a succesful vgetrandom_alloc(), or should there also be a vgetrandom_free(void *states, long num, long size_per_each)? And if so, what color should the bikeshed really have. I.e., - does it need to take that size_per_each parameter which the kernel knows - should it rather take the product so it can for now be a simple alias for munmap - should it also have a flags argument just because that's what all well-behaving syscalls have these days... Also, should vgetrandom_alloc() take a void *hint argument that would/could be passed through to mmap() to give userspace some control over where the memory is located - possibly only in the future, i.e. insist on it being NULL for now, but it could open the possibility for adding e.g. VGRND_MAP_FIXED[_NOREPLACE] that would translate to the corresponding MAP_ flags. Rasmus
* Jason A. Donenfeld: > static void *vgetrandom_alloc(size_t *num, size_t *size_per_each, unsigned int flags) > { > unsigned long ret = syscall(__NR_vgetrandom_alloc, num, size_per_each, flags); > return ret > -4096UL ? NULL : (void *)ret; > } The traditional syscall function returns -1 on error and set errors, so using unsing long and the 4096 is quite misleading. Thanks, Florian
Hi Florian, On Wed, Nov 23, 2022 at 11:48:06AM +0100, Florian Weimer wrote: > * Jason A. Donenfeld: > > > static void *vgetrandom_alloc(size_t *num, size_t *size_per_each, unsigned int flags) > > { > > unsigned long ret = syscall(__NR_vgetrandom_alloc, num, size_per_each, flags); > > return ret > -4096UL ? NULL : (void *)ret; > > } > > The traditional syscall function returns -1 on error and set errors, so > using unsing long and the 4096 is quite misleading. Not sure I have any idea at all whatsoever about what you're talking about. Firstly, the function you quoted is from the "sample userspace code" in the commit message, so it might not be code for the context you have in mind. Secondly, it's just doing the thing to figure out if the return value is an error value or a pointer. Were we in glibc, we'd write this as: return INTERNAL_SYSCALL_ERROR_P(r) ? NULL : (void *) r; Right? And if you look at the expansion of that glibc macro, it's just: #define INTERNAL_SYSCALL_ERROR_P(val) \ ((unsigned long int) (val) > -4096UL) So it looks like the same exact thing? The only difference I could see is that I assign it to a `unsigned long ret`, while glibc code tends to assign it to a `long r`? Is that the difference you're pointing out? Except that clearly doesn't matter because it just gets casted to unsigned by that macro anyway? Confused. Jason
Hi Rasmus, On Wed, Nov 23, 2022 at 09:51:04AM +0100, Rasmus Villemoes wrote: > On 21/11/2022 16.29, Jason A. Donenfeld wrote: > > Cc += linux-api > > > > > if (!new_block) > > goto out; > > new_cap = grnd_allocator.cap + num; > > new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states)); > > if (!new_states) { > > munmap(new_block, num * size_per_each); > > Hm. This does leak an implementation detail of vgetrandom_alloc(), > namely that it is based on mmap() of that size rounded up to page size. > Do we want to commit to this being the proper way of disposing of a > succesful vgetrandom_alloc(), or should there also be a > vgetrandom_free(void *states, long num, long size_per_each)? Yes, this is intentional, and this is exactly what I wanted to do. There are various wrappers of vm_mmap() throughout, mmap being one of them, and they typically then resort to munmap to unmap it. This is how userspace handles memory - maps, always maps. So I think doing that is fine and consistent. However, your point about it relying on it being a rounded up size isn't correct. `munmap` will unmap the whole page if the size you pass lies within a page. So `num * size_of_each` will always do the right thing, without needing to have userspace code round anything up. (From the man page: "The address addr must be a multiple of the page size (but length need not be). All pages containing a part of the indicated range are unmapped.") And as you can see in my example code, nothing is rounded up. So I don't know why you made that comment. > And if so, what color should the bikeshed really have. I.e., No color, thanks. > Also, should vgetrandom_alloc() take a void *hint argument that > would/could be passed through to mmap() to give userspace some control > over where the memory is located - possibly only in the future, i.e. > insist on it being NULL for now, but it could open the possibility for > adding e.g. VGRND_MAP_FIXED[_NOREPLACE] that would translate to the > corresponding MAP_ flags. I think adding more control is exactly what this is trying to avoid. It's very intentionally *not* a general allocator function, but something specific for vDSO getrandom(). However, it does already, in this very patchset here, take a (currently unused) flags argument, in case we have the need for later extension. In the meantime, however, I'm not very interested in complicating this interface into oblivion. Firstly, it ensures nothing will get done. But moreover, this interface needs to be somewhat future-proof, yes, but it does not need to be a general syscall; rather, it's a specific syscall for a specific task. Jason
* Jason A. Donenfeld: > Hi Florian, > > On Wed, Nov 23, 2022 at 11:48:06AM +0100, Florian Weimer wrote: >> * Jason A. Donenfeld: >> >> > static void *vgetrandom_alloc(size_t *num, size_t *size_per_each, unsigned int flags) >> > { >> > unsigned long ret = syscall(__NR_vgetrandom_alloc, num, size_per_each, flags); >> > return ret > -4096UL ? NULL : (void *)ret; >> > } >> >> The traditional syscall function returns -1 on error and set errors, so >> using unsing long and the 4096 is quite misleading. > > Not sure I have any idea at all whatsoever about what you're talking > about. Firstly, the function you quoted is from the "sample userspace > code" in the commit message, so it might not be code for the context you > have in mind. I'm talking about the syscall function that is available through userspace via <sys/syscall.h>. > Secondly, it's just doing the thing to figure out if the return value is > an error value or a pointer. Were we in glibc, we'd write this as: > > return INTERNAL_SYSCALL_ERROR_P(r) ? NULL : (void *) r; > > Right? And if you look at the expansion of that glibc macro, it's just: > > #define INTERNAL_SYSCALL_ERROR_P(val) \ > ((unsigned long int) (val) > -4096UL) > > So it looks like the same exact thing? syscall already does internally (with a translation to -1, not NULL), so the caller shouldn't do it again. The userspace syscall function does *not* return an error code. Thanks, Florian
Hi Florian, On Thu, Nov 24, 2022 at 06:28:44AM +0100, Florian Weimer wrote: > > Right? And if you look at the expansion of that glibc macro, it's just: > > > > #define INTERNAL_SYSCALL_ERROR_P(val) \ > > ((unsigned long int) (val) > -4096UL) > > > > So it looks like the same exact thing? > > syscall already does internally (with a translation to -1, not NULL), so > the caller shouldn't do it again. The userspace syscall function does > *not* return an error code. Ahh, okay. Thanks. I'll fix up the example to assume those semantics. Jason
On 24/11/2022 02.18, Jason A. Donenfeld wrote: > Hi Rasmus, > > On Wed, Nov 23, 2022 at 09:51:04AM +0100, Rasmus Villemoes wrote: >> On 21/11/2022 16.29, Jason A. Donenfeld wrote: >> >> Cc += linux-api >> >>> >>> if (!new_block) >>> goto out; >>> new_cap = grnd_allocator.cap + num; >>> new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states)); >>> if (!new_states) { >>> munmap(new_block, num * size_per_each); >> >> Hm. This does leak an implementation detail of vgetrandom_alloc(), >> namely that it is based on mmap() of that size rounded up to page size. >> Do we want to commit to this being the proper way of disposing of a >> succesful vgetrandom_alloc(), or should there also be a >> vgetrandom_free(void *states, long num, long size_per_each)? > > Yes, this is intentional, and this is exactly what I wanted to do. There > are various wrappers of vm_mmap() throughout, mmap being one of them, > and they typically then resort to munmap to unmap it. This is how > userspace handles memory - maps, always maps. So I think doing that is > fine and consistent. OK. Perhaps for the benefit of future libc implementors drop a comment somewhere as to how to dealloc the blob. > However, your point about it relying on it being a rounded up size isn't > correct. `munmap` will unmap the whole page if the size you pass lies > within a page. So `num * size_of_each` will always do the right thing, > without needing to have userspace code round anything up. (From the man > page: "The address addr must be a multiple of the page size (but length > need not be). I know, and I never said userspace needed to round anything up. All pages containing a part of the indicated range are > unmapped.") And as you can see in my example code, nothing is rounded > up. So I don't know why you made that comment. I made that comment because it's clear from what this does that you get something back that is _at least_ num*size_per_each in size, but what is not clear is that the actual allocation is exactly and will always be that size rounded up to a page size (and no more), so that munmap(num*size_per_each), with its well-known and documented semantics, will DTRT. > I think adding more control is exactly what this is trying to avoid. > It's very intentionally *not* a general allocator function, but > something specific for vDSO getrandom(). However, it does already, in > this very patchset here, take a (currently unused) flags argument, in > case we have the need for later extension. OK. Perhaps you can spend a few more words on why this allocation _needs_ to be MAP_LOCKED? That seems somewhat of a policy thing imposed by the kernel, something that would be better left to the libc or distro or whatnot to request via a flag. I could imagine applications that currently run at the mlock limit start failing after a libc upgrade - which could of course be considered a libc problem, and perhaps it's too unlikely to worry about. Rasmus
diff --git a/MAINTAINERS b/MAINTAINERS index 843dd6a49538..e0aa33f54c57 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17287,6 +17287,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 F: lib/vdso/getrandom.h RAPIDIO SUBSYSTEM diff --git a/drivers/char/random.c b/drivers/char/random.c index 9b64db52849f..5b51e1cb0fcf 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -61,6 +61,9 @@ #include <asm/irq.h> #include <asm/irq_regs.h> #include <asm/io.h> +#ifdef CONFIG_HAVE_VDSO_GETRANDOM +#include <vdso/datapage.h> +#endif #include "../../lib/vdso/getrandom.h" /********************************************************************* @@ -307,6 +310,9 @@ static void crng_reseed(struct work_struct *work) if (next_gen == ULONG_MAX) ++next_gen; WRITE_ONCE(base_crng.generation, next_gen); +#ifdef CONFIG_HAVE_VDSO_GETRANDOM + smp_store_release(&_vdso_rng_data.generation, next_gen + 1); +#endif if (!static_branch_likely(&crng_is_ready)) crng_init = CRNG_READY; spin_unlock_irqrestore(&base_crng.lock, flags); @@ -756,6 +762,9 @@ static void __cold _credit_init_bits(size_t bits) crng_reseed(NULL); /* Sets crng_init to CRNG_READY under base_crng.lock. */ if (static_key_initialized) execute_in_process_context(crng_set_ready, &set_ready); +#ifdef CONFIG_HAVE_VDSO_GETRANDOM + smp_store_release(&_vdso_rng_data.is_ready, true); +#endif 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/Kconfig b/lib/vdso/Kconfig index d883ac299508..c35fac664574 100644 --- a/lib/vdso/Kconfig +++ b/lib/vdso/Kconfig @@ -30,4 +30,9 @@ config GENERIC_VDSO_TIME_NS Selected by architectures which support time namespaces in the VDSO +config HAVE_VDSO_GETRANDOM + bool + help + Selected by architectures that support vDSO getrandom(). + endif diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c new file mode 100644 index 000000000000..da5ad9b193b2 --- /dev/null +++ b/lib/vdso/getrandom.c @@ -0,0 +1,113 @@ +// 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 "getrandom.h" + +static void memcpy_and_zero(void *dst, void *src, size_t len) +{ +#define CASCADE(type) \ + while (len >= sizeof(type)) { \ + __put_unaligned_t(type, __get_unaligned_t(type, src), dst); \ + __put_unaligned_t(type, 0, src); \ + dst += sizeof(type); \ + src += sizeof(type); \ + len -= sizeof(type); \ + } +#if IS_ENABLED(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_data(const struct vdso_rng_data *rng_info, void *buffer, size_t len, + unsigned int flags, void *opaque_state) +{ + ssize_t ret = min_t(size_t, MAX_RW_COUNT, len); + struct vgetrandom_state *state = opaque_state; + unsigned long current_generation; + void *orig_buffer = buffer; + size_t orig_len = len; + u32 counter[2] = { 0 }; + size_t batch_len, nblocks; + + /* + * If the kernel isn't yet initialized, then the various flags might have some effect + * that we can't emulate in userspace, so use the syscall. Otherwise, the flags have + * no effect, and can continue. + */ + if (unlikely(!rng_info->is_ready)) + return getrandom_syscall(orig_buffer, orig_len, flags); + + if (unlikely(!len)) + return 0; + +retry_generation: + current_generation = READ_ONCE(rng_info->generation); + if (unlikely(state->generation != current_generation)) { + /* Write the generation before filling the key, in case there's a fork before. */ + WRITE_ONCE(state->generation, current_generation); + /* If the generation is wrong, the kernel has reseeded, so we should too. */ + if (getrandom_syscall(state->key, sizeof(state->key), 0) != sizeof(state->key)) + return getrandom_syscall(orig_buffer, orig_len, flags); + /* Set state->pos so that the batch is considered emptied. */ + state->pos = sizeof(state->batch); + } + + len = ret; +more_batch: + /* First use whatever is left from the last call. */ + batch_len = min_t(size_t, sizeof(state->batch) - state->pos, len); + if (batch_len) { + /* Zero out bytes as they're copied out, to preserve forward secrecy. */ + memcpy_and_zero(buffer, state->batch + state->pos, batch_len); + state->pos += batch_len; + buffer += batch_len; + len -= batch_len; + } + if (!len) { + /* + * Since rng_info->generation will never be 0, we re-read state->generation, + * rather than using the local current_generation variable, to learn whether + * we forked. Primarily, though, this indicates whether the rng itself has + * reseeded, in which case we should generate a new key and start over. + */ + if (unlikely(READ_ONCE(state->generation) != READ_ONCE(rng_info->generation))) { + buffer = orig_buffer; + goto retry_generation; + } + return ret; + } + + /* Generate blocks of rng output directly into the buffer while there's enough left. */ + nblocks = len / CHACHA_BLOCK_SIZE; + if (nblocks) { + __arch_chacha20_blocks_nostack(buffer, state->key, counter, nblocks); + buffer += nblocks * CHACHA_BLOCK_SIZE; + len -= nblocks * CHACHA_BLOCK_SIZE; + } + + /* Refill the batch and then overwrite the key, in order to preserve forward secrecy. */ + __arch_chacha20_blocks_nostack(state->batch_key, state->key, counter, 2); + state->pos = 0; + goto more_batch; +} + +static __always_inline ssize_t +__cvdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state) +{ + return __cvdso_getrandom_data(__arch_get_vdso_rng_data(), buffer, len, flags, opaque_state); +}
Provide a generic C vDSO getrandom() implementation, which operates on an opaque state returned by vgetrandom_alloc() and produces random bytes the same way as getrandom(). This has a the API signature: ssize_t vgetrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state); The return value and the first 3 arguments are the same as ordinary getrandom(), while the last argument is a pointer to the opaque allocated state. Were all four arguments passed to the getrandom() syscall, nothing different would happen, and the functions would have the exact same behavior. The actual vDSO RNG algorithm implemented is the same one implemented by drivers/char/random.c, using the same fast-erasure techniques as that. Should the in-kernel implementation change, so too will the vDSO one. It requires an implementation of ChaCha20 that does not use any stack, in order to maintain forward secrecy, so this is left as an architecture-specific fill-in. Stack-less ChaCha20 is an easy algorithm to implement on a variety of architectures, so this shouldn't be too onerous. Initially, the state is keyless, and so the first call makes a getrandom() syscall to generate that key, and then uses it for subsequent calls. By keeping track of a generation counter, it knows when its key is invalidated and it should fetch a new one using the syscall. Later, more than just a generation counter might be used. Since MADV_WIPEONFORK is set on the opaque state, the key and related state is wiped during a fork(), so secrets don't roll over into new processes, and the same state doesn't accidentally generate the same random stream. The generation counter, as well, is always >0, so that the 0 counter is a useful indication of a fork() or otherwise uninitialized state. If the kernel RNG is not yet initialized, then the vDSO always calls the syscall, because that behavior cannot be emulated in userspace, but fortunately that state is short lived and only during early boot. If it has been initialized, then there is no need to inspect the `flags` argument, because the behavior does not change post-initialization regardless of the `flags` value. Since the opaque state passed to it is mutated, vDSO getrandom() is not reentrant, when used with the same opaque state, which libc should be mindful of. Together with the previous commit that introduces vgetrandom_alloc(), this functionality is intended to be integrated into libc's thread management. As an illustrative example, the following code might be used to do the same outside of libc. All of the static functions are to be considered implementation private, including the vgetrandom_alloc() syscall wrapper, which generally shouldn't be exposed outside of libc, with the non-static vgetrandom() function at the end being the exported interface. The various pthread-isms are expected to be elided into libc internals. This per-thread allocation scheme is very naive and does not shrink; other implementations may choose to be more complex. static void *vgetrandom_alloc(size_t *num, size_t *size_per_each, unsigned int flags) { unsigned long ret = syscall(__NR_vgetrandom_alloc, num, size_per_each, flags); return ret > -4096UL ? NULL : (void *)ret; } static struct { pthread_mutex_t lock; void **states; size_t len, cap; } grnd_allocator = { .lock = PTHREAD_MUTEX_INITIALIZER }; static void *vgetrandom_get_state(void) { void *state = NULL; pthread_mutex_lock(&grnd_allocator.lock); if (!grnd_allocator.len) { size_t new_cap, size_per_each, num = 16; /* Just a hint. */ void *new_block = vgetrandom_alloc(&num, &size_per_each, 0), *new_states; if (!new_block) goto out; new_cap = grnd_allocator.cap + num; new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states)); if (!new_states) { munmap(new_block, num * size_per_each); goto out; } grnd_allocator.cap = new_cap; grnd_allocator.states = new_states; for (size_t i = 0; i < num; ++i) { grnd_allocator.states[i] = new_block; new_block += size_per_each; } grnd_allocator.len = num; } state = grnd_allocator.states[--grnd_allocator.len]; out: pthread_mutex_unlock(&grnd_allocator.lock); return state; } static void vgetrandom_put_state(void *state) { if (!state) return; pthread_mutex_lock(&grnd_allocator.lock); grnd_allocator.states[grnd_allocator.len++] = state; pthread_mutex_unlock(&grnd_allocator.lock); } static struct { ssize_t(*fn)(void *buf, size_t len, unsigned long flags, void *state); pthread_key_t key; pthread_once_t initialized; } grnd_ctx = { .initialized = PTHREAD_ONCE_INIT }; static void vgetrandom_init(void) { if (pthread_key_create(&grnd_ctx.key, vgetrandom_put_state) != 0) return; grnd_ctx.fn = __vdsosym("LINUX_2.6", "__vdso_getrandom"); } ssize_t vgetrandom(void *buf, size_t len, unsigned long flags) { void *state; pthread_once(&grnd_ctx.initialized, vgetrandom_init); if (!grnd_ctx.fn) return getrandom(buf, len, flags); state = pthread_getspecific(grnd_ctx.key); if (!state) { state = vgetrandom_get_state(); if (pthread_setspecific(grnd_ctx.key, state) != 0) { vgetrandom_put_state(state); state = NULL; } if (!state) return getrandom(buf, len, flags); } return grnd_ctx.fn(buf, len, flags, state); } Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- MAINTAINERS | 1 + drivers/char/random.c | 9 ++++ include/vdso/datapage.h | 6 +++ lib/vdso/Kconfig | 5 ++ lib/vdso/getrandom.c | 113 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 134 insertions(+) create mode 100644 lib/vdso/getrandom.c