Re: [GIT PULL] random number generator fixes for 5.18-rc2

[Linus Torvalds]

On Thu, Apr 7, 2022 at 3:29 AM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
>
> - 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.

Ugh. After fixing a bug where the signal pending state isn't checked
enough, you then go to extra effort to not do it too much.

The whole historical "give at least 256 bytes without even checking
for signal_pending" is also cryptographically entirely bogus, since we
only actually have CHACHA_BLOCK_SIZE worth of random state

So if some program doesn't check for short reads, the difference
between one chacha block and 256 bytes (or PAGE_SIZE like you changed
it to) really *really* doesn't matter, the rest is going to be purely
filler anyway. Nice good filler, but still..

Also, if you hit a EFAULT, you should still return the partial result
you got before to be consistent with what we normally do in these
kinds of situations.

So I think we could just drop all these games, and make the code
simple and streamlined?

Attached patch not committed, only for "why not just stop the silly
games" comments..

                Linus
 drivers/char/random.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index e15063d61460..32c3d1dde16f 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -547,33 +547,30 @@ static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
 		goto out_zero_chacha;
 	}
 
-	do {
+	for (;;) {
 		chacha20_block(chacha_state, output);
 		if (unlikely(chacha_state[12] == 0))
 			++chacha_state[13];
 
 		len = min_t(size_t, nbytes, CHACHA_BLOCK_SIZE);
-		if (copy_to_user(buf, output, len)) {
-			ret = -EFAULT;
+		if (copy_to_user(buf, output, len))
 			break;
-		}
 
-		nbytes -= len;
 		buf += len;
 		ret += len;
+		nbytes -= len;
+		if (!nbytes)
+			break;
 
-		BUILD_BUG_ON(PAGE_SIZE % CHACHA_BLOCK_SIZE != 0);
-		if (!(ret % PAGE_SIZE) && nbytes) {
-			if (signal_pending(current))
-				break;
-			cond_resched();
-		}
-	} while (nbytes);
+		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;
 }
 
 /*