unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] hurd: Make getrandom cache the server port
       [not found] <20221130003150.4mnx6xd2g53rox7a@begin>
@ 2022-12-02  9:13 ` Sergey Bugaev via Libc-alpha
  2022-12-02  9:17   ` Samuel Thibault via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Sergey Bugaev via Libc-alpha @ 2022-12-02  9:13 UTC (permalink / raw)
  To: bug-hurd, libc-alpha, samuel.thibault
  Cc: gfleury, riccardo.mottola, andrew.eggenberger, Sergey Bugaev

Previously, getrandom would, each time it's called, traverse the file
system to find /dev/urandom, fetch some random data from it, then throw
away that port. This is quite slow, while calls to getrandom are
genrally expected to be fast.

Additionally, this means that getrandom can not work when /dev/urandom
is unavailable, such as inside a chroot that lacks one. User programs
expect calls to getrandom to work inside a chroot if they first call
getrandom outside of the chroot.

In particular, this is known to break the OpenSSH server, and in that
case the issue is exacerbated by the API of arc4random, which prevents
it from properly reporting errors, forcing glibc to abort on failure.
This causes sshd to just die once it tries to generate a random number.

Caching the random server port, in a manner similar to how socket
server ports are cached, both improves the performance and works around
the chroot issue.

Tested on i686-gnu with the following program:

pthread_barrier_t barrier;

void *worker(void*) {
    pthread_barrier_wait(&barrier);
    uint32_t sum = 0;
    for (int i = 0; i < 10000; i++) {
        sum += arc4random();
    }
    return (void *)(uintptr_t) sum;
}

