Message ID | 20240104134243.1581383-1-xry111@xry111.site |
---|---|
State | New |
Headers | show |
Series | [v2] Make __getrandom_nocancel set errno and add a _nostatus version | expand |
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.
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); }
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(-)