[v5,1/3] random: add vgetrandom_alloc() syscall

  The vDSO getrandom() works over an opaque per-thread state of an
unexported size, which must be marked as MADV_WIPEONFORK and be
mlock()'d 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([inout] size_t *num, [out] size_t *size_per_each, unsigned int flags);

This takes the desired 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. (The
`flags` argument is always 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 following commit shows
an example of this, being used in conjunction with the getrandom() vDSO
function.

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.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 MAINTAINERS                             |  1 +
 arch/x86/entry/syscalls/syscall_32.tbl  |  1 +
 arch/x86/entry/syscalls/syscall_64.tbl  |  1 +
 arch/x86/include/asm/unistd.h           |  1 +
 drivers/char/random.c                   | 59 +++++++++++++++++++++++++
 include/uapi/asm-generic/unistd.h       |  7 ++-
 kernel/sys_ni.c                         |  3 ++
 lib/vdso/getrandom.h                    | 23 ++++++++++
 scripts/checksyscalls.sh                |  4 ++
 tools/include/uapi/asm-generic/unistd.h |  7 ++-
 10 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 lib/vdso/getrandom.h
  

On Sat, Nov 19, 2022 at 01:09:27PM +0100, Jason A. Donenfeld wrote:
> +SYSCALL_DEFINE3(vgetrandom_alloc, unsigned long __user *, num,
> +		unsigned long __user *, size_per_each, unsigned int, flags)
> +{
> +	unsigned long alloc_size;
> +	unsigned long num_states;
> +	unsigned long pages_addr;
> +	int ret;
> +
> +	if (flags)
> +		return -EINVAL;
> +
> +	if (get_user(num_states, num))
> +		return -EFAULT;
> +
> +	alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
> +	if (alloc_size == SIZE_MAX)
> +		return -EOVERFLOW;
> +	alloc_size = roundup(alloc_size, PAGE_SIZE);

Small detail: the roundup to PAGE_SIZE can make alloc_size overflow to 0.

Also, 'roundup(alloc_size, PAGE_SIZE)' could be 'PAGE_ALIGN(alloc_size)'.

> +	pages_addr = vm_mmap(NULL, 0, alloc_size, PROT_READ | PROT_WRITE,
> +			     MAP_PRIVATE | MAP_ANONYMOUS | MAP_LOCKED, 0);
> +	if (IS_ERR_VALUE(pages_addr))
> +		return pages_addr;

This will only succeed if the userspace process has permission to mlock pages,
i.e. if there is space available in RLIMIT_MEMLOCK or if process has
CAP_IPC_LOCK.  I suppose this is working as intended, as this syscall can be
used to try to allocate and mlock arbitrary amounts of memory.

I wonder if this permission check will cause problems.  Maybe there could be a
way to relax it for just one page per task?  I don't know how that would work,
though, especially when the planned usage involves userspace allocating a single
pool of these contexts per process that get handed out to threads.

- Eric
  

Hi Eric,

On Sat, Nov 19, 2022 at 12:39:26PM -0800, Eric Biggers wrote:
> On Sat, Nov 19, 2022 at 01:09:27PM +0100, Jason A. Donenfeld wrote:
> > +SYSCALL_DEFINE3(vgetrandom_alloc, unsigned long __user *, num,
> > +		unsigned long __user *, size_per_each, unsigned int, flags)
> > +{
> > +	unsigned long alloc_size;
> > +	unsigned long num_states;
> > +	unsigned long pages_addr;
> > +	int ret;
> > +
> > +	if (flags)
> > +		return -EINVAL;
> > +
> > +	if (get_user(num_states, num))
> > +		return -EFAULT;
> > +
> > +	alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
> > +	if (alloc_size == SIZE_MAX)
> > +		return -EOVERFLOW;
> > +	alloc_size = roundup(alloc_size, PAGE_SIZE);
> 
> Small detail: the roundup to PAGE_SIZE can make alloc_size overflow to 0.
> 
> Also, 'roundup(alloc_size, PAGE_SIZE)' could be 'PAGE_ALIGN(alloc_size)'.

Good catch, thanks. So perhaps this?

        alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
        if (alloc_size > SIZE_MAX - PAGE_SIZE + 1)
                return -EOVERFLOW;
        alloc_size = PAGE_ALIGN(alloc_size);

Does that look right?

> > +	pages_addr = vm_mmap(NULL, 0, alloc_size, PROT_READ | PROT_WRITE,
> > +			     MAP_PRIVATE | MAP_ANONYMOUS | MAP_LOCKED, 0);
> > +	if (IS_ERR_VALUE(pages_addr))
> > +		return pages_addr;
> 
> This will only succeed if the userspace process has permission to mlock pages,
> i.e. if there is space available in RLIMIT_MEMLOCK or if process has
> CAP_IPC_LOCK.  I suppose this is working as intended, as this syscall can be
> used to try to allocate and mlock arbitrary amounts of memory.
> 
> I wonder if this permission check will cause problems.  Maybe there could be a
> way to relax it for just one page per task?  I don't know how that would work,
> though, especially when the planned usage involves userspace allocating a single
> pool of these contexts per process that get handed out to threads.

Probably though, we don't want to create a mlock backdoor, right? I
suppose if a user is above RLIMIT_MEMLOCK, it'll just fallback to the
slowpath, which still works. That seems like an okay enough
circumstance.

Jason
  

On Sun, Nov 20, 2022 at 12:59:07AM +0100, Jason A. Donenfeld wrote:
> Hi Eric,
> 
> On Sat, Nov 19, 2022 at 12:39:26PM -0800, Eric Biggers wrote:
> > On Sat, Nov 19, 2022 at 01:09:27PM +0100, Jason A. Donenfeld wrote:
> > > +SYSCALL_DEFINE3(vgetrandom_alloc, unsigned long __user *, num,
> > > +		unsigned long __user *, size_per_each, unsigned int, flags)
> > > +{
> > > +	unsigned long alloc_size;
> > > +	unsigned long num_states;
> > > +	unsigned long pages_addr;
> > > +	int ret;
> > > +
> > > +	if (flags)
> > > +		return -EINVAL;
> > > +
> > > +	if (get_user(num_states, num))
> > > +		return -EFAULT;
> > > +
> > > +	alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
> > > +	if (alloc_size == SIZE_MAX)
> > > +		return -EOVERFLOW;
> > > +	alloc_size = roundup(alloc_size, PAGE_SIZE);
> > 
> > Small detail: the roundup to PAGE_SIZE can make alloc_size overflow to 0.
> > 
> > Also, 'roundup(alloc_size, PAGE_SIZE)' could be 'PAGE_ALIGN(alloc_size)'.
> 
> Good catch, thanks. So perhaps this?
> 
>         alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
>         if (alloc_size > SIZE_MAX - PAGE_SIZE + 1)
>                 return -EOVERFLOW;
>         alloc_size = PAGE_ALIGN(alloc_size);
> 
> Does that look right?

Yes.  Maybe use 'SIZE_MAX & PAGE_MASK'?

Another alternative is the following:

	if (num_states >
	    (SIZE_MAX & PAGE_MASK) / sizeof(struct vgetrandom_state))
		return -EOVERFLOW;
	alloc_size = PAGE_ALIGN(num_states * sizeof(struct vgetrandom_state));

- Eric
  

On Sat, Nov 19, 2022 at 05:40:04PM -0800, Eric Biggers wrote:
> On Sun, Nov 20, 2022 at 12:59:07AM +0100, Jason A. Donenfeld wrote:
> > Hi Eric,
> > 
> > On Sat, Nov 19, 2022 at 12:39:26PM -0800, Eric Biggers wrote:
> > > On Sat, Nov 19, 2022 at 01:09:27PM +0100, Jason A. Donenfeld wrote:
> > > > +SYSCALL_DEFINE3(vgetrandom_alloc, unsigned long __user *, num,
> > > > +		unsigned long __user *, size_per_each, unsigned int, flags)
> > > > +{
> > > > +	unsigned long alloc_size;
> > > > +	unsigned long num_states;
> > > > +	unsigned long pages_addr;
> > > > +	int ret;
> > > > +
> > > > +	if (flags)
> > > > +		return -EINVAL;
> > > > +
> > > > +	if (get_user(num_states, num))
> > > > +		return -EFAULT;
> > > > +
> > > > +	alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
> > > > +	if (alloc_size == SIZE_MAX)
> > > > +		return -EOVERFLOW;
> > > > +	alloc_size = roundup(alloc_size, PAGE_SIZE);
> > > 
> > > Small detail: the roundup to PAGE_SIZE can make alloc_size overflow to 0.
> > > 
> > > Also, 'roundup(alloc_size, PAGE_SIZE)' could be 'PAGE_ALIGN(alloc_size)'.
> > 
> > Good catch, thanks. So perhaps this?
> > 
> >         alloc_size = size_mul(num_states, sizeof(struct vgetrandom_state));
> >         if (alloc_size > SIZE_MAX - PAGE_SIZE + 1)
> >                 return -EOVERFLOW;
> >         alloc_size = PAGE_ALIGN(alloc_size);
> > 
> > Does that look right?
> 
> Yes.  Maybe use 'SIZE_MAX & PAGE_MASK'?
> 
> Another alternative is the following:
> 
> 	if (num_states >
> 	    (SIZE_MAX & PAGE_MASK) / sizeof(struct vgetrandom_state))
> 		return -EOVERFLOW;
> 	alloc_size = PAGE_ALIGN(num_states * sizeof(struct vgetrandom_state));

Thanks, that's much nicer.

Jason