int main() {
    pthread_t threads[THREAD_COUNT];

    pthread_barrier_init(&barrier, NULL, THREAD_COUNT);

    for (int i = 0; i < THREAD_COUNT; i++) {
        pthread_create(&threads[i], NULL, worker, NULL);
    }
    for (int i = 0; i < THREAD_COUNT; i++) {
        void *retval;
        pthread_join(threads[i], &retval);
        printf("Thread %i: %lu\n", i, (unsigned long)(uintptr_t) retval);
    }

In my totally unscientific benchmark, with this patch, this completes
in about 7 seconds, whereas previously it took about 50 seconds. This
program was also used to test that getrandom () doesn't explode if the
random server dies, but instead reopens the /dev/urandom anew. I have
also verified that with this patch, OpenSSH can once again accept
connections properly.

Caveat: this new implementation does not respect the GRND_RANDOM flag
and always uses /dev/urandom to read random data. This does not seem to
be much of a problem, since there is only a single random server
implementation in the Hurd, and /dev/urandom is actually a symlink to
/dev/random:

$ showtrans /dev/*random
/dev/random: /hurd/random --seed-file /var/lib/random-seed
/dev/urandom: /hurd/symlink random

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/getrandom.c | 101 ++++++++++++++++++++++++++++------
 1 file changed, 85 insertions(+), 16 deletions(-)

diff --git a/sysdeps/mach/hurd/getrandom.c b/sysdeps/mach/hurd/getrandom.c
index ad2d3ba3..70a0e582 100644
--- a/sysdeps/mach/hurd/getrandom.c
+++ b/sysdeps/mach/hurd/getrandom.c
@@ -16,10 +16,12 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <hurd.h>
 #include <sys/random.h>
 #include <fcntl.h>
-#include <unistd.h>
-#include <not-cancel.h>
+
+__libc_rwlock_define_initialized (static, lock);
+static file_t random_server, random_server_nonblock;
 
 extern char *__trivfs_server_name __attribute__((weak));
 
@@ -28,10 +30,14 @@ extern char *__trivfs_server_name __attribute__((weak));
 ssize_t
 __getrandom (void *buffer, size_t length, unsigned int flags)
 {
-  const char *random_source = "/dev/urandom";
-  int open_flags = O_RDONLY | O_CLOEXEC;
-  size_t amount_read;
-  int fd;
+  file_t server, *cached_server;;
+  error_t err;
+  int open_flags;
+  char *data = buffer;
+  mach_msg_type_number_t nread = length;
+
+  if (flags & ~(GRND_RANDOM | GRND_NONBLOCK))
+    return __hurd_fail (EINVAL);
 
   if (&__trivfs_server_name && __trivfs_server_name
       && __trivfs_server_name[0] == 'r'
@@ -44,19 +50,82 @@ __getrandom (void *buffer, size_t length, unsigned int flags)
     /* We are random, don't try to read ourselves!  */
     return length;
 
-  if (flags & GRND_RANDOM)
-    random_source = "/dev/random";
+  cached_server = (flags & GRND_NONBLOCK) ? &random_server_nonblock
+                                          : &random_server;
+
+again:
+  __libc_rwlock_rdlock (lock);
+  server = *cached_server;
+  if (MACH_PORT_VALID (server))
+    /* Attempt to read some random data using this port.  */
+    err = __io_read (server, &data, &nread, -1, length);
+  else
+    err = MACH_SEND_INVALID_DEST;
+  __libc_rwlock_unlock (lock);
+
+  if (err == MACH_SEND_INVALID_DEST || err == MIG_SERVER_DIED)
+    {
+      file_t oldserver = server;
+      mach_port_urefs_t urefs;
+
+      /* Slow path: the cached port didn't work, or there was no
+         cached port in the first place.  */
+
+      __libc_rwlock_wrlock (lock);
+      server = *cached_server;
+      if (server != oldserver)
+        {
+          /* Someone else must have refetched the port while we were
+             waiting for the lock. */
+          __libc_rwlock_unlock (lock);
+          goto again;
+        }
+
+      if (MACH_PORT_VALID (server))
+        {
+          /* It could be that someone else has refetched the port and
+             it got the very same name.  So check whether it is a send
+             right (and not a dead name).  */
+          err = __mach_port_get_refs (__mach_task_self (), server,
+                                      MACH_PORT_RIGHT_SEND, &urefs);
+          if (!err && urefs > 0)
+            {
+              __libc_rwlock_unlock (lock);
+              goto again;
+            }
+
+          /* Now we're sure that it's dead.  */
+          __mach_port_deallocate (__mach_task_self (), server);
+        }
+
+      open_flags = O_RDONLY | O_NOCTTY;
+      if (flags & GRND_NONBLOCK)
+        open_flags |= O_NONBLOCK;
+      server = *cached_server = __file_name_lookup ("/dev/urandom",
+                                                    open_flags, 0);
+      __libc_rwlock_unlock (lock);
+      if (!MACH_PORT_VALID (server))
+        /* No luck.  */
+        return -1;
+
+      goto again;
+    }
 
-  if (flags & GRND_NONBLOCK)
-    open_flags |= O_NONBLOCK;
+  if (err)
+    return __hurd_fail (err);
 
-  fd = __open_nocancel(random_source, open_flags);
-  if (fd == -1)
-    return -1;
+  if (data != buffer)
+    {
+      if (nread > length)
+        {
+          __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
+          return __hurd_fail (EGRATUITOUS);
+        }
+      memcpy (buffer, data, nread);
+      __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
+    }
 
-  amount_read = __read_nocancel(fd, buffer, length);
-  __close_nocancel_nostatus(fd);
-  return amount_read;
+  return nread;
 }
 
 libc_hidden_def (__getrandom)
-- 
2.38.1


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

* Re: [PATCH v2] hurd: Make getrandom cache the server port
  2022-12-02  9:13 ` [PATCH v2] hurd: Make getrandom cache the server port Sergey Bugaev via Libc-alpha
@ 2022-12-02  9:17   ` Samuel Thibault via Libc-alpha
  2022-12-02 13:18     ` Sergey Bugaev via Libc-alpha
  0 siblings, 1 reply; 7+ messages in thread
From: Samuel Thibault via Libc-alpha @ 2022-12-02  9:17 UTC (permalink / raw)
  To: Sergey Bugaev
  Cc: bug-hurd, libc-alpha, gfleury, riccardo.mottola,
	andrew.eggenberger

Hello,

Sergey Bugaev, le ven. 02 déc. 2022 12:13:51 +0300, a ecrit:
> Caveat: this new implementation does not respect the GRND_RANDOM flag
> and always uses /dev/urandom to read random data.

It should be easy to fix that?

> +static file_t random_server, random_server_nonblock;

By adding a second series of file_t

> -  if (flags & GRND_RANDOM)
> -    random_source = "/dev/random";
> +  cached_server = (flags & GRND_NONBLOCK) ? &random_server_nonblock
> +                                          : &random_server;

and multiplexing here.

Thanks for your work on this,
Samuel

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

* Re: [PATCH v2] hurd: Make getrandom cache the server port
  2022-12-02  9:17   ` Samuel Thibault via Libc-alpha
