[v18,2/5] random: add vgetrandom_alloc() syscall

The vDSO getrandom() works over an opaque per-thread state of an
unexported size, which must be marked VM_WIPEONFORK, VM_DONTDUMP,
VM_NORESERVE, and VM_DROPPABLE for proper operation. Over time, the
nuances of these allocations may change or grow or even differ based on
architectural features.

The syscall has the signature:

  void *vgetrandom_alloc(unsigned int *num, unsigned int *size_per_each,
                         unsigned long addr, unsigned int flags);

This takes a hinted number of opaque states in `num`, and returns a
pointer to an array of opaque states, the number actually allocated back
in `num`, and the size in bytes of each one in `size_per_each`, enabling
a libc to slice up the returned array into a state per each thread,
while ensuring that no single state straddles a page boundary. (The
`flags` and `addr` arguments, as well as the `*size_per_each` input
value, are reserved for the future and are forced to be zero zero for
now.)

Libc is expected to allocate a chunk of these on first use, and then
dole them out to threads as they're created, allocating more when
needed. The returned address of the first state may be passed to
munmap(2) with a length of `DIV_ROUND_UP(num, PAGE_SIZE / size_per_each)
* PAGE_SIZE`, in order to deallocate the memory.

We very intentionally do *not* leave state allocation for vDSO
getrandom() up to userspace itself, but rather provide this new syscall
for such allocations. vDSO getrandom() must not store its state in just
any old memory address, but rather just ones that the kernel specially
allocates for it, leaving the particularities of those allocations up to
the kernel.

The allocation of states 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. Though, vgetrandom_alloc() is not
expected to be exposed outside of libc, and the pthread usage here is
expected to be elided into libc internals. This allocation scheme is
very naive and does not shrink; other implementations may choose to be
more complex.

  static void *vgetrandom_alloc(unsigned int *num, unsigned int *size_per_each)
  {
    *size_per_each = 0; /* Must be zero on input. */
    return (void *)syscall(__NR_vgetrandom_alloc, &num, &size_per_each,
                           0 /* reserved @addr */, 0 /* reserved @flags */);
  }

  static struct {
    pthread_mutex_t lock;
    void **states;
    size_t len, cap, size_per_each;
  } 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_t page_size = getpagesize();
      unsigned int num = sysconf(_SC_NPROCESSORS_ONLN); /* Could be arbitrary, just a hint. */
      unsigned int size_per_each;
      void *new_block = vgetrandom_alloc(&num, &size_per_each);
      void *new_states;

      if (new_block == MAP_FAILED)
        goto out;
      if (grnd_allocator.size_per_each && grnd_allocator.size_per_each != size_per_each)
        goto unmap;
      grnd_allocator.size_per_each = size_per_each;
      new_cap = grnd_allocator.cap + num;
      new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states));
      if (!new_states)
        goto unmap;
      grnd_allocator.cap = new_cap;
      grnd_allocator.states = new_states;

      for (size_t i = 0; i < num; ++i) {
        grnd_allocator.states[i] = new_block;
        if (((uintptr_t)new_block & (page_size - 1)) + size_per_each > page_size)
          new_block = (void *)(((uintptr_t)new_block + page_size) & (page_size - 1));
        else
          new_block += size_per_each;
      }
      grnd_allocator.len = num;
      goto success;

    unmap:
      munmap(new_block, DIV_ROUND_UP(num, page_size / size_per_each) * page_size);
      goto out;
    }
  success:
    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);
  }

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 MAINTAINERS              |   1 +
 drivers/char/random.c    | 135 ++++++++++++++++++++++++++++++++++++++-
 include/linux/syscalls.h |   3 +
 include/vdso/getrandom.h |  16 +++++
 kernel/sys_ni.c          |   3 +
 lib/vdso/Kconfig         |   6 ++
 6 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 include/vdso/getrandom.h

Jason!

On Thu, Jun 20 2024 at 14:18, Jason A. Donenfeld wrote:
> On Wed, Jun 19, 2024 at 07:13:26PM -0700, Aleksa Sarai wrote:
>> Then again, I guess since libc is planned to be the primary user,
>> creating a new syscall in a decade if necessary is probably not that big
>> of an issue.
>
> I'm not sure going the whole big struct thing is really necessary, and
> for an additional reason: this is only meant to be used with the vDSO
> function, which is also coupled with the kernel. It doesn't return
> information that's made to be used (or allowed to be used) anywhere
> else. So both the vdso code and the syscall code are part of the same
> basic thing that will evolve together. So I'm not convinced extensible
> struct really makes sense for this, as neat as it is.
>
> If there's wide consensus that it's desirable, in contrast to what I'm
> saying, I'm not vehemently opposed to it and could do it, but it just
> seems like massive overkill and not at all necessary. Things are
> intentionally as simple and straightforward as can be.

Right, but the problem is that this is a syscall, so people are free to
explore it even without the vdso part. Now when you want to change it
later then you are caught in the no-regression trap.

So making it extensible with backwards compability in place (add the
unused flag field and check for 0) will allow you to expand without
breaking users.

Thanks,

        tglx

