Skip to content

Commit 3638bd9

Browse files
committed
Merge tag 'random-5.18-rc2-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/crng/random
Pull random number generator fixes from Jason Donenfeld: - Another fixup to the fast_init/crng_init split, this time in how much entropy is being credited, from Jan Varho. - As discussed, we now opportunistically call try_to_generate_entropy() in /dev/urandom reads, as a replacement for the reverted commit. I opted to not do the more invasive wait_for_random_bytes() change at least for now, preferring to do something smaller and more obvious for the time being, but maybe that can be revisited as things evolve later. - Userspace can use FUSE or userfaultfd or simply move a process to idle priority in order to make a read from the random device never complete, which breaks forward secrecy, fixed by overwriting sensitive bytes early on in the function. - Jann Horn noticed that /dev/urandom reads were only checking for pending signals if need_resched() was true, a bug going back to the genesis commit, now fixed by always checking for signal_pending() and calling cond_resched(). This explains various noticeable signal delivery delays I've seen in programs over the years that do long reads from /dev/urandom. - In order to be more like other devices (e.g. /dev/zero) and to mitigate the impact of fixing the above bug, which has been around forever (users have never really needed to check the return value of read() for medium-sized reads and so perhaps many didn't), we now move signal checking to the bottom part of the loop, and do so every PAGE_SIZE-bytes. * tag 'random-5.18-rc2-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/crng/random: random: check for signals every PAGE_SIZE chunk of /dev/[u]random random: check for signal_pending() outside of need_resched() check random: do not allow user to keep crng key around on stack random: opportunistically initialize on /dev/urandom reads random: do not split fast init input in add_hwgenerator_randomness()
2 parents 640b503 + e3c1c4f commit 3638bd9

1 file changed

Lines changed: 39 additions & 35 deletions

File tree

drivers/char/random.c

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -437,11 +437,8 @@ static void crng_make_state(u32 chacha_state[CHACHA_STATE_WORDS],
437437
* This shouldn't be set by functions like add_device_randomness(),
438438
* where we can't trust the buffer passed to it is guaranteed to be
439439
* unpredictable (so it might not have any entropy at all).
440-
*
441-
* Returns the number of bytes processed from input, which is bounded
442-
* by CRNG_INIT_CNT_THRESH if account is true.
443440
*/
444-
static size_t crng_pre_init_inject(const void *input, size_t len, bool account)
441+
static void crng_pre_init_inject(const void *input, size_t len, bool account)
445442
{
446443
static int crng_init_cnt = 0;
447444
struct blake2s_state hash;
@@ -452,18 +449,15 @@ static size_t crng_pre_init_inject(const void *input, size_t len, bool account)
452449
spin_lock_irqsave(&base_crng.lock, flags);
453450
if (crng_init != 0) {
454451
spin_unlock_irqrestore(&base_crng.lock, flags);
455-
return 0;
452+
return;
456453
}
457454

458-
if (account)
459-
len = min_t(size_t, len, CRNG_INIT_CNT_THRESH - crng_init_cnt);
460-
461455
blake2s_update(&hash, base_crng.key, sizeof(base_crng.key));
462456
blake2s_update(&hash, input, len);
463457
blake2s_final(&hash, base_crng.key);
464458

465459
if (account) {
466-
crng_init_cnt += len;
460+
crng_init_cnt += min_t(size_t, len, CRNG_INIT_CNT_THRESH - crng_init_cnt);
467461
if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
468462
++base_crng.generation;
469463
crng_init = 1;
@@ -474,8 +468,6 @@ static size_t crng_pre_init_inject(const void *input, size_t len, bool account)
474468

475469
if (crng_init == 1)
476470
pr_notice("fast init done\n");
477-
478-
return len;
479471
}
480472

481473
static void _get_random_bytes(void *buf, size_t nbytes)
@@ -531,7 +523,6 @@ EXPORT_SYMBOL(get_random_bytes);
531523

532524
static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
533525
{
534-
bool large_request = nbytes > 256;
535526
ssize_t ret = 0;
536527
size_t len;
537528
u32 chacha_state[CHACHA_STATE_WORDS];
@@ -540,22 +531,23 @@ static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
540531
if (!nbytes)
541532
return 0;
542533

543-
len = min_t(size_t, 32, nbytes);
544-
crng_make_state(chacha_state, output, len);
545-
546-
if (copy_to_user(buf, output, len))
547-
return -EFAULT;
548-
nbytes -= len;
549-
buf += len;
550-
ret += len;
551-
552-
while (nbytes) {
553-
if (large_request && need_resched()) {
554-
if (signal_pending(current))
555-
break;
556-
schedule();
557-
}
534+
/*
535+
* Immediately overwrite the ChaCha key at index 4 with random
536+
* bytes, in case userspace causes copy_to_user() below to sleep
537+
* forever, so that we still retain forward secrecy in that case.
538+
*/
539+
crng_make_state(chacha_state, (u8 *)&chacha_state[4], CHACHA_KEY_SIZE);
540+
/*
541+
* However, if we're doing a read of len <= 32, we don't need to
542+
* use chacha_state after, so we can simply return those bytes to
543+
* the user directly.
544+
*/
545+
if (nbytes <= CHACHA_KEY_SIZE) {
546+
ret = copy_to_user(buf, &chacha_state[4], nbytes) ? -EFAULT : nbytes;
547+
goto out_zero_chacha;
548+
}
558549

550+
do {
559551
chacha20_block(chacha_state, output);
560552
if (unlikely(chacha_state[12] == 0))
561553
++chacha_state[13];
@@ -569,10 +561,18 @@ static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
569561
nbytes -= len;
570562
buf += len;
571563
ret += len;
572-
}
573564

574-
memzero_explicit(chacha_state, sizeof(chacha_state));
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+
}
571+
} while (nbytes);
572+
575573
memzero_explicit(output, sizeof(output));
574+
out_zero_chacha:
575+
memzero_explicit(chacha_state, sizeof(chacha_state));
576576
return ret;
577577
}
578578

@@ -1141,12 +1141,9 @@ void add_hwgenerator_randomness(const void *buffer, size_t count,
11411141
size_t entropy)
11421142
{
11431143
if (unlikely(crng_init == 0 && entropy < POOL_MIN_BITS)) {
1144-
size_t ret = crng_pre_init_inject(buffer, count, true);
1145-
mix_pool_bytes(buffer, ret);
1146-
count -= ret;
1147-
buffer += ret;
1148-
if (!count || crng_init == 0)
1149-
return;
1144+
crng_pre_init_inject(buffer, count, true);
1145+
mix_pool_bytes(buffer, count);
1146+
return;
11501147
}
11511148

11521149
/*
@@ -1545,6 +1542,13 @@ static ssize_t urandom_read(struct file *file, char __user *buf, size_t nbytes,
15451542
{
15461543
static int maxwarn = 10;
15471544

1545+
/*
1546+
* Opportunistically attempt to initialize the RNG on platforms that
1547+
* have fast cycle counters, but don't (for now) require it to succeed.
1548+
*/
1549+
if (!crng_ready())
1550+
try_to_generate_entropy();
1551+
15481552
if (!crng_ready() && maxwarn > 0) {
15491553
maxwarn--;
15501554
if (__ratelimit(&urandom_warning))

0 commit comments

Comments
 (0)