On Wed, Nov 30, 2022 at 04:39:55PM +0100, Jason A. Donenfeld wrote:
> 2) Convert vgetrandom_alloc() into a clone3-style syscall, as Christian
> suggested earlier, which might allow for a bit more overloading
> capability. That would be a struct that looks like:
>
> struct vgetrandom_alloc_args {
> __aligned_u64 flags;
> __aligned_u64 states;
> __aligned_u64 num;
> __aligned_u64 size_of_each;
> }
>
> - If flags is VGRA_ALLOCATE, states and size_of_each must be zero on
> input, while num is the hint, as is the case now. On output, states,
> size_of_each, and num are filled in.
>
> - If flags is VGRA_DEALLOCATE, states, size_of_each, and num must be as
> they were originally, and then it deallocates.
>
> I suppose (2) would alleviate your concerns entirely, without future
> uncertainty over what it'd be like to add special cases to munmap(). And
> it'd add a bit more future proofing to the syscall, depending on what we
> do.
>
> So maybe I'm warming up to that approach a bit.
So I just did a little quick implementation to see what it'd feel like,
and actually, it's quite simple, and might address a lot of concerns all
at once. What do you think of the below? Documentation and such still
needs work obviously, but the bones should be there.
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 4341c6a91207..dae6095b937d 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -189,44 +189,53 @@ int __cold execute_with_initialized_rng(struct notifier_block *nb)
/**
* sys_vgetrandom_alloc - Allocate opaque states for use with vDSO getrandom().
*
- * @num: On input, a pointer to a suggested hint of how many states to
- * allocate, and on output the number of states actually allocated.
- *
- * @size_per_each: The size of each state allocated, so that the caller can
- * split up the returned allocation into individual states.
- *
- * @flags: Currently always zero.
+ * @uargs: A vgetrandom_alloc_args which may be updated on return.
+ * allocate, and on output the number of states actually allocated.
+ * @usize: The size of @uargs, which determines the version of the struct used.
*
* The getrandom() vDSO function in userspace requires an opaque state, which
* this function allocates by mapping a certain number of special pages into
* the calling process. It takes a hint as to the number of opaque states
* desired, and provides the caller with the number of opaque states actually
* allocated, the size of each one in bytes, and the address of the first
- * state.
+ * state. Alternatively, if the VGRA_DEALLOCATE flag is specified, the provided
+ * states parameter is unmapped.
*
- * Returns the address of the first state in the allocation on success, or a
- * negative error value on failure.
+ * Returns 0 on success and an error value otherwise.
*/
-SYSCALL_DEFINE3(vgetrandom_alloc, unsigned int __user *, num,
- unsigned int __user *, size_per_each, unsigned int, flags)
+SYSCALL_DEFINE2(vgetrandom_alloc, struct vgetrandom_alloc_args __user *, uargs, size_t, usize)
{
const size_t state_size = sizeof(struct vgetrandom_state);
+ const size_t max_states = (SIZE_MAX & PAGE_MASK) / state_size;
+ struct vgetrandom_alloc_args args;
size_t alloc_size, num_states;
unsigned long pages_addr;
- unsigned int num_hint;
int ret;
- if (flags)
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+ if (usize < VGETRANDOM_ALLOC_ARGS_SIZE_VER0)
return -EINVAL;
+ ret = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+ if (ret)
+ return ret;
- if (get_user(num_hint, num))
- return -EFAULT;
+ /* Currently only VGRA_DEALLOCATE is defined. */
+ if (args.flags & ~VGRA_DEALLOCATE)
+ return -EINVAL;
- num_states = clamp_t(size_t, num_hint, 1, (SIZE_MAX & PAGE_MASK) / state_size);
- alloc_size = PAGE_ALIGN(num_states * state_size);
+ if (args.flags & VGRA_DEALLOCATE) {
+ if (args.size_per_each != state_size || args.num > max_states || !args.states)
+ return -EINVAL;
+ return vm_munmap(args.states, args.num * state_size);
+ }
- if (put_user(alloc_size / state_size, num) || put_user(state_size, size_per_each))
- return -EFAULT;
+ /* These don't make sense as input values if allocating, so reject them. */
+ if (args.size_per_each || args.states)
+ return -EINVAL;
+
+ num_states = clamp_t(size_t, args.num, 1, max_states);
+ alloc_size = PAGE_ALIGN(num_states * state_size);
pages_addr = vm_mmap(NULL, 0, alloc_size, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_LOCKED, 0);
@@ -237,7 +246,14 @@ SYSCALL_DEFINE3(vgetrandom_alloc, unsigned int __user *, num,
if (ret < 0)
goto err_unmap;
- return pages_addr;
+ args.num = num_states;
+ args.size_per_each = state_size;
+ args.states = pages_addr;
+
+ ret = -EFAULT;
+ if (copy_to_user(uargs, &args, sizeof(args)))
+ goto err_unmap;
+ return 0;
err_unmap:
vm_munmap(pages_addr, alloc_size);
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 7741dc94f10c..de4338e26db0 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -72,6 +72,7 @@ struct open_how;
struct mount_attr;
struct landlock_ruleset_attr;
enum landlock_rule_type;
+struct vgetrandom_alloc_args;
#include <linux/types.h>
#include <linux/aio_abi.h>
@@ -1006,9 +1007,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
void __user *uargs);
asmlinkage long sys_getrandom(char __user *buf, size_t count,
unsigned int flags);
-asmlinkage long sys_vgetrandom_alloc(unsigned int __user *num,
- unsigned int __user *size_per_each,
- unsigned int flags);
+asmlinkage long sys_vgetrandom_alloc(struct vgetrandom_alloc_args __user *uargs,
+ size_t size);
asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
asmlinkage long sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
asmlinkage long sys_execveat(int dfd, const char __user *filename,
diff --git a/include/uapi/linux/random.h b/include/uapi/linux/random.h
index e744c23582eb..49911ea2c343 100644
--- a/include/uapi/linux/random.h
+++ b/include/uapi/linux/random.h
@@ -55,4 +55,30 @@ struct rand_pool_info {
#define GRND_RANDOM 0x0002
#define GRND_INSECURE 0x0004
+/*
+ * Flags for vgetrandom_alloc(2)
+ *
+ * VGRA_DEALLOCATE Deallocate supplied states.
+ */
+#define VGRA_DEALLOCATE 0x0001ULL
+
+/**
+ * struct vgetrandom_alloc_args - Arguments for the vgetrandom_alloc(2) syscall.
+ *
+ * @flags: Zero or more VGRA_* flags.
+ * @states: Zero on input if allocating, and filled in on successful
+ * return. An existing allocation, if deallocating.
+ * @num: A hint as to the desired number of states, if allocating. The
+ * number of existing states in @states, if deallocating
+ * @size_per_each: The size of each state in @states.
+ */
+struct vgetrandom_alloc_args {
+ __aligned_u64 flags;
+ __aligned_u64 states;
+ __aligned_u64 num;
+ __aligned_u64 size_per_each;
+};
+
+#define VGETRANDOM_ALLOC_ARGS_SIZE_VER0 32 /* sizeof first published struct */
+
#endif /* _UAPI_LINUX_RANDOM_H */