On Fri, Jun 28, 2024 at 03:56:12PM +0200, Thomas Gleixner wrote:
> Jason!
> 
> On Thu, Jun 20 2024 at 14:18, Jason A. Donenfeld wrote:
> > On Wed, Jun 19, 2024 at 07:13:26PM -0700, Aleksa Sarai wrote:
> >> Then again, I guess since libc is planned to be the primary user,
> >> creating a new syscall in a decade if necessary is probably not that big
> >> of an issue.
> >
> > I'm not sure going the whole big struct thing is really necessary, and
> > for an additional reason: this is only meant to be used with the vDSO
> > function, which is also coupled with the kernel. It doesn't return
> > information that's made to be used (or allowed to be used) anywhere
> > else. So both the vdso code and the syscall code are part of the same
> > basic thing that will evolve together. So I'm not convinced extensible
> > struct really makes sense for this, as neat as it is.
> >
> > If there's wide consensus that it's desirable, in contrast to what I'm
> > saying, I'm not vehemently opposed to it and could do it, but it just
> > seems like massive overkill and not at all necessary. Things are
> > intentionally as simple and straightforward as can be.
> 
> Right, but the problem is that this is a syscall, so people are free to
> explore it even without the vdso part. Now when you want to change it
> later then you are caught in the no-regression trap.
> 
> So making it extensible with backwards compability in place (add the
> unused flag field and check for 0) will allow you to expand without
> breaking users.

Okay, so it sounds like you're also in camp-struct. I guess let's do it
then. This opens up a few questions, but I think we can get them sorted.
Right now this version of the patch has this signature:

  void *vgetrandom_alloc(unsigned int *num, unsigned int *size_per_each,
                         unsigned long addr, unsigned int flags);

The semantics are currently:

- [in] unsigned int num - desired number of states
- [in] unsigned long addr - reserved, nothing
- [in] unsigned int flags - reserved, nothing
- [out] unsigned int num - actual number of states
- [out] unsigned int size_per_each - size of each state
- [out] void* return value - the allocated thing

Following Aleksa's suggestion, we keep the `[out] void* return value` as
a return value, but move all the other into a struct:

    void *vgetrandom_alloc(struct vgetrandom_args *arg, size_t size);

So now the struct can become:

    struct vgetrandom_args {
        [in] u64 flags;
        [in/out] u32 num;
        [out] u32 size_per_each;
    }

Alternatively, this now opens the possibility to incorporate Eric's
suggestion of also returning the number of allocated bytes, which is
perhaps definitely to deal with, but I didn't do because I wanted
symmetry in the argument list. So doing that, now we have:

    struct vgetrandom_args {
        [in] u64 flags;
        [in/out] u32 num;
        [out] u32 size_per_each;
        [out] u64 bytes_allocated;
    }

Does that seem reasonable? There's a little bit of mixing of ins and
outs within the struct, and the return value is still a return value,
rather than a `[out] void *ret` inside of the struct. But maybe that's
fine. Also I used u32 there for the two smaller arguments, but maybe
that's silly and we should go straight to u64?

Anyway, how does that look to you?

Jason

On Fri, Jun 28, 2024 at 4:09 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> perhaps definitely to deal with,

What?! I meant "definitely easier to deal with".

On Fri, Jun 28, 2024 at 04:09:01PM +0200, Jason A. Donenfeld wrote:
> fine. Also I used u32 there for the two smaller arguments, but maybe
> that's silly and we should go straight to u64?

Judging by `struct clone_args`, it looks like I've got to use
__aligned_u64 for every argument:

    struct clone_args {
        __aligned_u64 flags;
        __aligned_u64 pidfd;
        __aligned_u64 child_tid;
        __aligned_u64 parent_tid;
        __aligned_u64 exit_signal;
        __aligned_u64 stack;
        __aligned_u64 stack_size;
        __aligned_u64 tls;
        __aligned_u64 set_tid;
        __aligned_u64 set_tid_size;
        __aligned_u64 cgroup;
    };
    #define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */
    #define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */
    #define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */
    
So okay, I'll do that, and will have an ARGS_SIZE_VER0 macro too.

Jason

On Mon, Jul 1, 2024 at 1:53 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Fri, Jun 28, 2024 at 04:09:01PM +0200, Jason A. Donenfeld wrote:
> > fine. Also I used u32 there for the two smaller arguments, but maybe
> > that's silly and we should go straight to u64?
>
> Judging by `struct clone_args`, it looks like I've got to use
> __aligned_u64 for every argument:
>
>     struct clone_args {
>         __aligned_u64 flags;
>         __aligned_u64 pidfd;
>         __aligned_u64 child_tid;
>         __aligned_u64 parent_tid;
>         __aligned_u64 exit_signal;
>         __aligned_u64 stack;
>         __aligned_u64 stack_size;
>         __aligned_u64 tls;
>         __aligned_u64 set_tid;
>         __aligned_u64 set_tid_size;
>         __aligned_u64 cgroup;
>     };
>     #define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */
>     #define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */
>     #define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */
>
> So okay, I'll do that, and will have an ARGS_SIZE_VER0 macro too.

This is now covered by v19 of this patchset:
https://lore.kernel.org/lkml/20240701135801.3698-1-Jason@zx2c4.com/