mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: musl@lists.openwall.com
Subject: Re: [musl] [PATCH] getentropy: fail if buffer not completely filled
Date: Sat, 9 Apr 2022 09:18:17 -0400	[thread overview]
Message-ID: <20220409131816.GJ7074@brightrain.aerifal.cx> (raw)
In-Reply-To: <20220409001047.234283-1-Jason@zx2c4.com>

On Sat, Apr 09, 2022 at 02:10:47AM +0200, Jason A. Donenfeld wrote:
> The man page for getentropy says that it either completely succeeds or
> completely fails, and indeed this is what glibc does. However, musl has
> a condition where it breaks out of the loop early, yet still returns a
> success. This patch fixes that by returning a success only if the buffer
> is completely filled. While we're at it, prevent an unexpected infinite
> loop if the function returns 0, the same way glibc does, just in case.
> ---
>  src/misc/getentropy.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/misc/getentropy.c b/src/misc/getentropy.c
> index 651ea95f..5b2fc7a1 100644
> --- a/src/misc/getentropy.c
> +++ b/src/misc/getentropy.c
> @@ -6,7 +6,7 @@
>  
>  int getentropy(void *buffer, size_t len)
>  {
> -	int cs, ret = 0;
> +	int cs, ret;
>  	char *pos = buffer;
>  
>  	if (len > 256) {
> @@ -18,16 +18,19 @@ int getentropy(void *buffer, size_t len)
>  
>  	while (len) {
>  		ret = getrandom(pos, len, 0);
> -		if (ret < 0) {
> +		if (ret <= 0) {
>  			if (errno == EINTR) continue;
>  			else break;
>  		}
>  		pos += ret;
>  		len -= ret;
> -		ret = 0;
>  	}
>  
>  	pthread_setcancelstate(cs, 0);
>  
> -	return ret;
> +	if (len) {
> +		errno = EIO;
> +		return -1;
> +	}
> +	return 0;
>  }
> -- 
> 2.35.1

We already seem to cover the case (which should not be possible, per
the getrandom syscall contract) where getrandom returns fewer than 256
bytes, and the one where it fails with EINTR due to interruption by a
signal before the urandom pool was initialized. It looks to me like
you're positing a case where the syscall returns 0, but that's not
something that can happen. A failure to read any bytes when 0 wasn't
the length requested is always an error of some sort (EAGAIN, EINTR,
etc.) not a short read of length zero.

As written the patch also has at least one bug, checking errno when a
non-error value (0) was returned.

I do want to enhance this function not to be able to fail at all
except on invalid input (the existing EIO case). This requires use of
the legacy SYS_sysctl syscall which is deprecated and removed from
modern kernels, but all the kernel versions (AIUI back to way before
supportable versions) lacking SYS_getrandom had it.

Rich

  reply	other threads:[~2022-04-09 13:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-09  0:10 Jason A. Donenfeld
2022-04-09 13:18 ` Rich Felker [this message]
2022-04-09 22:50   ` Jason A. Donenfeld
2022-04-09 22:58     ` [musl] [PATCH v2] " Jason A. Donenfeld
2022-04-09 22:58       ` [musl] [PATCH v3] " Jason A. Donenfeld
2022-04-10 15:30       ` [musl] [PATCH v2] " Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220409131816.GJ7074@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=Jason@zx2c4.com \
    --cc=musl@lists.openwall.com \
    --subject='Re: [musl] [PATCH] getentropy: fail if buffer not completely filled' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).