From patchwork Fri Dec 2 13:55:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Bugaev X-Patchwork-Id: 61368 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6DB6D3858C2D for ; Fri, 2 Dec 2022 13:56:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6DB6D3858C2D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1669989412; bh=i4ODQ7TEbDI7+B479PHpNcNPchu5kdE64shH9zKxTbM=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=PXS+dfShusNkoNCqgLLld9AzTRJQV30ieGGDkqJ6NDImEKxq2M2mildNwYRf5STYG 18+YRyktbADAr97AMb/D/sMJrT6wsn8Vy5FPkXKXGSSqV7U/zdd467p1sKNifMREN+ CuuW3fB7+3x4Y2eb5crFbahw4DjiYgm1Cb9798Aw= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by sourceware.org (Postfix) with ESMTPS id 9F7433858D39 for ; Fri, 2 Dec 2022 13:56:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9F7433858D39 Received: by mail-lf1-x129.google.com with SMTP id d6so7484900lfs.10 for ; Fri, 02 Dec 2022 05:56:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=i4ODQ7TEbDI7+B479PHpNcNPchu5kdE64shH9zKxTbM=; b=Tt/ntyzUgsDCxVQIdeuFgCaeTdr6uF1LLfYJnuETUWndobbOqXKgACUNb5iEjo7tLK 6uITZ1Fir2iShr8RSsWF/c0M7JsX8y0etyuTQCgNAtZUyo92X6mrQIndXw7Pzgq4oQGQ 2RVzhX8s7ebXvydakcT4UI4daqM2dhdFWX4jmxL5lZt4ljiMKskzkZ7istUd4kXbhiX7 Sqz8fBYNe9NdsvKUhGR2IzzrDVUS6ZIfKmgmMtcG1Axtqm23rNFf2qDZnXIbOPV+5cI2 Na+kjhsH2AV89pfXAHdwcGaYe9Pa5PNIhUVz+7p1XhG+iQUr/h4qEki6q5732swF/3rE m3iA== X-Gm-Message-State: ANoB5pmhOKCFK7HPyMahgmHx7BfdH3O3iUH9uoplDVyJKJW/8qpPOsZ2 4xzMlD32C7drbkkWDbeTljY= X-Google-Smtp-Source: AA0mqf7oMGOEvbbUSkhPFZW+h4hWBMFp/BlsVsPzVtwX/DxEOazX+WVILWcJnVsQ3q+1X+/9IVvhwg== X-Received: by 2002:ac2:4c42:0:b0:4a5:bf09:a700 with SMTP id o2-20020ac24c42000000b004a5bf09a700mr17042084lfk.656.1669989387863; Fri, 02 Dec 2022 05:56:27 -0800 (PST) Received: from surface-pro-6.. ([194.190.106.50]) by smtp.gmail.com with ESMTPSA id u15-20020a05651220cf00b004b4e4671212sm1022317lfr.232.2022.12.02.05.56.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Dec 2022 05:56:27 -0800 (PST) To: bug-hurd@gnu.org, libc-alpha@sourceware.org, samuel.thibault@gnu.org Cc: gfleury@disroot.org, riccardo.mottola@libero.it, andrew.eggenberger@gmail.com, Sergey Bugaev Subject: [PATCH v3] hurd: Make getrandom cache the server port Date: Fri, 2 Dec 2022 16:55:58 +0300 Message-Id: <20221202135558.23781-1-bugaevc@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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: , X-Patchwork-Original-From: Sergey Bugaev via Libc-alpha From: Sergey Bugaev Reply-To: Sergey Bugaev Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" 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)