lkml.org 
[lkml]   [2022]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 4.14 143/237] random: check for signals every PAGE_SIZE chunk of /dev/[u]random
Date
From: "Jason A. Donenfeld" <Jason@zx2c4.com>

commit e3c1c4fd9e6d14059ed93ebfe15e1c57793b1a05 upstream.

In 1448769c9cdb ("random: check for signal_pending() outside of
need_resched() check"), Jann pointed out that we previously were only
checking the TIF_NOTIFY_SIGNAL and TIF_SIGPENDING flags if the process
had TIF_NEED_RESCHED set, which meant in practice, super long reads to
/dev/[u]random would delay signal handling by a long time. I tried this
using the below program, and indeed I wasn't able to interrupt a
/dev/urandom read until after several megabytes had been read. The bug
he fixed has always been there, and so code that reads from /dev/urandom
without checking the return value of read() has mostly worked for a long
time, for most sizes, not just for <= 256.

Maybe it makes sense to keep that code working. The reason it was so
small prior, ignoring the fact that it didn't work anyway, was likely
because /dev/random used to block, and that could happen for pretty
large lengths of time while entropy was gathered. But now, it's just a
chacha20 call, which is extremely fast and is just operating on pure
data, without having to wait for some external event. In that sense,
/dev/[u]random is a lot more like /dev/zero.

Taking a page out of /dev/zero's read_zero() function, it always returns
at least one chunk, and then checks for signals after each chunk. Chunk
sizes there are of length PAGE_SIZE. Let's just copy the same thing for
/dev/[u]random, and check for signals and cond_resched() for every
PAGE_SIZE amount of data. This makes the behavior more consistent with
expectations, and should mitigate the impact of Jann's fix for the
age-old signal check bug.

---- test program ----

#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <sys/random.h>

static unsigned char x[~0U];

static void handle(int) { }

int main(int argc, char *argv[])
{
pid_t pid = getpid(), child;
signal(SIGUSR1, handle);
if (!(child = fork())) {
for (;;)
kill(pid, SIGUSR1);
}
pause();
printf("interrupted after reading %zd bytes\n", getrandom(x, sizeof(x), 0));
kill(child, SIGTERM);
return 0;
}

Cc: Jann Horn <jannh@google.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/char/random.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)

--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -523,9 +523,7 @@ EXPORT_SYMBOL(get_random_bytes);

static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
{
- bool large_request = nbytes > 256;
- ssize_t ret = 0;
- size_t len;
+ size_t len, left, ret = 0;
u32 chacha_state[CHACHA20_BLOCK_SIZE / sizeof(u32)];
u8 output[CHACHA20_BLOCK_SIZE];

@@ -537,46 +535,47 @@ static ssize_t get_random_bytes_user(voi
* bytes, in case userspace causes copy_to_user() below to sleep
* forever, so that we still retain forward secrecy in that case.
*/
- crng_make_state(chacha_state, (u8 *)&chacha_state[4], CHACHA_KEY_SIZE);
+ crng_make_state(chacha_state, (u8 *)&chacha_state[4], CHACHA20_KEY_SIZE);
/*
* However, if we're doing a read of len <= 32, we don't need to
* use chacha_state after, so we can simply return those bytes to
* the user directly.
*/
- if (nbytes <= CHACHA_KEY_SIZE) {
- ret = copy_to_user(buf, &chacha_state[4], nbytes) ? -EFAULT : nbytes;
+ if (nbytes <= CHACHA20_KEY_SIZE) {
+ ret = nbytes - copy_to_user(buf, &chacha_state[4], nbytes);
goto out_zero_chacha;
}

- do {
- if (large_request) {
- if (signal_pending(current)) {
- if (!ret)
- ret = -ERESTARTSYS;
- break;
- }
- cond_resched();
- }
-
+ for (;;) {
chacha20_block(chacha_state, output);
if (unlikely(chacha_state[12] == 0))
++chacha_state[13];

len = min_t(size_t, nbytes, CHACHA20_BLOCK_SIZE);
- if (copy_to_user(buf, output, len)) {
- ret = -EFAULT;
+ left = copy_to_user(buf, output, len);
+ if (left) {
+ ret += len - left;
break;
}

- nbytes -= len;
buf += len;
ret += len;
- } while (nbytes);
+ nbytes -= len;
+ if (!nbytes)
+ break;
+
+ BUILD_BUG_ON(PAGE_SIZE % CHACHA20_BLOCK_SIZE != 0);
+ if (ret % PAGE_SIZE == 0) {
+ if (signal_pending(current))
+ break;
+ cond_resched();
+ }
+ }

memzero_explicit(output, sizeof(output));
out_zero_chacha:
memzero_explicit(chacha_state, sizeof(chacha_state));
- return ret;
+ return ret ? ret : -EFAULT;
}

/*

\
 
 \ /
  Last update: 2022-06-23 19:42    [W:1.179 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site