Now that the read side has been converted to fix a regression with
splice, convert the write side as well to have some symmetry in the
interface used (and help deprecate ->write()).
Signed-off-by: Jens Axboe <[email protected]>
---
Jason, this has only been booted. I did verify that it seems to take a
write just fine, but I would appreciate if you could vet this one with
your testing. Thanks!
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 41ca5966aa4f..3da04068c225 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1283,20 +1283,20 @@ static __poll_t random_poll(struct file *file, poll_table *wait)
return crng_ready() ? EPOLLIN | EPOLLRDNORM : EPOLLOUT | EPOLLWRNORM;
}
-static int write_pool(const char __user *ubuf, size_t len)
+static size_t write_pool(struct iov_iter *iter)
{
size_t block_len;
int ret = 0;
u8 block[BLAKE2S_BLOCK_SIZE];
- while (len) {
- block_len = min(len, sizeof(block));
- if (copy_from_user(block, ubuf, block_len)) {
- ret = -EFAULT;
+ while (iov_iter_count(iter)) {
+ block_len = min(iov_iter_count(iter), sizeof(block));
+ if (!copy_from_iter(block, block_len, iter)) {
+ if (!ret)
+ ret = -EFAULT;
goto out;
}
- len -= block_len;
- ubuf += block_len;
+ ret += block_len;
mix_pool_bytes(block, block_len);
cond_resched();
}
@@ -1306,16 +1306,9 @@ static int write_pool(const char __user *ubuf, size_t len)
return ret;
}
-static ssize_t random_write(struct file *file, const char __user *ubuf,
- size_t len, loff_t *ppos)
+static ssize_t random_write_iter(struct kiocb *kiocb, struct iov_iter *from)
{
- int ret;
-
- ret = write_pool(ubuf, len);
- if (ret)
- return ret;
-
- return (ssize_t)len;
+ return write_pool(from);
}
static ssize_t urandom_read_iter(struct kiocb *kiocb, struct iov_iter *to)
@@ -1373,7 +1366,10 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
return -EINVAL;
credit_init_bits(ent_count);
return 0;
- case RNDADDENTROPY:
+ case RNDADDENTROPY: {
+ struct iov_iter iter;
+ struct iovec iov;
+
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
if (get_user(ent_count, p++))
@@ -1382,11 +1378,16 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
return -EINVAL;
if (get_user(size, p++))
return -EFAULT;
- retval = write_pool((const char __user *)p, size);
+
+ iov.iov_base = p;
+ iov.iov_len = size;
+ iov_iter_init(&iter, WRITE, &iov, 1, size);
+ retval = write_pool(&iter);
if (retval < 0)
return retval;
credit_init_bits(ent_count);
return 0;
+ }
case RNDZAPENTCNT:
case RNDCLEARPOOL:
/* No longer has any effect. */
@@ -1412,7 +1413,7 @@ static int random_fasync(int fd, struct file *filp, int on)
const struct file_operations random_fops = {
.read_iter = random_read_iter,
- .write = random_write,
+ .write_iter = random_write_iter,
.poll = random_poll,
.unlocked_ioctl = random_ioctl,
.compat_ioctl = compat_ptr_ioctl,
@@ -1423,7 +1424,7 @@ const struct file_operations random_fops = {
const struct file_operations urandom_fops = {
.read_iter = urandom_read_iter,
- .write = random_write,
+ .write_iter = random_write_iter,
.unlocked_ioctl = random_ioctl,
.compat_ioctl = compat_ptr_ioctl,
.fasync = random_fasync,
--
Jens Axboe
On Thu, May 19, 2022 at 05:43:15PM -0600, Jens Axboe wrote:
> -static int write_pool(const char __user *ubuf, size_t len)
> +static size_t write_pool(struct iov_iter *iter)
> {
> size_t block_len;
> int ret = 0;
>
> - while (len) {
> - block_len = min(len, sizeof(block));
> - if (copy_from_user(block, ubuf, block_len)) {
> - ret = -EFAULT;
> + while (iov_iter_count(iter)) {
> + block_len = min(iov_iter_count(iter), sizeof(block));
> + if (!copy_from_iter(block, block_len, iter)) {
> + if (!ret)
> + ret = -EFAULT;
> goto out;
> }
Feed it a buffer with only 1 byte mapped, watch it'll pass to mix_pool_bytes().
And see how much of 'block' has been used uninitialized...
And why bother with that min thing, anyway?
ssize_t ret = 0;
while (iov_iter_count(iter)) {
u8 block[BLAKE2S_BLOCK_SIZE];
size_t copied = copy_from_iter(block, sizeof(block), iter);
if (!copied) {
if (!ret)
ret = -EFAULT;
break;
}
mix_pool_bytes(block, copied);
ret += copied;
}
return ret;
and be done with that...
> @@ -1382,11 +1378,16 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> return -EINVAL;
> if (get_user(size, p++))
> return -EFAULT;
> - retval = write_pool((const char __user *)p, size);
> +
> + iov.iov_base = p;
> + iov.iov_len = size;
> + iov_iter_init(&iter, WRITE, &iov, 1, size);
That'd be
import_single_range(WRITE, p, size, &iov, &iter);
Hi Jens,
On Thu, May 19, 2022 at 05:43:15PM -0600, Jens Axboe wrote:
> -static int write_pool(const char __user *ubuf, size_t len)
> +static size_t write_pool(struct iov_iter *iter)
> {
> size_t block_len;
> int ret = 0;
Changing the return value to size_t isn't quite right, as this can
return -EFAULT. So at the very minimum, it should return a ssize_t.
Jason