[v2] Make __getrandom_nocancel set errno and add a _nostatus version
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
Commit Message
The __getrandom_nocancel function returns errors as negative values
instead of errno. This is inconsistent with other _nocancel functions
and it breaks "TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0))" in
__arc4random_buf. Use INLINE_SYSCALL_CALL instead of
INTERNAL_SYSCALL_CALL to fix this issue.
But __getrandom_nocancel has been avoiding from touching errno for a
reason, see BZ 29624. So add a __getrandom_nocancel_nostatus function
and use it in tcache_key_initialize.
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
Superseds the incorrect patch [v1].
[v1]:https://sourceware.org/pipermail/libc-alpha/2024-January/153727.html.
malloc/malloc.c | 4 +++-
sysdeps/generic/not-cancel.h | 2 ++
sysdeps/mach/hurd/not-cancel.h | 7 ++++++-
sysdeps/unix/sysv/linux/not-cancel.h | 8 ++++++++
4 files changed, 19 insertions(+), 2 deletions(-)
Comments
Ping. Sorry for an abnormal time period but now we are close to hard
freeze, and the next review meeting won't happen before the hard freeze.
On Thu, 2024-01-04 at 21:41 +0800, Xi Ruoyao wrote:
> The __getrandom_nocancel function returns errors as negative values
> instead of errno. This is inconsistent with other _nocancel functions
> and it breaks "TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0))" in
> __arc4random_buf. Use INLINE_SYSCALL_CALL instead of
> INTERNAL_SYSCALL_CALL to fix this issue.
>
> But __getrandom_nocancel has been avoiding from touching errno for a
> reason, see BZ 29624. So add a __getrandom_nocancel_nostatus function
> and use it in tcache_key_initialize.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>
> Superseds the incorrect patch [v1].
>
> [v1]:https://sourceware.org/pipermail/libc-alpha/2024-January/153727.html.
>
> malloc/malloc.c | 4 +++-
> sysdeps/generic/not-cancel.h | 2 ++
> sysdeps/mach/hurd/not-cancel.h | 7 ++++++-
> sysdeps/unix/sysv/linux/not-cancel.h | 8 ++++++++
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 198e78a162..bcb6e5b83c 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3139,7 +3139,9 @@ static uintptr_t tcache_key;
> static void
> tcache_key_initialize (void)
> {
> - if (__getrandom_nocancel (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK)
> + /* We need to use the _nostatus version here, see BZ 29624. */
> + if (__getrandom_nocancel_nostatus (&tcache_key, sizeof(tcache_key),
> + GRND_NONBLOCK)
> != sizeof (tcache_key))
> {
> tcache_key = random_bits ();
> diff --git a/sysdeps/generic/not-cancel.h b/sysdeps/generic/not-cancel.h
> index d9a6cba443..2dd1064600 100644
> --- a/sysdeps/generic/not-cancel.h
> +++ b/sysdeps/generic/not-cancel.h
> @@ -51,6 +51,8 @@
> __fcntl64 (fd, cmd, __VA_ARGS__)
> #define __getrandom_nocancel(buf, size, flags) \
> __getrandom (buf, size, flags)
> +#define __getrandom_nocancel_nostatus(buf, size, flags) \
> + __getrandom (buf, size, flags)
> #define __poll_infinity_nocancel(fds, nfds) \
> __poll (fds, nfds, -1)
>
> diff --git a/sysdeps/mach/hurd/not-cancel.h b/sysdeps/mach/hurd/not-cancel.h
> index 411f5796ae..69fb3c00ef 100644
> --- a/sysdeps/mach/hurd/not-cancel.h
> +++ b/sysdeps/mach/hurd/not-cancel.h
> @@ -76,8 +76,10 @@ __typeof (__fcntl) __fcntl_nocancel;
> #define __fcntl64_nocancel(...) \
> __fcntl_nocancel (__VA_ARGS__)
>
> +/* Non cancellable getrandom syscall that does not also set errno in case of
> + failure. */
> static inline ssize_t
> -__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> +__getrandom_nocancel_nostatus (void *buf, size_t buflen, unsigned int flags)
> {
> int save_errno = errno;
> ssize_t r = __getrandom (buf, buflen, flags);
> @@ -86,6 +88,9 @@ __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> return r;
> }
>
> +#define __getrandom_nocancel(buf, size, flags) \
> + __getrandom (buf, size, flags)
> +
> #define __poll_infinity_nocancel(fds, nfds) \
> __poll (fds, nfds, -1)
>
> diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
> index 50483d9e74..2a7585b73f 100644
> --- a/sysdeps/unix/sysv/linux/not-cancel.h
> +++ b/sysdeps/unix/sysv/linux/not-cancel.h
> @@ -85,6 +85,14 @@ __writev_nocancel_nostatus (int fd, const struct iovec *iov, int iovcnt)
>
> static inline ssize_t
> __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> +{
> + return INLINE_SYSCALL_CALL (getrandom, buf, buflen, flags);
> +}
> +
> +/* Non cancellable getrandom syscall that does not also set errno in case of
> + failure. */
> +static inline ssize_t
> +__getrandom_nocancel_nostatus (void *buf, size_t buflen, unsigned int flags)
> {
> return INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags);
> }
On 04/01/24 10:41, Xi Ruoyao wrote:
> The __getrandom_nocancel function returns errors as negative values
> instead of errno. This is inconsistent with other _nocancel functions
> and it breaks "TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0))" in
> __arc4random_buf. Use INLINE_SYSCALL_CALL instead of
> INTERNAL_SYSCALL_CALL to fix this issue.
>
> But __getrandom_nocancel has been avoiding from touching errno for a
> reason, see BZ 29624. So add a __getrandom_nocancel_nostatus function
> and use it in tcache_key_initialize.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
>
> Superseds the incorrect patch [v1].
>
> [v1]:https://sourceware.org/pipermail/libc-alpha/2024-January/153727.html.
>
> malloc/malloc.c | 4 +++-
> sysdeps/generic/not-cancel.h | 2 ++
> sysdeps/mach/hurd/not-cancel.h | 7 ++++++-
> sysdeps/unix/sysv/linux/not-cancel.h | 8 ++++++++
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 198e78a162..bcb6e5b83c 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3139,7 +3139,9 @@ static uintptr_t tcache_key;
> static void
> tcache_key_initialize (void)
> {
> - if (__getrandom_nocancel (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK)
> + /* We need to use the _nostatus version here, see BZ 29624. */
> + if (__getrandom_nocancel_nostatus (&tcache_key, sizeof(tcache_key),
> + GRND_NONBLOCK)
> != sizeof (tcache_key))
> {
> tcache_key = random_bits ();
Ok.
> diff --git a/sysdeps/generic/not-cancel.h b/sysdeps/generic/not-cancel.h
> index d9a6cba443..2dd1064600 100644
> --- a/sysdeps/generic/not-cancel.h
> +++ b/sysdeps/generic/not-cancel.h
> @@ -51,6 +51,8 @@
> __fcntl64 (fd, cmd, __VA_ARGS__)
> #define __getrandom_nocancel(buf, size, flags) \
> __getrandom (buf, size, flags)
> +#define __getrandom_nocancel_nostatus(buf, size, flags) \
> + __getrandom (buf, size, flags)
> #define __poll_infinity_nocancel(fds, nfds) \
> __poll (fds, nfds, -1)
>
Ok.
> diff --git a/sysdeps/mach/hurd/not-cancel.h b/sysdeps/mach/hurd/not-cancel.h
> index 411f5796ae..69fb3c00ef 100644
> --- a/sysdeps/mach/hurd/not-cancel.h
> +++ b/sysdeps/mach/hurd/not-cancel.h
> @@ -76,8 +76,10 @@ __typeof (__fcntl) __fcntl_nocancel;
> #define __fcntl64_nocancel(...) \
> __fcntl_nocancel (__VA_ARGS__)
>
> +/* Non cancellable getrandom syscall that does not also set errno in case of
> + failure. */
> static inline ssize_t
> -__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> +__getrandom_nocancel_nostatus (void *buf, size_t buflen, unsigned int flags)
> {
> int save_errno = errno;
> ssize_t r = __getrandom (buf, buflen, flags);
> @@ -86,6 +88,9 @@ __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> return r;
> }
>
> +#define __getrandom_nocancel(buf, size, flags) \
> + __getrandom (buf, size, flags)
> +
> #define __poll_infinity_nocancel(fds, nfds) \
> __poll (fds, nfds, -1)
>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
> index 50483d9e74..2a7585b73f 100644
> --- a/sysdeps/unix/sysv/linux/not-cancel.h
> +++ b/sysdeps/unix/sysv/linux/not-cancel.h
> @@ -85,6 +85,14 @@ __writev_nocancel_nostatus (int fd, const struct iovec *iov, int iovcnt)
>
> static inline ssize_t
> __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> +{
> + return INLINE_SYSCALL_CALL (getrandom, buf, buflen, flags);
> +}
> +
> +/* Non cancellable getrandom syscall that does not also set errno in case of
> + failure. */
> +static inline ssize_t
> +__getrandom_nocancel_nostatus (void *buf, size_t buflen, unsigned int flags)
> {
> return INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags);
> }
Ok.
@@ -3139,7 +3139,9 @@ static uintptr_t tcache_key;
static void
tcache_key_initialize (void)
{
- if (__getrandom_nocancel (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK)
+ /* We need to use the _nostatus version here, see BZ 29624. */
+ if (__getrandom_nocancel_nostatus (&tcache_key, sizeof(tcache_key),
+ GRND_NONBLOCK)
!= sizeof (tcache_key))
{
tcache_key = random_bits ();
@@ -51,6 +51,8 @@
__fcntl64 (fd, cmd, __VA_ARGS__)
#define __getrandom_nocancel(buf, size, flags) \
__getrandom (buf, size, flags)
+#define __getrandom_nocancel_nostatus(buf, size, flags) \
+ __getrandom (buf, size, flags)
#define __poll_infinity_nocancel(fds, nfds) \
__poll (fds, nfds, -1)
@@ -76,8 +76,10 @@ __typeof (__fcntl) __fcntl_nocancel;
#define __fcntl64_nocancel(...) \
__fcntl_nocancel (__VA_ARGS__)
+/* Non cancellable getrandom syscall that does not also set errno in case of
+ failure. */
static inline ssize_t
-__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
+__getrandom_nocancel_nostatus (void *buf, size_t buflen, unsigned int flags)
{
int save_errno = errno;
ssize_t r = __getrandom (buf, buflen, flags);
@@ -86,6 +88,9 @@ __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
return r;
}
+#define __getrandom_nocancel(buf, size, flags) \
+ __getrandom (buf, size, flags)
+
#define __poll_infinity_nocancel(fds, nfds) \
__poll (fds, nfds, -1)
@@ -85,6 +85,14 @@ __writev_nocancel_nostatus (int fd, const struct iovec *iov, int iovcnt)
static inline ssize_t
__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
+{
+ return INLINE_SYSCALL_CALL (getrandom, buf, buflen, flags);
+}
+
+/* Non cancellable getrandom syscall that does not also set errno in case of
+ failure. */
+static inline ssize_t
+__getrandom_nocancel_nostatus (void *buf, size_t buflen, unsigned int flags)
{
return INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags);
}