Skip to content

Commit e3c1c4f

Browse files
committed
random: check for signals every PAGE_SIZE chunk of /dev/[u]random
In 1448769 ("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>
1 parent 1448769 commit e3c1c4f

1 file changed

Lines changed: 7 additions & 10 deletions

File tree

drivers/char/random.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,6 @@ EXPORT_SYMBOL(get_random_bytes);
523523

524524
static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
525525
{
526-
bool large_request = nbytes > 256;
527526
ssize_t ret = 0;
528527
size_t len;
529528
u32 chacha_state[CHACHA_STATE_WORDS];
@@ -549,15 +548,6 @@ static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
549548
}
550549

551550
do {
552-
if (large_request) {
553-
if (signal_pending(current)) {
554-
if (!ret)
555-
ret = -ERESTARTSYS;
556-
break;
557-
}
558-
cond_resched();
559-
}
560-
561551
chacha20_block(chacha_state, output);
562552
if (unlikely(chacha_state[12] == 0))
563553
++chacha_state[13];
@@ -571,6 +561,13 @@ static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
571561
nbytes -= len;
572562
buf += len;
573563
ret += len;
564+
565+
BUILD_BUG_ON(PAGE_SIZE % CHACHA_BLOCK_SIZE != 0);
566+
if (!(ret % PAGE_SIZE) && nbytes) {
567+
if (signal_pending(current))
568+
break;
569+
cond_resched();
570+
}
574571
} while (nbytes);
575572

576573
memzero_explicit(output, sizeof(output));

0 commit comments

Comments
 (0)