@ 2022-12-02 13:18     ` Sergey Bugaev via Libc-alpha
  2022-12-02 13:55       ` Samuel Thibault via Libc-alpha
  2022-12-02 13:55       ` [PATCH v3] " Sergey Bugaev via Libc-alpha
  0 siblings, 2 replies; 7+ messages in thread
From: Sergey Bugaev via Libc-alpha @ 2022-12-02 13:18 UTC (permalink / raw)
  To: Samuel Thibault
  Cc: bug-hurd, libc-alpha, gfleury, riccardo.mottola,
	andrew.eggenberger

On Fri, Dec 2, 2022 at 12:17 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Hello,
>
> Sergey Bugaev, le ven. 02 déc. 2022 12:13:51 +0300, a ecrit:
> > Caveat: this new implementation does not respect the GRND_RANDOM flag
> > and always uses /dev/urandom to read random data.
>
> It should be easy to fix that?
> By adding a second series of file_t
> and multiplexing here.

Sure, if you would prefer it that way. I'll send v3 in a moment.

> Thanks for your work on this,
> Samuel

Sergey

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

* Re: [PATCH v2] hurd: Make getrandom cache the server port
  2022-12-02 13:18     ` Sergey Bugaev via Libc-alpha
@ 2022-12-02 13:55       ` Samuel Thibault via Libc-alpha
  2022-12-02 13:55       ` [PATCH v3] " Sergey Bugaev via Libc-alpha
  1 sibling, 0 replies; 7+ messages in thread
From: Samuel Thibault via Libc-alpha @ 2022-12-02 13:55 UTC (permalink / raw)
  To: Sergey Bugaev
  Cc: bug-hurd, libc-alpha, gfleury, riccardo.mottola,
	andrew.eggenberger

Sergey Bugaev, le ven. 02 déc. 2022 16:18:33 +0300, a ecrit:
> On Fri, Dec 2, 2022 at 12:17 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > Sergey Bugaev, le ven. 02 déc. 2022 12:13:51 +0300, a ecrit:
> > > Caveat: this new implementation does not respect the GRND_RANDOM flag
> > > and always uses /dev/urandom to read random data.
> >
> > It should be easy to fix that?
> > By adding a second series of file_t
> > and multiplexing here.
> 
> Sure, if you would prefer it that way.

Yes, please, better not hide an issue :)

Samuel

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

* [PATCH v3] hurd: Make getrandom cache the server port
  2022-12-02 13:18     ` Sergey Bugaev via Libc-alpha
  2022-12-02 13:55       ` Samuel Thibault via Libc-alpha
