Messages in this thread | | | Date | Fri, 20 May 2022 01:20:23 +0200 | From | "Jason A. Donenfeld" <> | Subject | Re: [PATCH 1/2] random: convert to using fops->read_iter() |
| |
On Fri, May 20, 2022 at 01:12:04AM +0200, Jason A. Donenfeld wrote: > Hi Jens, > > On Thu, May 19, 2022 at 01:31:32PM -0600, Jens Axboe wrote: > > for (;;) { > > chacha20_block(chacha_state, output); > > if (unlikely(chacha_state[12] == 0)) > > ++chacha_state[13]; > > > > block_len = min_t(size_t, len, CHACHA_BLOCK_SIZE); > > - left = copy_to_user(ubuf, output, block_len); > > - if (left) { > > - ret += block_len - left; > > + block_len = copy_to_iter(output, block_len, to); > > + if (!block_len) > > break; > > - } > > > > - ubuf += block_len; > > ret += block_len; > > len -= block_len; > > - if (!len) > > - break; > > > > BUILD_BUG_ON(PAGE_SIZE % CHACHA_BLOCK_SIZE != 0); > > if (ret % PAGE_SIZE == 0) { > > if (signal_pending(current)) > > break; > > cond_resched(); > > } > > } > > This isn't quite the same, is it? Before, it would immediately break out > of the loop on any short copy. Now, it will only break out on a zero > copy, which means it's possible that ret % PAGE_SIZE == 0, and there'll > be an unnecessary cond_resched() before copy_to_iter() runs again and > then breaks.
Maybe something like the below would do the trick?
static ssize_t get_random_bytes_user(struct iov_iter *to) { size_t block_len, copied, ret = 0, len = iov_iter_count(to); u32 chacha_state[CHACHA_STATE_WORDS]; u8 output[CHACHA_BLOCK_SIZE];
if (!len) return 0;
/* * Immediately overwrite the ChaCha key at index 4 with random * 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); /* * 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 (len <= CHACHA_KEY_SIZE) { ret = copy_to_iter(&chacha_state[4], len, to); goto out_zero_chacha; }
for (;;) { chacha20_block(chacha_state, output); if (unlikely(chacha_state[12] == 0)) ++chacha_state[13];
block_len = min_t(size_t, len, CHACHA_BLOCK_SIZE); copied = copy_to_iter(output, block_len, to); ret += copied; if (block_len != copied) break; len -= copied;
BUILD_BUG_ON(PAGE_SIZE % CHACHA_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 ? ret : -EFAULT; }
| |