Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Zach Brown]

 SYNOPSIS
   #include linux/random.h
 
   int getrandom(void *buf, size_t buflen, unsigned int flags);

I certainly like the idea of getting entropy without having to worry
about fds.

   If the GRND_RANDOM flags bit is not set, then the /dev/raundom

(raundom typo)

 RETURN VALUE
On success, the number of bytes that was returned is returned.

The description talks about filling the buffer, maybe say 'the number of
bytes filled is returned'?  

 +DECLARE_COMPLETION(urandom_initialized);

static?

 +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 + unsigned int, flags)
 +{
 + int r;
 +

Michael Kerrisk wants you to return -EINVAL on unknown flags :)

http://lwn.net/Articles/588444/

 + if (count  256)
 + return -EINVAL;

I'd vote for not having the limit.  It seems easy enough to iterate over
the buffer.  We'd need to clamp the count to ssize_t, though.

 + if (flags  GRND_RANDOM) {
 + return _random_read(!(flags  GRND_BLOCK), buf, count);
 + }

Do we want it to block by default and have the flag be _NONBLOCK?  Feels
more.. familiar.

 + if (flags  GRND_BLOCK) {
 + r = wait_for_completion_interruptible(urandom_initialized);
 + if (r)
 + return r;

I can *never* remember the rules for -ERESTARTSYS.  The syscall callers
take care of this?

 + return urandom_read(NULL, buf, count, NULL);

I wonder if we want to refactor the entry points a bit more instead of
directly calling the device read functions.  get_random_bytes() and
urandom_read() both have their own uninitialied use warning message and
tracing.  Does the syscall want its own little extraction function as
well?

- z
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Zach Brown]

On Thu, Jul 17, 2014 at 04:54:17PM -0400, Theodore Ts'o wrote:
 On Thu, Jul 17, 2014 at 12:48:12PM -0700, Zach Brown wrote:
  
   + return urandom_read(NULL, buf, count, NULL);
  
  I wonder if we want to refactor the entry points a bit more instead of
  directly calling the device read functions.
 
 I could refactor the entropy point, but it probably wouldn't add any
 extra bloat, since the compiler would hopefully compile it away, but
 adding the extra static function would seem to make things less
 readable at least in my opinion.

Fair enough, I don't have a strong preference either way.  It was just
something I noticed when leafing through the (unfamiliar) code.

- z
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC -v2] random: introduce getrandom(2) system call

[Zach Brown]

On Thu, Jul 17, 2014 at 05:38:20PM -0400, Theodore Ts'o wrote:
 The getrandom(2) system call was requested by the LibreSSL Portable
 developers.  It is analoguous to the getentropy(2) system call in
 OpenBSD.

 +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 + unsigned int, flags)
 +{
 + int r;
 +
 + if (flags  ~(GRND_NONBLOCK|GRND_RANDOM))
 + return -EINVAL;
 +
 + if (count  INT_MAX)
 + count = INT_MAX;
 +
 + if (flags  GRND_RANDOM)
 + return _random_read(flags  GRND_NONBLOCK, buf, count);
 + if (flags  GRND_NONBLOCK) {
 + if (!completion_done(urandom_initialized))
 + return -EAGAIN;
 + } else {
 + r = wait_for_completion_interruptible(urandom_initialized);
 + if (r)
 + return r;
 + }
 + return urandom_read(NULL, buf, count, NULL);
 +}

I like how tiny this ends up being.  Feel free to add my rb:.

Reviewed-by: Zach Brown z...@zabbo.net

- z
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html