Samuel Thibault, le ven. 02 déc. 2022 23:46:21 +0100, a ecrit:
> Fixed the indentation and commited, thanks!
I have also uploaded a glibc version 2.36-7~0 to unreleased, so people
can easily upgrade to it and get openssh 9.1 working.
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 <buga...@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.
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
