From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 4F9501F601 for ; Fri, 2 Dec 2022 22:47:01 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="EhGoKhe2"; dkim-atps=neutral Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9561C385B18C for ; Fri, 2 Dec 2022 22:46:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9561C385B18C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1670021216; bh=L8g7/7F6KfL+6ReFVxkBYIvAPEvXUl5GUpe+U+HjYC0=; h=Date:To:Cc:Subject:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=EhGoKhe2kRSIXq9ImqqTGCXspNRcli0r5Y7Vsnv4zqv+rynejxPMBZ8Pq8PeCOsR1 g7jfdmRRtAcpRDruXvMNXBrWeYu03nW1PpiTcGQKQzTt9fR6Qm0Bf1Ni1CSgZQHjdx ka3OB5OXfX0WkUaqTxnMSMdBSMLxcoL9dJJHOPiE= Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id 0693E385B195 for ; Fri, 2 Dec 2022 22:46:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0693E385B195 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p1EnZ-0006j8-Qa; Fri, 02 Dec 2022 17:46:25 -0500 Received: from [2a01:cb19:956:1b00:de41:a9ff:fe47:ec49] (helo=begin) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p1EnZ-0000sx-Ge; Fri, 02 Dec 2022 17:46:25 -0500 Received: from samy by begin with local (Exim 4.96) (envelope-from ) id 1p1EnV-003Avu-2L; Fri, 02 Dec 2022 23:46:21 +0100 Date: Fri, 2 Dec 2022 23:46:21 +0100 To: Sergey Bugaev Cc: bug-hurd@gnu.org, libc-alpha@sourceware.org, gfleury@disroot.org, riccardo.mottola@libero.it, andrew.eggenberger@gmail.com Subject: Re: [PATCH v3] hurd: Make getrandom cache the server port Message-ID: <20221202224621.rmy5xtoy3wnqgldx@begin> Mail-Followup-To: Sergey Bugaev , bug-hurd@gnu.org, libc-alpha@sourceware.org, gfleury@disroot.org, riccardo.mottola@libero.it, andrew.eggenberger@gmail.com References: <20221202135558.23781-1-bugaevc@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221202135558.23781-1-bugaevc@gmail.com> Organization: I am not organized User-Agent: NeoMutt/20170609 (1.8.3) X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Samuel Thibault via Libc-alpha Reply-To: Samuel Thibault Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" 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 > --- > 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 > . */ > > +#include > #include > #include > -#include > -#include > + > +__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.