@ 2022-12-02 13:55       ` Sergey Bugaev via Libc-alpha
  2022-12-02 14:04         ` Sergey Bugaev via Libc-alpha
  2022-12-02 22:46         ` Samuel Thibault via Libc-alpha
  1 sibling, 2 replies; 7+ messages in thread
From: Sergey Bugaev via Libc-alpha @ 2022-12-02 13:55 UTC (permalink / raw)
  To: bug-hurd, libc-alpha, samuel.thibault
  Cc: gfleury, riccardo.mottola, andrew.eggenberger, Sergey Bugaev

Changes since v2:
* Bring back support for reading /dev/random if so requested with
  GRND_RANDOM;
* Fix 'git commit' having eaten a #define in the commit message;
* Do not pass O_NOCTTY, because:
  a) O_NOCTTY is 0; I mixed it up with O_IGNORE_CTTY,
  b) we're not opening an fd anyway, so it's irrelevant.

-- >8 --

Previously, getrandom would, each time it's called, traverse the file
system to find /dev/urandom, fetch some random data from it, then throw
away that port. This is quite slow, while calls to getrandom are
genrally expected to be fast.

Additionally, this means that getrandom can not work when /dev/urandom
is unavailable, such as inside a chroot that lacks one. User programs
expect calls to getrandom to work inside a chroot if they first call
getrandom outside of the chroot.

In particular, this is known to break the OpenSSH server, and in that
case the issue is exacerbated by the API of arc4random, which prevents
it from properly reporting errors, forcing glibc to abort on failure.
This causes sshd to just die once it tries to generate a random number.

Caching the random server port, in a manner similar to how socket
server ports are cached, both improves the performance and works around
the chroot issue.

Tested on i686-gnu with the following program:

#define THREAD_COUNT 100

pthread_barrier_t barrier;

void *worker(void*) {
    pthread_barrier_wait(&barrier);
    uint32_t sum = 0;
    for (int i = 0; i < 10000; i++) {
        sum += arc4random();
    }
    return (void *)(uintptr_t) sum;
}

int main() {
    pthread_t threads[THREAD_COUNT];

    pthread_barrier_init(&barrier, NULL, THREAD_COUNT);

    for (int i = 0; i < THREAD_COUNT; i++) {
        pthread_create(&threads[i], NULL, worker, NULL);
    }
    for (int i = 0; i < THREAD_COUNT; i++) {
        void *retval;
        pthread_join(threads[i], &retval);
        printf("Thread %i: %lu\n", i, (unsigned long)(uintptr_t) retval);
    }

In my totally unscientific benchmark, with this patch, this completes
in about 7 seconds, whereas previously it took about 50 seconds. This
program was also used to test that getrandom () doesn't explode if the
random server dies, but instead reopens the /dev/urandom anew. I have
also verified that with this patch, OpenSSH can once again accept
connections properly.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/getrandom.c | 117 +++++++++++++++++++++++++++++-----
 1 file changed, 102 insertions(+), 15 deletions(-)

diff --git a/sysdeps/mach/hurd/getrandom.c b/sysdeps/mach/hurd/getrandom.c
index ad2d3ba3..d58c691d 100644
--- a/sysdeps/mach/hurd/getrandom.c
+++ b/sysdeps/mach/hurd/getrandom.c
@@ -16,10 +16,13 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <hurd.h>
 #include <sys/random.h>
 #include <fcntl.h>
-#include <unistd.h>
-#include <not-cancel.h>
+
+__libc_rwlock_define_initialized (static, lock);
+static file_t random_server, random_server_nonblock,
+              urandom_server, urandom_server_nonblock;
 
 extern char *__trivfs_server_name __attribute__((weak));
 
@@ -29,9 +32,36 @@ ssize_t
 __getrandom (void *buffer, size_t length, unsigned int flags)
 {
   const char *random_source = "/dev/urandom";
-  int open_flags = O_RDONLY | O_CLOEXEC;
-  size_t amount_read;
-  int fd;
+  int open_flags = O_RDONLY;
+  file_t server, *cached_server;
+  error_t err;
+  char *data = buffer;
+  mach_msg_type_number_t nread = length;
+
+  switch (flags)
+    {
+    case 0:
+      cached_server = &urandom_server;
+      break;
+    case GRND_RANDOM:
+      cached_server = &random_server;
+      break;
+    case GRND_NONBLOCK:
+      cached_server = &urandom_server_nonblock;
+      break;
+    case GRND_RANDOM | GRND_NONBLOCK:
+      cached_server = &random_server_nonblock;
+      break;
+    default:
+      return __hurd_fail (EINVAL);
+    }
+
+    if (flags & GRND_RANDOM)
+      random_source = "/dev/random";
+    if (flags & GRND_NONBLOCK)
+      open_flags |= O_NONBLOCK;
+    /* No point in passing either O_NOCTTY, O_IGNORE_CTTY, or O_CLOEXEC
+       to file_name_lookup, since we're not making an fd.  */
 
   if (&__trivfs_server_name && __trivfs_server_name
       && __trivfs_server_name[0] == 'r'
@@ -44,19 +74,76 @@ __getrandom (void *buffer, size_t length, unsigned int flags)
     /* We are random, don't try to read ourselves!  */
     return length;
 
-  if (flags & GRND_RANDOM)
-    random_source = "/dev/random";
+again:
+  __libc_rwlock_rdlock (lock);
+  server = *cached_server;
+  if (MACH_PORT_VALID (server))
+    /* Attempt to read some random data using this port.  */
+    err = __io_read (server, &data, &nread, -1, length);
+  else
+    err = MACH_SEND_INVALID_DEST;
+  __libc_rwlock_unlock (lock);
+
+  if (err == MACH_SEND_INVALID_DEST || err == MIG_SERVER_DIED)
+    {
+      file_t oldserver = server;
+      mach_port_urefs_t urefs;
+
+      /* Slow path: the cached port didn't work, or there was no
+         cached port in the first place.  */
+
+      __libc_rwlock_wrlock (lock);
+      server = *cached_server;
+      if (server != oldserver)
+        {
+          /* Someone else must have refetched the port while we were
+             waiting for the lock. */
+          __libc_rwlock_unlock (lock);
+          goto again;
+        }
+
+      if (MACH_PORT_VALID (server))
+        {
+          /* It could be that someone else has refetched the port and
+             it got the very same name.  So check whether it is a send
+             right (and not a dead name).  */
+          err = __mach_port_get_refs (__mach_task_self (), server,
+                                      MACH_PORT_RIGHT_SEND, &urefs);
+          if (!err && urefs > 0)
+            {
+              __libc_rwlock_unlock (lock);
+              goto again;
+            }
+
+          /* Now we're sure that it's dead.  */
+          __mach_port_deallocate (__mach_task_self (), server);
+        }
+
+      server = *cached_server = __file_name_lookup (random_source,
+                                                    open_flags, 0);
+      __libc_rwlock_unlock (lock);
+      if (!MACH_PORT_VALID (server))
+        /* No luck.  */
+        return -1;
+
+      goto again;
+    }
 
-  if (flags & GRND_NONBLOCK)
-    open_flags |= O_NONBLOCK;
+  if (err)
+    return __hurd_fail (err);
 
-  fd = __open_nocancel(random_source, open_flags);
-  if (fd == -1)
-    return -1;
+  if (data != buffer)
+    {
+      if (nread > length)
+        {
+          __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
+          return __hurd_fail (EGRATUITOUS);
+        }
+      memcpy (buffer, data, nread);
+      __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
+    }
 
-  amount_read = __read_nocancel(fd, buffer, length);
-  __close_nocancel_nostatus(fd);
-  return amount_read;
+  return nread;
 }
 
 libc_hidden_def (__getrandom)
-- 
2.38.1


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

* Re: [PATCH v3] hurd: Make getrandom cache the server port
  2022-12-02 13:55       ` [PATCH v3] " Sergey Bugaev via Libc-alpha
