[PATCH v2] Add getrandom syscall wrapper and getentropy function

[PATCH v2] Add getrandom syscall wrapper and getentropy function

[Hauke Mehrtens]

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);
+
+#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);
+		if (ret == -EINTR) {
+			continue;
+		} else if (ret == -EFAULT || ret == -ENOSYS) {
+			return __syscall_ret(ret);
+		} else if (ret <= 0) {
+			return __syscall_ret(-EIO);
+		}
+
+		pos += ret;
+		rem -= ret;
+	}
+	return 0;
+}
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);
+}
-- 
2.11.0

Re: [PATCH v2] Add getrandom syscall wrapper and getentropy function

[William Pitcock]

Hi,

On Sat, Jan 6, 2018 at 4:08 PM, Hauke Mehrtens <hauke@hauke-m.de> 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);

Function prototypes in headers should be provided without naming the arguments.

> +
> +#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);
> +               if (ret == -EINTR) {
> +                       continue;
> +               } else if (ret == -EFAULT || ret == -ENOSYS) {
> +                       return __syscall_ret(ret);
> +               } else if (ret <= 0) {
> +                       return __syscall_ret(-EIO);
> +               }
> +
> +               pos += ret;
> +               rem -= ret;
> +       }
> +       return 0;
> +}
> 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);
> +}
> --
> 2.11.0
>

Otherwise this looks okay to me.

William

Re: [PATCH v2] Add getrandom syscall wrapper and getentropy function

[Rich Felker]

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

Re: [PATCH v2] Add getrandom syscall wrapper and getentropy function

[Hauke Mehrtens]

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