public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] malloc: Fix clobbered errno when getrandom failed [BZ #29624]
@ 2022-09-29 12:42 Wilco Dijkstra
  2022-09-29 13:25 ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 4+ messages in thread
From: Wilco Dijkstra @ 2022-09-29 12:42 UTC (permalink / raw)
  To: fw, peterlin; +Cc: 'GNU C Library'

Hi,

Also note the getrandom syscall is redundant since random_bits uses
clock_gettime. There is no need for a cryptographic random number here,
so the syscall can be removed.

And in places where we do want to use getrandom, it makes sense to
wrap the syscall so that we avoid introducing this bug in more places.

Cheers,
Wilco

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] malloc: Fix clobbered errno when getrandom failed [BZ #29624]
  2022-09-29 12:42 [PATCH] malloc: Fix clobbered errno when getrandom failed [BZ #29624] Wilco Dijkstra
@ 2022-09-29 13:25 ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 4+ messages in thread
From: Adhemerval Zanella Netto @ 2022-09-29 13:25 UTC (permalink / raw)
  To: libc-alpha



On 29/09/22 09:42, Wilco Dijkstra via Libc-alpha wrote:
> Hi,
> 
> Also note the getrandom syscall is redundant since random_bits uses
> clock_gettime. There is no need for a cryptographic random number here,
> so the syscall can be removed.

It will be a syscall on some architecture anyway (although very unlikely
to fail).  But I agree it make sense to use random_bits here.

> 
> And in places where we do want to use getrandom, it makes sense to
> wrap the syscall so that we avoid introducing this bug in more places.

Yeah, I will send a patch to fix both issues.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] malloc: Fix clobbered errno when getrandom failed [BZ #29624]
  2022-09-29  8:33 Yu Chien Peter Lin
@ 2022-09-29 10:00 ` Florian Weimer
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer @ 2022-09-29 10:00 UTC (permalink / raw)
  To: Yu Chien Peter Lin; +Cc: libc-alpha, ycliang, alankao

* Yu Chien Peter Lin:

> The patch resets errno when getrandom syscall failed, which 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 | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 953183e956..21f2bf5431 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3140,6 +3140,7 @@ tcache_key_initialize (void)
>  #if __WORDSIZE == 64
>        tcache_key = (tcache_key << 32) | random_bits ();
>  #endif
> +      __set_errno(0);
>      }
>  }

Sorry, this is wrong for the dynamically linked case because we do not
call malloc before calling the main function.  The first call to
malloc will set errno to 0, which is observable by the application in
this case.  And such errno-setting behavior is not permitted by POSIX.

You need to save and restore errno instead.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] malloc: Fix clobbered errno when getrandom failed [BZ #29624]
@ 2022-09-29  8:33 Yu Chien Peter Lin
  2022-09-29 10:00 ` Florian Weimer
  0 siblings, 1 reply; 4+ messages in thread
From: Yu Chien Peter Lin @ 2022-09-29  8:33 UTC (permalink / raw)
  To: libc-alpha; +Cc: alankao, ycliang, Yu Chien Peter Lin

The patch resets errno when getrandom syscall failed, which 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 953183e956..21f2bf5431 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3140,6 +3140,7 @@ tcache_key_initialize (void)
 #if __WORDSIZE == 64
       tcache_key = (tcache_key << 32) | random_bits ();
 #endif
+      __set_errno(0);
     }
 }
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-09-29 13:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 12:42 [PATCH] malloc: Fix clobbered errno when getrandom failed [BZ #29624] Wilco Dijkstra
2022-09-29 13:25 ` Adhemerval Zanella Netto
  -- strict thread matches above, loose matches on Subject: below --
2022-09-29  8:33 Yu Chien Peter Lin
2022-09-29 10:00 ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).