@ 2022-12-02 14:04         ` Sergey Bugaev via Libc-alpha
  2022-12-02 22:46         ` Samuel Thibault via Libc-alpha
  1 sibling, 0 replies; 7+ messages in thread
From: Sergey Bugaev via Libc-alpha @ 2022-12-02 14:04 UTC (permalink / raw)
  To: bug-hurd, libc-alpha, samuel.thibault

On Fri, Dec 2, 2022 at 4:56 PM Sergey Bugaev <bugaevc@gmail.com> wrote:
> +  switch (flags)
> +    {
> +    case 0:
> +      cached_server = &urandom_server;
> +      break;
> +    case GRND_RANDOM:
> +      cached_server = &random_server;
> +      break;
> +    case GRND_NONBLOCK:
> +      cached_server = &urandom_server_nonblock;
> +      break;
> +    case GRND_RANDOM | GRND_NONBLOCK:
> +      cached_server = &random_server_nonblock;
> +      break;
> +    default:
> +      return __hurd_fail (EINVAL);
> +    }
> +
> +    if (flags & GRND_RANDOM)
> +      random_source = "/dev/random";
> +    if (flags & GRND_NONBLOCK)
> +      open_flags |= O_NONBLOCK;
> +    /* No point in passing either O_NOCTTY, O_IGNORE_CTTY, or O_CLOEXEC
> +       to file_name_lookup, since we're not making an fd.  */

Ooops, the indentation is off here, and of course I'm only seeing this
after having sent the patch. Nor did GCC warn me. Sigh.

If the patch is otherwise alright, could you please fix that when
committing? If it's not, I'll fix it in v4.

Sergey

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

* Re: [PATCH v3] hurd: Make getrandom cache the server port
  2022-12-02 13:55       ` [PATCH v3] " Sergey Bugaev via Libc-alpha
  2022-12-02 14:04         ` Sergey Bugaev via Libc-alpha
