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