random: allow partial reads if later user copies fail
authorJason A. Donenfeld <Jason@zx2c4.com>
Thu, 7 Apr 2022 19:23:08 +0000 (21:23 +0200)
committerJason A. Donenfeld <Jason@zx2c4.com>
Wed, 13 Apr 2022 11:58:57 +0000 (13:58 +0200)
Rather than failing entirely if a copy_to_user() fails at some point,
instead we should return a partial read for the amount that succeeded
prior, unless none succeeded at all, in which case we return -EFAULT as
before.

This makes it consistent with other reader interfaces. For example, the
following snippet for /dev/zero outputs "4" followed by "1":

  int fd;
  void *x = mmap(NULL, 4096, PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
  assert(x != MAP_FAILED);
  fd = open("/dev/zero", O_RDONLY);
  assert(fd >= 0);
  printf("%zd\n", read(fd, x, 4));
  printf("%zd\n", read(fd, x + 4095, 4));
  close(fd);

This brings that same standard behavior to the various RNG reader
interfaces.

While we're at it, we can streamline the loop logic a little bit.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jann Horn <jannh@google.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
drivers/char/random.c

index e15063d614600e79851e6a87c52a028821d4aa1e..df43c5060f00d2ea23758ff5e37a97eea4764772 100644 (file)
@@ -523,8 +523,7 @@ EXPORT_SYMBOL(get_random_bytes);
 
 static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
 {
-       ssize_t ret = 0;
-       size_t len;
+       size_t len, left, ret = 0;
        u32 chacha_state[CHACHA_STATE_WORDS];
        u8 output[CHACHA_BLOCK_SIZE];
 
@@ -543,37 +542,40 @@ static ssize_t get_random_bytes_user(void __user *buf, size_t nbytes)
         * the user directly.
         */
        if (nbytes <= CHACHA_KEY_SIZE) {
-               ret = copy_to_user(buf, &chacha_state[4], nbytes) ? -EFAULT : nbytes;
+               ret = nbytes - copy_to_user(buf, &chacha_state[4], 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;
+               left = copy_to_user(buf, output, len);
+               if (left) {
+                       ret += len - left;
                        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 (ret % PAGE_SIZE == 0) {
                        if (signal_pending(current))
                                break;
                        cond_resched();
                }
-       } while (nbytes);
+       }
 
        memzero_explicit(output, sizeof(output));
 out_zero_chacha:
        memzero_explicit(chacha_state, sizeof(chacha_state));
-       return ret;
+       return ret ? ret : -EFAULT;
 }
 
 /*