From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12347 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH v2] Add getrandom syscall wrapper and getentropy function Date: Tue, 9 Jan 2018 13:48:13 -0500 Message-ID: <20180109184813.GD1627@brightrain.aerifal.cx> References: <20180106220809.10078-1-hauke@hauke-m.de> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1515523594 5997 195.159.176.226 (9 Jan 2018 18:46:34 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 9 Jan 2018 18:46:34 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12363-gllmg-musl=m.gmane.org@lists.openwall.com Tue Jan 09 19:46:30 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1eYyul-0000n7-JW for gllmg-musl@m.gmane.org; Tue, 09 Jan 2018 19:46:23 +0100 Original-Received: (qmail 9660 invoked by uid 550); 9 Jan 2018 18:48:25 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 9636 invoked from network); 9 Jan 2018 18:48:25 -0000 Content-Disposition: inline In-Reply-To: <20180106220809.10078-1-hauke@hauke-m.de> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12347 Archived-At: 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 > + > +#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 > +#include > +#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 > +#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