public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624)
@ 2022-09-29 16:45 Adhemerval Zanella
  2022-09-29 17:31 ` Andreas Schwab
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Adhemerval Zanella @ 2022-09-29 16:45 UTC (permalink / raw)
  To: libc-alpha, Wilco Dijkstra, Yu Chien Peter Lin

I am not sure if changing thread cache initialization to not use
getrandom syscall will add much, on recent kernel getrandom should
fast (although still a syscall) and I am not sure of the performance
implications.

---
Use INTERNAL_SYSCALL_CALL instead of INLINE_SYSCALL_CALL.  This
requires emulate the semantic for hurd call (so __arc4random_buf
uses the fallback).

Checked on x86_64-linux-gnu.
---
 stdlib/arc4random.c                  |  4 ++--
 sysdeps/mach/hurd/not-cancel.h       | 11 +++++++++--
 sysdeps/unix/sysv/linux/not-cancel.h |  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

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/mach/hurd/not-cancel.h b/sysdeps/mach/hurd/not-cancel.h
index ae58b734e3..f2cb9b60ba 100644
--- a/sysdeps/mach/hurd/not-cancel.h
+++ b/sysdeps/mach/hurd/not-cancel.h
@@ -27,6 +27,7 @@
 #include <sys/uio.h>
 #include <hurd.h>
 #include <hurd/fd.h>
+#include <sys/random.h>
 
 /* Non cancellable close syscall.  */
 __typeof (__close) __close_nocancel;
@@ -75,8 +76,14 @@ __typeof (__fcntl) __fcntl_nocancel;
 #define __fcntl64_nocancel(...) \
   __fcntl_nocancel (__VA_ARGS__)
 
-#define __getrandom_nocancel(buf, size, flags) \
-  __getrandom (buf, size, flags)
+static inline int
+__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
+{
+  int save_errno = errno;
+  int r = __getrandom (buf, buflen, flags);
+  __set_errno (save_errno);
+  return r == -1 ? -save_errno : r;
+}
 
 #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 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
-- 
2.34.1


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

* Re: [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624)
  2022-09-29 16:45 [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624) Adhemerval Zanella
@ 2022-09-29 17:31 ` Andreas Schwab
  2022-09-29 17:46 ` Yann Droneaud
  2022-09-29 17:52 ` Wilco Dijkstra
  2 siblings, 0 replies; 5+ messages in thread
From: Andreas Schwab @ 2022-09-29 17:31 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha
  Cc: Wilco Dijkstra, Yu Chien Peter Lin, Adhemerval Zanella

On Sep 29 2022, Adhemerval Zanella via Libc-alpha wrote:

> @@ -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)

This is a redundant condition.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

* Re: [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624)
  2022-09-29 16:45 [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624) Adhemerval Zanella
  2022-09-29 17:31 ` Andreas Schwab
@ 2022-09-29 17:46 ` Yann Droneaud
  2022-09-29 17:49   ` Adhemerval Zanella Netto
  2022-09-29 17:52 ` Wilco Dijkstra
  2 siblings, 1 reply; 5+ messages in thread
From: Yann Droneaud @ 2022-09-29 17:46 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

Hi,

29 septembre 2022 à 18:45 "Adhemerval Zanella via Libc-alpha" a écrit:

> Use INTERNAL_SYSCALL_CALL instead of INLINE_SYSCALL_CALL. This
> requires emulate the semantic for hurd call (so __arc4random_buf
> uses the fallback).
> 
> Checked on x86_64-linux-gnu.
> ---
>  stdlib/arc4random.c | 4 ++--
>  sysdeps/mach/hurd/not-cancel.h | 11 +++++++++--
>  sysdeps/unix/sysv/linux/not-cancel.h | 2 +-
>  3 files changed, 12 insertions(+), 5 deletions(-)

[...] 

