Re: Implementing getrandom/getentropy, anybody?

[Samuel Thibault]

Hello,

Andrew Eggenberger, le jeu. 31 oct. 2019 22:55:58 -0500, a ecrit:
> I'm not sure if the call to __read_nocancel needs to be tested because
> read returns -1 on error anyway.

> +/* Write up to LENGTH bytes of randomness starting at BUFFER.
> +   Return the number of bytes written, or -1 on error.  */
> +ssize_t
> +getrandom (void *buffer, size_t length, unsigned int flags)
> +{
> +  const char *random_source = "/dev/urandom";
> +  size_t amount_read;
> +  int fd, err;
> +
> +  if (flags & GRND_RANDOM){
> +    random_source = "/dev/random";
> +  }

Look at existing code for the codestyle: braces are on their own line,
and indented. That being said, here there is no need for braces, so
don't put them.

> +  fd = __open_nocancel(random_source, O_CLOEXEC);

You still need O_RDONLY in addition to O_CLOEXEC.


> +  if (fd == -1){
> +    return -1;
> +  }

Again, no braces.


> +  amount_read = __read_nocancel(fd, buffer, length);
> +  if (amount_read == -1){
> +    return -1;
> +  }

This is leaking the fd in case of read error. You have to close it
before returning.


> +  err = __close_nocancel(fd);
> +  if (err == -1){
> +    return -1;
> +  }

Again, no braces.


Now, that being said you don't need to check for the value returned by
close.  The goal of getrandom was to get data.  So the value to be
returned by the function is amount_read, whether __close_nocancel
succeeds or not. Thus don't even test the content of amount_read and the
value returned by __close_nocancel: just return amount_read in all
cases.

Samuel