Re: [PATCH v9] Add getentropy, getrandom, <sys/random.h> [BZ #17252]

[Torvald Riegel <triegel at redhat dot com>]

On Wed, 2016-11-30 at 17:15 +0100, Florian Weimer wrote:
> On 11/30/2016 02:33 PM, Florian Weimer wrote:
> > This iteration of the patch implements both getrandom and getentropy at
> > the same time.  getrandom is a cancellation point, has short reads, can
> > return EINTR etc., but getentropy is the more uniform interface.  I
> > added a retry loop in the getentropy implementation, to avoid exposing
> > an implicit magic number in the kernel implementation (the maximum size
> > of a single read; it differs between Solaris/OpenBSD/Linux).
> >
> > (You get to decide which function is Mulready and which one is Lang.)
> >
> > getentropy definitely needs interposition protection because it is
> > frequently redefined.  We'll need to rebuild a distribution to see if
> > the current approach is sufficient.  For consistency, I also added
> > interposition protection for getrandom.
> >
> > In addition, I had to make a few fixes so that the ABI is stable across
> > supported kernel headers, and had to tweak the stub implementations, too.
> 
> I received a request off-list to align the getentropy behavior more 
> closely to the OpenBSD and Solaris behavior.  I think this makes sense 
> (I had not realized before they were so closely aligned), so I've added 
> the 256-byte check to the glibc version which is performed by the other 
> implementation in the attached version of the patch.

This basically looks good to me (though I'm no expert on the actual
syscall etc.).

I think it could be improved though how the manual explains the pools
for getrandom: first saying that one is blocking the other is
nonblocking, and then saying that the nonblocking one can actually cause
blocking too is surprising.  Do you need to call them the blocking and
the nonblocking source?  Maybe other names could be used?

Also, can you document all allowed error codes in the manual?  If you
cannot, it would be good to at least give an indication why we can't, or
where the user can figure out what errors to expect.

Last thing: You seem to return EIO for two different conditions in
getentropy, but only document one of them.