[v2] malloc: Fix clobbered errno when getrandom failed [BZ #29624]

  Save and restore errno when getrandom failed. On failure it will result
in errno clobbered at statically linked program startup. This scenario
is possible if getrandom is called by tcache_key_initialize when crng is
not ready thus EAGAIN is returned.

Fixes bug 29624.

Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
---
 malloc/malloc.c | 2 ++
 1 file changed, 2 insertions(+)
  

On 29/09/22 08:13, Yu Chien Peter Lin wrote:
> Save and restore errno when getrandom failed. On failure it will result
> in errno clobbered at statically linked program startup. This scenario
> is possible if getrandom is called by tcache_key_initialize when crng is
> not ready thus EAGAIN is returned.
> 
> Fixes bug 29624.
> 
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> ---
>  malloc/malloc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 953183e956..823d454c99 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3133,9 +3133,11 @@ static uintptr_t tcache_key;
>  static void
>  tcache_key_initialize (void)
>  {
> +  int saved_errno = errno;
>    if (__getrandom_nocancel (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK)
>        != sizeof (tcache_key))
>      {
> +      __set_errno(saved_errno);
>        tcache_key = random_bits ();
>  #if __WORDSIZE == 64
>        tcache_key = (tcache_key << 32) | random_bits ();

I think it would be better to just use INTERNAL_SYSCALL now that we have all
architecture to return a negative value in case of error:

diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c
index e417ef624d..20886e0445 100644
--- a/stdlib/arc4random.c
+++ b/stdlib/arc4random.c
@@ -34,7 +34,7 @@ void
 __arc4random_buf (void *p, size_t n)
 {
   static int seen_initialized;
-  size_t l;
+  int l;
   int fd;

   if (n == 0)
@@ -51,7 +51,7 @@ __arc4random_buf (void *p, size_t n)
          n -= l;
          continue; /* Interrupted by a signal; keep going.  */
        }
-      else if (l < 0 && errno == ENOSYS)
+      else if (l < 0 && l == -ENOSYS)
        break; /* No syscall, so fallback to /dev/urandom.  */
       arc4random_getrandom_failure ();
     }
diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
index a263d294b1..00ab75a405 100644
--- a/sysdeps/unix/sysv/linux/not-cancel.h
+++ b/sysdeps/unix/sysv/linux/not-cancel.h
@@ -71,7 +71,7 @@ __writev_nocancel_nostatus (int fd, const struct iovec *iov, int iovcnt)
 static inline int
 __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
 {
-  return INLINE_SYSCALL_CALL (getrandom, buf, buflen, flags);
+  return INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags);
 }

 static inline int
  

* Adhemerval Zanella Netto:

> diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
> index a263d294b1..00ab75a405 100644
> --- a/sysdeps/unix/sysv/linux/not-cancel.h
> +++ b/sysdeps/unix/sysv/linux/not-cancel.h
> @@ -71,7 +71,7 @@ __writev_nocancel_nostatus (int fd, const struct iovec *iov, int iovcnt)
>  static inline int
>  __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
>  {
> -  return INLINE_SYSCALL_CALL (getrandom, buf, buflen, flags);
> +  return INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags);
>  }
>
>  static inline int

Doesn't this need a matching change to sysdeps/mach/hurd/not-cancel.h?
  

On 29/09/22 10:31, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
>> index a263d294b1..00ab75a405 100644
>> --- a/sysdeps/unix/sysv/linux/not-cancel.h
>> +++ b/sysdeps/unix/sysv/linux/not-cancel.h
>> @@ -71,7 +71,7 @@ __writev_nocancel_nostatus (int fd, const struct iovec *iov, int iovcnt)
>>  static inline int
>>  __getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
>>  {
>> -  return INLINE_SYSCALL_CALL (getrandom, buf, buflen, flags);
>> +  return INTERNAL_SYSCALL_CALL (getrandom, buf, buflen, flags);
>>  }
>>
>>  static inline int
> 
> Doesn't this need a matching change to sysdeps/mach/hurd/not-cancel.h?

Yeah, it will to make __arc4random_buf use the /dev/random fallback.