[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.
  

On Wed, Feb 12, 2025 at 6:40 PM Cristian Rodríguez
<cristian@rodriguez.im> wrote:
>
> 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.

Went back to this again.. what to do with GETENTROPY_MAX now.. ? Posix says

{GETENTROPY_MAX}The maximum value of the length argument in calls to
the getentropy() function.
Minimum Acceptable Value: 256

However since:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e3c1c4fd9e6d14059ed93ebfe15e1c57793b1a05

 The kernel special case is PAGE_SIZE not 256..and the special 256
never worked in the first place..

So at least for the linux parts

- GETENTROPY_MAX == 256 ? and forget such a limit will not be enforced?
-  GETENTROPY_MAX == getpagesize() ? because PAGE_SIZE is not fixed in
userspace?
- Do not define it all.. make the wrapper return EINVAL if bigger than
a pagesize to match the current kernel special case.

looking forward to your inputs.
  

On 22/11/25 11:35, Cristian Rodríguez wrote:
> On Wed, Feb 12, 2025 at 6:40 PM Cristian Rodríguez
> <cristian@rodriguez.im> wrote:
>>
>> 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.
> 
> Went back to this again.. what to do with GETENTROPY_MAX now.. ? Posix says
> 
> {GETENTROPY_MAX}The maximum value of the length argument in calls to
> the getentropy() function.
> Minimum Acceptable Value: 256
> 
> However since:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e3c1c4fd9e6d14059ed93ebfe15e1c57793b1a05
> 
>  The kernel special case is PAGE_SIZE not 256..and the special 256
> never worked in the first place..
> 
> So at least for the linux parts
> 
> - GETENTROPY_MAX == 256 ? and forget such a limit will not be enforced?
> -  GETENTROPY_MAX == getpagesize() ? because PAGE_SIZE is not fixed in
> userspace?
> - Do not define it all.. make the wrapper return EINVAL if bigger than
> a pagesize to match the current kernel special case.
> 
> looking forward to your inputs.

Can we assume 'GETENTROPY_MAX == getpagesize()' works for older kernels than 5.18?

We did changed PTHREAD_STACK_MIN to be dynamic based on sysconf(_SC_THREAD_STACK_MIN)
(5d98a7dae955bafa6740c26eaba9c86060ae0344), so I think we will need something
similar to make GETENTROPY_MAX based on the system page size.

This is really a lot of complexity, and I am not sure it users will really
require GETENTROPY_MAX > 256 for most usercases.
  

On Tue, Nov 25, 2025 at 1:12 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 22/11/25 11:35, Cristian Rodríguez wrote:
> > On Wed, Feb 12, 2025 at 6:40 PM Cristian Rodríguez
> > <cristian@rodriguez.im> wrote:
> >>
> >> 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.
> >
> > Went back to this again.. what to do with GETENTROPY_MAX now.. ? Posix says
> >
> > {GETENTROPY_MAX}The maximum value of the length argument in calls to
> > the getentropy() function.
> > Minimum Acceptable Value: 256
> >
> > However since:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e3c1c4fd9e6d14059ed93ebfe15e1c57793b1a05
> >
> >  The kernel special case is PAGE_SIZE not 256..and the special 256
> > never worked in the first place..
> >
> > So at least for the linux parts
> >
> > - GETENTROPY_MAX == 256 ? and forget such a limit will not be enforced?
> > -  GETENTROPY_MAX == getpagesize() ? because PAGE_SIZE is not fixed in
> > userspace?
> > - Do not define it all.. make the wrapper return EINVAL if bigger than
> > a pagesize to match the current kernel special case.
> >
> > looking forward to your inputs.
>
> Can we assume 'GETENTROPY_MAX == getpagesize()' works for older kernels than 5.18?

as far as I understood, the 256 special case has never worked. the
kernel never reacted to signals until all bytes were read.
so it should work.

> This is really a lot of complexity, and I am not sure it users will really
> require GETENTROPY_MAX > 256 for most usercases.

Yeah.. What about using the smallest possible page size as a limit
then? I think adding further complexity in this case aint worth it.
  

On 25/11/25 13:50, Cristian Rodríguez wrote:
> On Tue, Nov 25, 2025 at 1:12 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 22/11/25 11:35, Cristian Rodríguez wrote:
>>> On Wed, Feb 12, 2025 at 6:40 PM Cristian Rodríguez
>>> <cristian@rodriguez.im> wrote:
>>>>
>>>> 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.
>>>
>>> Went back to this again.. what to do with GETENTROPY_MAX now.. ? Posix says
>>>
>>> {GETENTROPY_MAX}The maximum value of the length argument in calls to
>>> the getentropy() function.
>>> Minimum Acceptable Value: 256
>>>
>>> However since:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e3c1c4fd9e6d14059ed93ebfe15e1c57793b1a05
>>>
>>>  The kernel special case is PAGE_SIZE not 256..and the special 256
>>> never worked in the first place..
>>>
>>> So at least for the linux parts
>>>
>>> - GETENTROPY_MAX == 256 ? and forget such a limit will not be enforced?
>>> -  GETENTROPY_MAX == getpagesize() ? because PAGE_SIZE is not fixed in
>>> userspace?
>>> - Do not define it all.. make the wrapper return EINVAL if bigger than
>>> a pagesize to match the current kernel special case.
>>>
>>> looking forward to your inputs.
>>
>> Can we assume 'GETENTROPY_MAX == getpagesize()' works for older kernels than 5.18?
> 
> as far as I understood, the 256 special case has never worked. the
> kernel never reacted to signals until all bytes were read.
> so it should work.

Right, it might be good to check this empirically on some older kernel.

> 
>> This is really a lot of complexity, and I am not sure it users will really
>> require GETENTROPY_MAX > 256 for most usercases.
> 
> Yeah.. What about using the smallest possible page size as a limit
> then? I think adding further complexity in this case aint worth it.

I think it seems reasonable, assuming it does work as intended in all kernel
version. We don't have an internal minimal page size definition, but we can
assume 4096 (and I don't think it would be worth to make it arch-dependent).