[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <64831ed9-fd78-281b-b522-fc581b0d4587@hauke-m.de>
Date: Sun, 21 Jan 2018 14:22:56 +0100
From: Hauke Mehrtens <hauke@...ke-m.de>
To: musl@...ts.openwall.com, Rich Felker <dalias@...c.org>
Subject: Re: [PATCH v2] Add getrandom syscall wrapper and getentropy
function
On 01/09/2018 07:48 PM, Rich Felker wrote:
> 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.
Will do that.
>
>> +#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?
Ok I will change this, glibc made getentropy no cancellation point, but
getrandom is a cancellation point I will do it the same way here.
>From glibc documentation:
> The @code{getentropy} function is not a cancellation point. A call to
> @code{getentropy} can block if the system has just booted and the
> kernel entropy pool has not yet been initialized. In this case, the
> function will keep blocking even if a signal arrives, and return only
> after the entropy pool has been initialized.
> The @code{getrandom} function is a cancellation point.
Source:
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=92dcaa3e2f7bf0f7f1c04cd2fb6a317df1a4e225
>
>> + 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?
The man page said that getentropy only returns, EFAULT, EIO or ENOSYS.
Should I forward all errors?
>> +
>> + 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.
In glibc it is not a cancellation point, so should I call the syscall
directly with no cancellation point or should I call the getrandom())
function which has a cancellation point?
>> 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
>
Hauke
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.
