diff mbox series

random: vDSO getrandom() must reject invalid flag

Message ID 20240825144758.325298-1-yann@droneaud.fr (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series random: vDSO getrandom() must reject invalid flag | expand

Commit Message

Yann Droneaud Aug. 25, 2024, 2:47 p.m. UTC
Like getrandom() syscall, vDSO getrandom() must not let
unknown flags unnoticed [1].

It could be possible to return -EINVAL from vDSO, but
in the likely case a new flag is added to getrandom()
syscall in the future, it would be nicer to get the
behavior from the syscall, instead of an error until
the vDSO is extended to support the new flag.

[1] Designing the API: Planning for Extension
    https://docs.kernel.org/process/adding-syscalls.html#designing-the-api-planning-for-extension

Signed-off-by: Yann Droneaud <yann@droneaud.fr>
---
 lib/vdso/getrandom.c | 4 ++++
 1 file changed, 4 insertions(+)

Hi Jason,

Please indulge me as I'm a bit late to add some junk to the conversation[2].

[2] Re: [RFC PATCH 0/4] random: a simple vDSO mechanism for reseeding userspace CSPRNGs
    https://lore.kernel.org/all/CAHmME9oXB8=jUz98tv6k1xS+ELaRmgartoT6go_1axhH1L-HJg@mail.gmail.com/

Bye.

Comments

Jason A. Donenfeld Aug. 26, 2024, 6:53 a.m. UTC | #1
Hi Yann,

On Sun, Aug 25, 2024 at 04:47:50PM +0200, Yann Droneaud wrote:
> Like getrandom() syscall, vDSO getrandom() must not let
> unknown flags unnoticed [1].
> 
> It could be possible to return -EINVAL from vDSO, but
> in the likely case a new flag is added to getrandom()
> syscall in the future, it would be nicer to get the
> behavior from the syscall, instead of an error until
> the vDSO is extended to support the new flag.

Thanks, that seems right to me.

Currently the @flags only matter if the RNG isn't initialized yet, so we
fallback if it's not initialized. But if it is initialized, all of the
flags behave the same way, so it didn't bother checking them. But that
doesn't account for invalid flags, and you're right to point out that
accepting them silently is an API problem.

I've applied this here, and I'll send it in as a fix soon:

    https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/commit/?id=ed9fbbeb29


Thanks for the patch,
Jason
diff mbox series

Patch

diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
index b230f0b10832..be9db42c8309 100644
--- a/lib/vdso/getrandom.c
+++ b/lib/vdso/getrandom.c
@@ -89,6 +89,10 @@  __cvdso_getrandom_data(const struct vdso_rng_data *rng_info, void *buffer, size_
 	if (unlikely(opaque_len != sizeof(*state)))
 		goto fallback_syscall;
 
+	/* Unexpected flags are to be handled by the kernel */
+	if (unlikely(flags & ~(GRND_NONBLOCK | GRND_RANDOM | GRND_INSECURE)))
+		goto fallback_syscall;
+
 	/*
 	 * If the kernel's RNG is not yet ready, then it's not possible to provide random bytes from
 	 * userspace, because A) the various @flags require this to block, or not, depending on