| Message ID | 20250321193444.2339539-1-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | stdlib: Consolidate getentropy and adapt to POSIX 2024 semantics | expand |
On Fri, Mar 21, 2025 at 4:34 PM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > POSIX.1-2024 added getentropy with some slight different semantics: > > 1. Buffer larger than 256 (GETENTROPY_MAX) should return EINVAL > insted of EIO. > > 2. There is no defined error is no entropy could be obtained. > It means that the function should not fail. > > Although both requirements would require new symbol version, the > implementation was initially provides for compatibility with BSD, > and both FreeBSD and OpenBSD already adapt their implementation > to POSIX 2024. So I think it is not worth a compatibily symbol. > > This patch does not add GETENTROPY_MAX on limits.h, since glibc > still does not have a preprocessor handling for POSIX 2024. > > The consolidation uses __getrandom_nocancel, which might uses > the vDSO implementation if supported. > > Checked on x86_64-linux-gnu. > (My comments here only apply to the linux version) Are we sure all this error handling is actually necessary or correct? I know the manual pages are outdated and do not reflect the state of things post Jason massive RNG rework.. But at least the previous implementation promised on reads <= 256 not to short read, always return at least that amount of bytes, so in theory returning 0 or EINTR is/was not possible with 0 flags. in fact the code nowadays appears to react to signals every PAGE_SIZE read.. The BSDs where this function originates from, promise to always return up to 256 bytes if the buffer is valid..
* Cristian Rodríguez: > Are we sure all this error handling is actually necessary or correct? > I know the manual pages are outdated and do not reflect the state of > things post Jason massive RNG rework.. > But at least the previous implementation promised on reads <= 256 not > to short read, always return at least that amount of bytes, so in > theory returning 0 or EINTR is/was not possible with 0 flags. Most Linux systems do not use the upstream random driver, but modified versions of it. Thanks, Florian
On 24/03/25 07:19, Florian Weimer wrote: > * Cristian Rodríguez: > >> Are we sure all this error handling is actually necessary or correct? >> I know the manual pages are outdated and do not reflect the state of >> things post Jason massive RNG rework.. >> But at least the previous implementation promised on reads <= 256 not >> to short read, always return at least that amount of bytes, so in >> theory returning 0 or EINTR is/was not possible with 0 flags. > > Most Linux systems do not use the upstream random driver, but modified > versions of it. Also we still support older kernels that might not have the /dev/random fixes done on Linux 4.18 to always provides 256 bytes or entropy (/proc/sys/kernel/random/entropy_avail).
On 3/21/25 3:34 PM, Adhemerval Zanella wrote: > POSIX.1-2024 added getentropy with some slight different semantics: Looking forward to a v2! :-) > 1. Buffer larger than 256 (GETENTROPY_MAX) should return EINVAL > insted of EIO. My opinion here is that we should version the symbol for this change and attempt a wrapper that can return the original EINVAL and then call a central function that implements the new functionality. > > 2. There is no defined error is no entropy could be obtained. > It means that the function should not fail. s/error is/error if/g If the function can't fail then we should abort if we can't get entropy, and this behavioural change is not something we should version. Running an older application on a newer glibc should bring about the security benefits of the new glibc. Users should work to inject enough entropy into their system that they do not run out of entropy. This includes running things like haveged to inject hidden entropy from the CPU into the running kernel. > Although both requirements would require new symbol version, the > implementation was initially provides for compatibility with BSD, > and both FreeBSD and OpenBSD already adapt their implementation > to POSIX 2024. So I think it is not worth a compatibily symbol. The larger number of applications for Linux means that we see a wider compatibility impact, so using the *BSD change as a justification to not version the symbol does not seem correct to me. I would version the symbol. And that way if we do have serious issues with the abort() due to loss of entropy we can make that change alter. > This patch does not add GETENTROPY_MAX on limits.h, since glibc > still does not have a preprocessor handling for POSIX 2024. OK. > > The consolidation uses __getrandom_nocancel, which might uses > the vDSO implementation if supported. OK. > Checked on x86_64-linux-gnu. > --- > manual/crypt.texi | 2 +- > stdlib/getentropy.c | 49 ++++++++++++++++++--- > stdlib/tst-getrandom.c | 2 +- > sysdeps/mach/hurd/getentropy.c | 59 ------------------------- > sysdeps/unix/sysv/linux/getentropy.c | 65 ---------------------------- > 5 files changed, 46 insertions(+), 131 deletions(-) > delete mode 100644 sysdeps/mach/hurd/getentropy.c > delete mode 100644 sysdeps/unix/sysv/linux/getentropy.c > > diff --git a/manual/crypt.texi b/manual/crypt.texi > index 4882ee34e5..ce68f2853a 100644 > --- a/manual/crypt.texi > +++ b/manual/crypt.texi > @@ -73,7 +73,7 @@ used by this function was added to the Linux kernel in version 3.17.) > The combination of @var{buffer} and @var{length} arguments specifies > an invalid memory range. > > -@item EIO > +@item EINVAL OK. > @var{length} is larger than 256, or the kernel entropy pool has > suffered a catastrophic failure. > @end table > diff --git a/stdlib/getentropy.c b/stdlib/getentropy.c > index 5149fbdde0..aa42341291 100644 > --- a/stdlib/getentropy.c > +++ b/stdlib/getentropy.c > @@ -16,16 +16,55 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > -#include <sys/random.h> > +#include <stdio.h> > #include <errno.h> > +#include <not-cancel.h> > +#include <sys/random.h> > + > +static void > +getentropy_fatal (void) > +{ > + __libc_fatal ("Fatal glibc error: cannot get entropy for getentropy\n"); > +} OK. Abort with a reasonable error message. > > /* Write LENGTH bytes of randomness starting at BUFFER. Return 0 on > success and -1 on failure. */ > int > getentropy (void *buffer, size_t length) > { > - __set_errno (ENOSYS); > - return -1; > -} > + if (length > 256) > + { > + __set_errno (EINVAL); > + return -1; > + } OK. > > -stub_warning (getentropy) > + /* 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); OK. > + if (bytes < 0) > + { > + switch (errno) > + { > + case EINTR: > + continue; > + case ENOSYS: > + return -1; > + default: > + getentropy_fatal (); OK. > + } > + } > + else if (bytes == 0) > + /* No more bytes available. This should not happen under normal > + circumstances. */ > + getentropy_fatal (); OK. > + > + /* Try again in case of a short read. */ > + buffer += bytes; OK. > + } > + return 0; > +} > diff --git a/stdlib/tst-getrandom.c b/stdlib/tst-getrandom.c > index 3b2153376b..9fff7a8309 100644 > --- a/stdlib/tst-getrandom.c > +++ b/stdlib/tst-getrandom.c > @@ -211,7 +211,7 @@ test_getentropy (void) > errors = true; > return; > } > - if (errno != EIO) > + if (errno != EINVAL) Please add a new small test for the versioned symbol that uses symbol_version_reference() and checks the old symbol returns the correct error code. > { > printf ("error: getentropy wrong error for 257 byte buffer: %m\n"); > errors = true; > diff --git a/sysdeps/mach/hurd/getentropy.c b/sysdeps/mach/hurd/getentropy.c > deleted file mode 100644 > index 6ad8acc773..0000000000 > --- a/sysdeps/mach/hurd/getentropy.c > +++ /dev/null > @@ -1,59 +0,0 @@ > -/* Implementation of getentropy based on getrandom. > - Copyright (C) 2016-2025 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <https://www.gnu.org/licenses/>. */ > - > -#include <sys/random.h> > -#include <assert.h> > -#include <errno.h> > -#include <unistd.h> > -#include <hurd.h> > - > -/* Write LENGTH bytes of randomness starting at BUFFER. Return 0 on > - success and -1 on failure. */ > -int > -getentropy (void *buffer, size_t length) > -{ > - /* The interface is documented to return EIO for buffer lengths > - longer than 256 bytes. */ > - if (length > 256) > - return __hurd_fail (EIO); > - > - /* 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 (buffer, end - buffer, 0); > - if (bytes < 0) > - { > - if (errno == EINTR) > - /* Try again if interrupted by a signal. */ > - continue; > - else > - return -1; > - } > - if (bytes == 0) > - /* No more bytes available. This should not happen under > - normal circumstances. */ > - return __hurd_fail (EIO); > - /* Try again in case of a short read. */ > - buffer += bytes; > - } > - return 0; > -} > diff --git a/sysdeps/unix/sysv/linux/getentropy.c b/sysdeps/unix/sysv/linux/getentropy.c > deleted file mode 100644 > index a62c9fb099..0000000000 > --- a/sysdeps/unix/sysv/linux/getentropy.c > +++ /dev/null > @@ -1,65 +0,0 @@ > -/* Implementation of getentropy based on the getrandom system call. > - Copyright (C) 2016-2025 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <https://www.gnu.org/licenses/>. */ > - > -#include <sys/random.h> > -#include <assert.h> > -#include <errno.h> > -#include <unistd.h> > -#include <sysdep.h> > - > -/* Write LENGTH bytes of randomness starting at BUFFER. Return 0 on > - success and -1 on failure. */ > -int > -getentropy (void *buffer, size_t length) > -{ > - /* The interface is documented to return EIO for buffer lengths > - longer than 256 bytes. */ > - if (length > 256) > - { > - __set_errno (EIO); > - 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 = INLINE_SYSCALL_CALL (getrandom, buffer, end - buffer, 0); > - if (bytes < 0) > - { > - if (errno == EINTR) > - /* Try again if interrupted by a signal. */ > - continue; > - else > - return -1; > - } > - if (bytes == 0) > - { > - /* No more bytes available. This should not happen under > - normal circumstances. */ > - __set_errno (EIO); > - return -1; > - } > - /* Try again in case of a short read. */ > - buffer += bytes; > - } > - return 0; > -}
diff --git a/manual/crypt.texi b/manual/crypt.texi index 4882ee34e5..ce68f2853a 100644 --- a/manual/crypt.texi +++ b/manual/crypt.texi @@ -73,7 +73,7 @@ used by this function was added to the Linux kernel in version 3.17.) The combination of @var{buffer} and @var{length} arguments specifies an invalid memory range. -@item EIO +@item EINVAL @var{length} is larger than 256, or the kernel entropy pool has suffered a catastrophic failure. @end table diff --git a/stdlib/getentropy.c b/stdlib/getentropy.c index 5149fbdde0..aa42341291 100644 --- a/stdlib/getentropy.c +++ b/stdlib/getentropy.c @@ -16,16 +16,55 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -#include <sys/random.h> +#include <stdio.h> #include <errno.h> +#include <not-cancel.h> +#include <sys/random.h> + +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) { - __set_errno (ENOSYS); - return -1; -} + if (length > 256) + { + __set_errno (EINVAL); + return -1; + } -stub_warning (getentropy) + /* 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; +} diff --git a/stdlib/tst-getrandom.c b/stdlib/tst-getrandom.c index 3b2153376b..9fff7a8309 100644 --- a/stdlib/tst-getrandom.c +++ b/stdlib/tst-getrandom.c @@ -211,7 +211,7 @@ test_getentropy (void) errors = true; return; } - if (errno != EIO) + if (errno != EINVAL) { printf ("error: getentropy wrong error for 257 byte buffer: %m\n"); errors = true; diff --git a/sysdeps/mach/hurd/getentropy.c b/sysdeps/mach/hurd/getentropy.c deleted file mode 100644 index 6ad8acc773..0000000000 --- a/sysdeps/mach/hurd/getentropy.c +++ /dev/null @@ -1,59 +0,0 @@ -/* Implementation of getentropy based on getrandom. - Copyright (C) 2016-2025 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#include <sys/random.h> -#include <assert.h> -#include <errno.h> -#include <unistd.h> -#include <hurd.h> - -/* Write LENGTH bytes of randomness starting at BUFFER. Return 0 on - success and -1 on failure. */ -int -getentropy (void *buffer, size_t length) -{ - /* The interface is documented to return EIO for buffer lengths - longer than 256 bytes. */ - if (length > 256) - return __hurd_fail (EIO); - - /* 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 (buffer, end - buffer, 0); - if (bytes < 0) - { - if (errno == EINTR) - /* Try again if interrupted by a signal. */ - continue; - else - return -1; - } - if (bytes == 0) - /* No more bytes available. This should not happen under - normal circumstances. */ - return __hurd_fail (EIO); - /* Try again in case of a short read. */ - buffer += bytes; - } - return 0; -} diff --git a/sysdeps/unix/sysv/linux/getentropy.c b/sysdeps/unix/sysv/linux/getentropy.c deleted file mode 100644 index a62c9fb099..0000000000 --- a/sysdeps/unix/sysv/linux/getentropy.c +++ /dev/null @@ -1,65 +0,0 @@ -/* Implementation of getentropy based on the getrandom system call. - Copyright (C) 2016-2025 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#include <sys/random.h> -#include <assert.h> -#include <errno.h> -#include <unistd.h> -#include <sysdep.h> - -/* Write LENGTH bytes of randomness starting at BUFFER. Return 0 on - success and -1 on failure. */ -int -getentropy (void *buffer, size_t length) -{ - /* The interface is documented to return EIO for buffer lengths - longer than 256 bytes. */ - if (length > 256) - { - __set_errno (EIO); - 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 = INLINE_SYSCALL_CALL (getrandom, buffer, end - buffer, 0); - if (bytes < 0) - { - if (errno == EINTR) - /* Try again if interrupted by a signal. */ - continue; - else - return -1; - } - if (bytes == 0) - { - /* No more bytes available. This should not happen under - normal circumstances. */ - __set_errno (EIO); - return -1; - } - /* Try again in case of a short read. */ - buffer += bytes; - } - return 0; -}
POSIX.1-2024 added getentropy with some slight different semantics: 1. Buffer larger than 256 (GETENTROPY_MAX) should return EINVAL insted of EIO. 2. There is no defined error is no entropy could be obtained. It means that the function should not fail. Although both requirements would require new symbol version, the implementation was initially provides for compatibility with BSD, and both FreeBSD and OpenBSD already adapt their implementation to POSIX 2024. So I think it is not worth a compatibily symbol. This patch does not add GETENTROPY_MAX on limits.h, since glibc still does not have a preprocessor handling for POSIX 2024. The consolidation uses __getrandom_nocancel, which might uses the vDSO implementation if supported. Checked on x86_64-linux-gnu. --- manual/crypt.texi | 2 +- stdlib/getentropy.c | 49 ++++++++++++++++++--- stdlib/tst-getrandom.c | 2 +- sysdeps/mach/hurd/getentropy.c | 59 ------------------------- sysdeps/unix/sysv/linux/getentropy.c | 65 ---------------------------- 5 files changed, 46 insertions(+), 131 deletions(-) delete mode 100644 sysdeps/mach/hurd/getentropy.c delete mode 100644 sysdeps/unix/sysv/linux/getentropy.c