[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180109184813.GD1627@brightrain.aerifal.cx>
Date: Tue, 9 Jan 2018 13:48:13 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH v2] Add getrandom syscall wrapper and getentropy
function
On Sat, Jan 06, 2018 at 11:08:09PM +0100, Hauke Mehrtens wrote:
> This syscall is available since Linux 3.17 and was also implemented in
> glibc in version 2.25 using the same interfaces.
> The getrandom function is a pure syscall wrapper liker glibc does it.
> getentropy is implemented on top of the getrandom syscall and fills the
> buffer.
>
> Currently no fallback is implemented this could be possible by using
> AT_RANDOM in the future.
> ---
> include/sys/random.h | 21 +++++++++++++++++++++
> src/linux/getentropy.c | 29 +++++++++++++++++++++++++++++
> src/linux/getrandom.c | 7 +++++++
> 3 files changed, 57 insertions(+)
> create mode 100644 include/sys/random.h
> create mode 100644 src/linux/getentropy.c
> create mode 100644 src/linux/getrandom.c
>
> diff --git a/include/sys/random.h b/include/sys/random.h
> new file mode 100644
> index 00000000..5b09e394
> --- /dev/null
> +++ b/include/sys/random.h
> @@ -0,0 +1,21 @@
> +#ifndef _SYS_RANDOM_H
> +#define _SYS_RANDOM_H
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define __NEED_size_t
> +#define __NEED_ssize_t
> +#include <bits/alltypes.h>
> +
> +#define GRND_NONBLOCK 0x0001
> +#define GRND_RANDOM 0x0002
> +
> +ssize_t getrandom(void *buf, size_t buflen, unsigned int flags);
> +
> +int getentropy(void *buffer, size_t length);
Needs to drop namespace pollution in arg lists; just don't use names
here.
> +#ifdef __cplusplus
> +}
> +#endif
> +#endif
> diff --git a/src/linux/getentropy.c b/src/linux/getentropy.c
> new file mode 100644
> index 00000000..48ca3d51
> --- /dev/null
> +++ b/src/linux/getentropy.c
> @@ -0,0 +1,29 @@
> +#include <sys/random.h>
> +#include <errno.h>
> +#include "syscall.h"
> +
> +int getentropy(void *buffer, size_t length)
> +{
> + int ret;
> + char *pos = buffer;
> + size_t rem = length;
> +
> + if (length > 256) {
> + return __syscall_ret(-EIO);
> + }
> +
> + while (rem) {
> + ret = __syscall_cp(SYS_getrandom, pos, rem, 0);
My understanding was that the glibc choice was not to make getentropy
a cancellation point. If that's correct, is there a reason you want to
do it differently?
> + if (ret == -EINTR) {
> + continue;
This in particular seems inconsistent with making it cancellable, but
maybe the idea is to avoid issues where the caller does not check the
return.
> + } else if (ret == -EFAULT || ret == -ENOSYS) {
> + return __syscall_ret(ret);
> + } else if (ret <= 0) {
> + return __syscall_ret(-EIO);
> + }
Is there a reason for collapsing some possible error conditions but
not others?
> +
> + pos += ret;
> + rem -= ret;
> + }
> + return 0;
> +}
Overall, my leaning for getentropy would be to implement it as a
wrapper around getrandom using the public interfaces (e.g.
pthread_setcancelstate if cancellation is to be blocked, and errno
rather than -EFOO stuff), with the idea that it should not need to be
changed if we later implement fallbacks for getrandom and that it be a
"generic" file that not depend on musl internals.
> diff --git a/src/linux/getrandom.c b/src/linux/getrandom.c
> new file mode 100644
> index 00000000..795b932c
> --- /dev/null
> +++ b/src/linux/getrandom.c
> @@ -0,0 +1,7 @@
> +#include <sys/random.h>
> +#include "syscall.h"
> +
> +ssize_t getrandom(void *buf, size_t buflen, unsigned int flags)
> +{
> + return syscall_cp(SYS_getrandom, buf, buflen, flags);
> +}
This part looks fine, I think.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