> diff --git a/sysdeps/mach/hurd/not-cancel.h b/sysdeps/mach/hurd/not-cancel.h
> index ae58b734e3..f2cb9b60ba 100644
> --- a/sysdeps/mach/hurd/not-cancel.h
> +++ b/sysdeps/mach/hurd/not-cancel.h
> @@ -75,8 +76,14 @@ __typeof (__fcntl) __fcntl_nocancel;
>  #define __fcntl64_nocancel(...) \
>  __fcntl_nocancel (__VA_ARGS__)
>  
> -#define __getrandom_nocancel(buf, size, flags) \
> - __getrandom (buf, size, flags)
> +static inline int
> +__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
> +{
> + int save_errno = errno;
> + int r = __getrandom (buf, buflen, flags);
> + __set_errno (save_errno);
> + return r == -1 ? -save_errno : r;
> +}
>  

I don't get why the saved errno value is returned.


Regards.

-- 
Yann Droneaud
OPTEYA

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

* Re: [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624)
  2022-09-29 17:46 ` Yann Droneaud
@ 2022-09-29 17:49   ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2022-09-29 17:49 UTC (permalink / raw)
  To: Yann Droneaud, libc-alpha



On 29/09/22 14:46, Yann Droneaud wrote:
> Hi,
> 
> 29 septembre 2022 à 18:45 "Adhemerval Zanella via Libc-alpha" a écrit:
> 
>> Use INTERNAL_SYSCALL_CALL instead of INLINE_SYSCALL_CALL. This
>> requires emulate the semantic for hurd call (so __arc4random_buf
>> uses the fallback).
>>
>> Checked on x86_64-linux-gnu.
>> ---
>>  stdlib/arc4random.c | 4 ++--
>>  sysdeps/mach/hurd/not-cancel.h | 11 +++++++++--
>>  sysdeps/unix/sysv/linux/not-cancel.h | 2 +-
>>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> [...] 
> 
>> diff --git a/sysdeps/mach/hurd/not-cancel.h b/sysdeps/mach/hurd/not-cancel.h
>> index ae58b734e3..f2cb9b60ba 100644
>> --- a/sysdeps/mach/hurd/not-cancel.h
>> +++ b/sysdeps/mach/hurd/not-cancel.h
>> @@ -75,8 +76,14 @@ __typeof (__fcntl) __fcntl_nocancel;
>>  #define __fcntl64_nocancel(...) \
>>  __fcntl_nocancel (__VA_ARGS__)
>>  
>> -#define __getrandom_nocancel(buf, size, flags) \
>> - __getrandom (buf, size, flags)
>> +static inline int
>> +__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
>> +{
>> + int save_errno = errno;
>> + int r = __getrandom (buf, buflen, flags);
>> + __set_errno (save_errno);
>> + return r == -1 ? -save_errno : r;
>> +}
>>  
> 
> I don't get why the saved errno value is returned.

Oops, because it is wrong.

> 
> 
> Regards.
> 

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

* Re: [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624)
  2022-09-29 16:45 [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624) Adhemerval Zanella
  2022-09-29 17:31 ` Andreas Schwab
  2022-09-29 17:46 ` Yann Droneaud
@ 2022-09-29 17:52 ` Wilco Dijkstra
  2 siblings, 0 replies; 5+ messages in thread
From: Wilco Dijkstra @ 2022-09-29 17:52 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha, Yu Chien Peter Lin

Hi Adhemerval,

+static inline int
+__getrandom_nocancel (void *buf, size_t buflen, unsigned int flags)
+{
+  int save_errno = errno;
+  int r = __getrandom (buf, buflen, flags);
+  __set_errno (save_errno);
+  return r == -1 ? -save_errno : r;
+}

How does this work? If it fails, it returns the negated original errno, which
doesn't make any sense. It can be zero and so it would imply success on
failure... Shouldn't it just return r as it tries to preserve errno or do we need
if (r == -1) r = -errno; before we restore errno?

Cheers,
Wilco

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

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

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 16:45 [PATCH] malloc: Do not clobber errno on __getrandom_nocancel (BZ#29624) Adhemerval Zanella
2022-09-29 17:31 ` Andreas Schwab
2022-09-29 17:46 ` Yann Droneaud
2022-09-29 17:49   ` Adhemerval Zanella Netto
2022-09-29 17:52 ` Wilco Dijkstra

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).