[2/2] linux: use __getrandom_nocancel in getentropy

  It must use the VDSO implementation if available instead of a raw
syscall.

Signed-off-by: Cristian Rodríguez <cristian@rodriguez.im>
---
 sysdeps/unix/sysv/linux/getentropy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

On Sat, Feb 8, 2025 at 2:05 PM Cristian Rodríguez <cristian@rodriguez.im> wrote:
>
> It must use the VDSO implementation if available instead of a raw
> syscall.
>
> Signed-off-by: Cristian Rodríguez <cristian@rodriguez.im>


I looked at the other implementations around and needs this needs to

- be made POSIX.2024 compatible, where the only possible errors are
EINVAL and maybe ENOSYS.
- The different BSDs implementations where this function originally
comes from all abort on irrecoverable errors, either by explicit
raise(SIGKILL) or because getentropy is just a wrapper against
arc4random_buf

I got to the poin that I think it has just ot be implemented like this now:

int
getentropy (void *buffer, size_t length)
{
  /* The interface is documented to return EINVAL for buffer lengths
     longer than 256 bytes.  */
  if (length > GETENTROPY_MAX)
    {
      __set_errno (EINVAL);
      return -1;
    }
    __arc4random_buf(buffer, length);
    return 0;
}

Will that fly here ?
  

On 08/02/25 14:05, Cristian Rodríguez wrote:
> It must use the VDSO implementation if available instead of a raw
> syscall.
> 
> Signed-off-by: Cristian Rodríguez <cristian@rodriguez.im>

I think it make sense now that we have an non-cancellable internal symbol,
and the vDSO should give us same guarantee as the kernel.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/getentropy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/getentropy.c b/sysdeps/unix/sysv/linux/getentropy.c
> index a62c9fb099..2134a32ae4 100644
> --- a/sysdeps/unix/sysv/linux/getentropy.c
> +++ b/sysdeps/unix/sysv/linux/getentropy.c
> @@ -21,7 +21,7 @@
>  #include <errno.h>
>  #include <unistd.h>
>  #include <sysdep.h>
> -
> +#include <not-cancel.h>
>  /* Write LENGTH bytes of randomness starting at BUFFER.  Return 0 on
>     success and -1 on failure.  */
>  int
> @@ -42,7 +42,7 @@ getentropy (void *buffer, size_t length)
>    while (buffer < end)
>      {
>        /* NB: No cancellation point.  */
> -      ssize_t bytes = INLINE_SYSCALL_CALL (getrandom, buffer, end - buffer, 0);
> +      ssize_t bytes = __getrandom_nocancel (buffer, end - buffer, 0);
>        if (bytes < 0)
>          {
>            if (errno == EINTR)
  

On Feb 08 2025, Cristian Rodríguez wrote:

> diff --git a/sysdeps/unix/sysv/linux/getentropy.c b/sysdeps/unix/sysv/linux/getentropy.c
> index a62c9fb099..2134a32ae4 100644
> --- a/sysdeps/unix/sysv/linux/getentropy.c
> +++ b/sysdeps/unix/sysv/linux/getentropy.c
> @@ -21,7 +21,7 @@
>  #include <errno.h>
>  #include <unistd.h>
>  #include <sysdep.h>
> -
> +#include <not-cancel.h>
>  /* Write LENGTH bytes of randomness starting at BUFFER.  Return 0 on
>     success and -1 on failure.  */
>  int

Please keep the empty line.
  

On 09/02/25 00:14, Cristian Rodríguez wrote:
> On Sat, Feb 8, 2025 at 2:05 PM Cristian Rodríguez <cristian@rodriguez.im> wrote:
>>
>> It must use the VDSO implementation if available instead of a raw
>> syscall.
>>
>> Signed-off-by: Cristian Rodríguez <cristian@rodriguez.im>
> 
> 
> I looked at the other implementations around and needs this needs to
> 
> - be made POSIX.2024 compatible, where the only possible errors are
> EINVAL and maybe ENOSYS.
> - The different BSDs implementations where this function originally
> comes from all abort on irrecoverable errors, either by explicit
> raise(SIGKILL) or because getentropy is just a wrapper against
> arc4random_buf
> 
> I got to the poin that I think it has just ot be implemented like this now:
> 
> int
> getentropy (void *buffer, size_t length)
> {
>   /* The interface is documented to return EINVAL for buffer lengths
>      longer than 256 bytes.  */
>   if (length > GETENTROPY_MAX)
>     {
>       __set_errno (EINVAL);
>       return -1;
>     }
>     __arc4random_buf(buffer, length);
>     return 0;
> }
> 
> Will that fly here ?

It would change getentropy semantics slight, since arc4random will try
to open/read/close the /dev/urandom if getrandom is not available (due
syscall filtering or old kernels).  In this case with current code, 
getentropy() will fail with ENOSYS; while for arc4random() it will abort() 
if /dev/urandom is not available or if it can't be open (ENFILE, etc.). 

I am not sure if this is problem, but I think it would be safe to avoid
changing for now.

Now that we have __getrandom_nocancel, we can just do something like:

static void
getentropy_fatal (void)
{
  __libc_fatal ("Fatal glibc error: cannot get entropy for getentropy\n");
}

/* Write LENGTH bytes of randomness starting at BUFFER.  Return 0 on
   success and -1 on failure.  */
int
getentropy (void *buffer, size_t length)
{
  if (length > 256)
    {
      __set_errno (EINVAL);
      return -1;
    }

  /* Try to fill the buffer completely.  Even with the 256 byte limit
     above, we might still receive an EINTR error (when blocking
     during boot).  */
  void *end = buffer + length;
  while (buffer < end)
    {
      /* NB: No cancellation point.  */
      ssize_t bytes = __getrandom_nocancel (buffer, end - buffer, 0);
      if (bytes < 0)
        {
          switch (errno)
            {
            case EINTR:
              continue;
            case ENOSYS:
              return -1;
            default:
              getentropy_fatal ();
            }
        }
      else if (bytes == 0)
        /* No more bytes available.  This should not happen under normal
           circumstances.  */
        getentropy_fatal ();

      /* Try again in case of a short read.  */
      buffer += bytes;
    }
  return 0;
}

And remove both sysdeps/mach/hurd/getentropy.c and sysdeps/unix/sysv/linux/getentropy.c
  

On Wed, Feb 12, 2025 at 9:57 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:

>
> And remove both sysdeps/mach/hurd/getentropy.c and sysdeps/unix/sysv/linux/getentropy.c


OK, Im gonna post an improved version later.