Re: [PATCH] Use getentropy() for seeding PRNG

[Janne Blomqvist <blomqvist.janne@gmail.com>]

On Tue, Aug 14, 2018 at 11:18 PM, Rainer Orth <ro@cebitec.uni-bielefeld.de>
wrote:

> Hi Janne,
>
> > PING
> >
> > On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist <
> blomqvist.janne@gmail.com>
> > wrote:
> >
> >> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> >>
> >>> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote:
> >>> > --- a/libgfortran/intrinsics/random.c
> >>> > +++ b/libgfortran/intrinsics/random.c
> >>> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen)
> >>> >    for (size_t i = 0; i < buflen / sizeof (unsigned int); i++)
> >>> >      rand_s (&b[i]);
> >>> >    return buflen;
> >>> > +#elif defined(HAVE_GETENTROPY)
> >>> > +  return getentropy (buf, buflen);
> >>> >  #else
> >>>
> >>> I wonder if it wouldn't be better to use getentropy only if it is
> >>> successful
> >>> and fall back to whatever you were doing before.
> >>>
> >>> E.g. on Linux, I believe getentropy in glibc just uses the getrandom
> >>> syscall, which has only been introduced in Linux kernel 3.17.
> >>>
> >>
> >> Yes, that is my understanding as well.
> >>
> >>
> >>> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it
> will
> >>> fail, even though reads from /dev/urandom could work.
> >>>
> >>
> >> Hmm, fair enough. So replace the random.c part of the patch with
> >>
> >> diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/
> >> random.c
> >> index 234c5ff95fd..229fa6995c0 100644
> >> --- a/libgfortran/intrinsics/random.c
> >> +++ b/libgfortran/intrinsics/random.c
> >> @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen)
> >>      rand_s (&b[i]);
> >>    return buflen;
> >>  #else
> >> -  /*
> >> -     TODO: When glibc adds a wrapper for the getrandom() system call
> >> -     on Linux, one could use that.
> >> -
> >> -     TODO: One could use getentropy() on OpenBSD.  */
> >> +#ifdef HAVE_GETENTROPY
> >> +  if (getentropy (buf, buflen) == 0)
> >> +    return 0;
> >> +#endif
> >>    int flags = O_RDONLY;
> >>  #ifdef O_CLOEXEC
> >>    flags |= O_CLOEXEC;
> >>
> >>
> >>
> >> Just to be sure, I regtested this slightly modified patch as well. Ok
> for
> >> trunk?
>
> the patch broke Solaris 11.3+ bootstrap:
>
> /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c: In function
> 'getosrandom':
> /vol/gcc/src/hg/trunk/local/libgfortran/intrinsics/random.c:314:7: error:
> implicit declaration of function 'getentropy'; did you mean 'get_nprocs'?
> [-Werror=implicit-function-declaration]
> 314 |   if (getentropy (buf, buflen) == 0)
>     |       ^~~~~~~~~~
>     |       get_nprocs
>
>
> According to the manpage, one needs to include <sys/random.h> to get the
> getentropy declaration.
>
> Fixed as follows.  Bootstraps on i386-pc-solaris2.11,
> i386-pc-solaris2.10 (which lacks getentropy), sparc-sun-solaris2.11, and
> x86_64-pc-linux-gnu still running, but all already into make check.
>
> Ok for mainline?
>
>         Rainer
>
>
Sorry about that, I wasn't aware that Solaris also has added getentropy.
Patch looks good, Ok for trunk!




-- 
Janne Blomqvist