@ 2022-12-02 22:46         ` Samuel Thibault via Libc-alpha
  1 sibling, 0 replies; 7+ messages in thread
From: Samuel Thibault via Libc-alpha @ 2022-12-02 22:46 UTC (permalink / raw)
  To: Sergey Bugaev
  Cc: bug-hurd, libc-alpha, gfleury, riccardo.mottola,
	andrew.eggenberger

Fixed the indentation and commited, thanks!

Samuel

Sergey Bugaev, le ven. 02 déc. 2022 16:55:58 +0300, a ecrit:
> Changes since v2:
> * Bring back support for reading /dev/random if so requested with
>   GRND_RANDOM;
> * Fix 'git commit' having eaten a #define in the commit message;
> * Do not pass O_NOCTTY, because:
>   a) O_NOCTTY is 0; I mixed it up with O_IGNORE_CTTY,
>   b) we're not opening an fd anyway, so it's irrelevant.
> 
> -- >8 --
> 
> Previously, getrandom would, each time it's called, traverse the file
> system to find /dev/urandom, fetch some random data from it, then throw
> away that port. This is quite slow, while calls to getrandom are
> genrally expected to be fast.
> 
> Additionally, this means that getrandom can not work when /dev/urandom
> is unavailable, such as inside a chroot that lacks one. User programs
> expect calls to getrandom to work inside a chroot if they first call
> getrandom outside of the chroot.
> 
> In particular, this is known to break the OpenSSH server, and in that
> case the issue is exacerbated by the API of arc4random, which prevents
> it from properly reporting errors, forcing glibc to abort on failure.
> This causes sshd to just die once it tries to generate a random number.
> 
> Caching the random server port, in a manner similar to how socket
> server ports are cached, both improves the performance and works around
> the chroot issue.
> 
> Tested on i686-gnu with the following program:
> 
> #define THREAD_COUNT 100
> 
> pthread_barrier_t barrier;
> 
> void *worker(void*) {
>     pthread_barrier_wait(&barrier);
>     uint32_t sum = 0;
>     for (int i = 0; i < 10000; i++) {
>         sum += arc4random();
>     }
>     return (void *)(uintptr_t) sum;
> }
> 
> int main() {
>     pthread_t threads[THREAD_COUNT];
> 
>     pthread_barrier_init(&barrier, NULL, THREAD_COUNT);
> 
>     for (int i = 0; i < THREAD_COUNT; i++) {
>         pthread_create(&threads[i], NULL, worker, NULL);
>     }
>     for (int i = 0; i < THREAD_COUNT; i++) {
>         void *retval;
>         pthread_join(threads[i], &retval);
>         printf("Thread %i: %lu\n", i, (unsigned long)(uintptr_t) retval);
>     }
> 
> In my totally unscientific benchmark, with this patch, this completes
> in about 7 seconds, whereas previously it took about 50 seconds. This
> program was also used to test that getrandom () doesn't explode if the
> random server dies, but instead reopens the /dev/urandom anew. I have
> also verified that with this patch, OpenSSH can once again accept
> connections properly.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/getrandom.c | 117 +++++++++++++++++++++++++++++-----
>  1 file changed, 102 insertions(+), 15 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/getrandom.c b/sysdeps/mach/hurd/getrandom.c
> index ad2d3ba3..d58c691d 100644
> --- a/sysdeps/mach/hurd/getrandom.c
> +++ b/sysdeps/mach/hurd/getrandom.c
> @@ -16,10 +16,13 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <hurd.h>
>  #include <sys/random.h>
>  #include <fcntl.h>
> -#include <unistd.h>
> -#include <not-cancel.h>
> +
> +__libc_rwlock_define_initialized (static, lock);
> +static file_t random_server, random_server_nonblock,
> +              urandom_server, urandom_server_nonblock;
>  
>  extern char *__trivfs_server_name __attribute__((weak));
>  
> @@ -29,9 +32,36 @@ ssize_t
>  __getrandom (void *buffer, size_t length, unsigned int flags)
>  {
>    const char *random_source = "/dev/urandom";
> -  int open_flags = O_RDONLY | O_CLOEXEC;
> -  size_t amount_read;
> -  int fd;
> +  int open_flags = O_RDONLY;
> +  file_t server, *cached_server;
> +  error_t err;
> +  char *data = buffer;
> +  mach_msg_type_number_t nread = length;
> +
> +  switch (flags)
> +    {
> +    case 0:
> +      cached_server = &urandom_server;
> +      break;
> +    case GRND_RANDOM:
> +      cached_server = &random_server;
> +      break;
> +    case GRND_NONBLOCK:
> +      cached_server = &urandom_server_nonblock;
> +      break;
> +    case GRND_RANDOM | GRND_NONBLOCK:
> +      cached_server = &random_server_nonblock;
> +      break;
> +    default:
> +      return __hurd_fail (EINVAL);
> +    }
> +
> +    if (flags & GRND_RANDOM)
> +      random_source = "/dev/random";
> +    if (flags & GRND_NONBLOCK)
> +      open_flags |= O_NONBLOCK;
> +    /* No point in passing either O_NOCTTY, O_IGNORE_CTTY, or O_CLOEXEC
> +       to file_name_lookup, since we're not making an fd.  */
>  
>    if (&__trivfs_server_name && __trivfs_server_name
>        && __trivfs_server_name[0] == 'r'
> @@ -44,19 +74,76 @@ __getrandom (void *buffer, size_t length, unsigned int flags)
>      /* We are random, don't try to read ourselves!  */
>      return length;
>  
> -  if (flags & GRND_RANDOM)
> -    random_source = "/dev/random";
> +again:
> +  __libc_rwlock_rdlock (lock);
> +  server = *cached_server;
> +  if (MACH_PORT_VALID (server))
> +    /* Attempt to read some random data using this port.  */
> +    err = __io_read (server, &data, &nread, -1, length);
> +  else
> +    err = MACH_SEND_INVALID_DEST;
> +  __libc_rwlock_unlock (lock);
> +
> +  if (err == MACH_SEND_INVALID_DEST || err == MIG_SERVER_DIED)
> +    {
> +      file_t oldserver = server;
> +      mach_port_urefs_t urefs;
> +
> +      /* Slow path: the cached port didn't work, or there was no
> +         cached port in the first place.  */
> +
> +      __libc_rwlock_wrlock (lock);
> +      server = *cached_server;
> +      if (server != oldserver)
> +        {
> +          /* Someone else must have refetched the port while we were
> +             waiting for the lock. */
> +          __libc_rwlock_unlock (lock);
> +          goto again;
> +        }
> +
> +      if (MACH_PORT_VALID (server))
> +        {
> +          /* It could be that someone else has refetched the port and
> +             it got the very same name.  So check whether it is a send
> +             right (and not a dead name).  */
> +          err = __mach_port_get_refs (__mach_task_self (), server,
> +                                      MACH_PORT_RIGHT_SEND, &urefs);
> +          if (!err && urefs > 0)
> +            {
> +              __libc_rwlock_unlock (lock);
> +              goto again;
> +            }
> +
> +          /* Now we're sure that it's dead.  */
> +          __mach_port_deallocate (__mach_task_self (), server);
> +        }
> +
> +      server = *cached_server = __file_name_lookup (random_source,
> +                                                    open_flags, 0);
> +      __libc_rwlock_unlock (lock);
> +      if (!MACH_PORT_VALID (server))
> +        /* No luck.  */
> +        return -1;
> +
> +      goto again;
> +    }
>  
> -  if (flags & GRND_NONBLOCK)
> -    open_flags |= O_NONBLOCK;
> +  if (err)
> +    return __hurd_fail (err);
>  
> -  fd = __open_nocancel(random_source, open_flags);
> -  if (fd == -1)
> -    return -1;
> +  if (data != buffer)
> +    {
> +      if (nread > length)
> +        {
> +          __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
> +          return __hurd_fail (EGRATUITOUS);
> +        }
> +      memcpy (buffer, data, nread);
> +      __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
> +    }
>  
> -  amount_read = __read_nocancel(fd, buffer, length);
> -  __close_nocancel_nostatus(fd);
> -  return amount_read;
> +  return nread;
>  }
>  
>  libc_hidden_def (__getrandom)
> -- 
> 2.38.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

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

end of thread, other threads:[~2022-12-02 22:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20221130003150.4mnx6xd2g53rox7a@begin>
2022-12-02  9:13 ` [PATCH v2] hurd: Make getrandom cache the server port Sergey Bugaev via Libc-alpha
2022-12-02  9:17   ` Samuel Thibault via Libc-alpha
2022-12-02 13:18     ` Sergey Bugaev via Libc-alpha
2022-12-02 13:55       ` Samuel Thibault via Libc-alpha
2022-12-02 13:55       ` [PATCH v3] " Sergey Bugaev via Libc-alpha
2022-12-02 14:04         ` Sergey Bugaev via Libc-alpha
2022-12-02 22:46         ` Samuel Thibault via Libc-alpha

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