On 11/20/24 9:02 AM, Mark Harmstone wrote:
> If we return -EAGAIN the first time because we need to block,
> btrfs_uring_encoded_read() will get called twice. Take a copy of args
> the first time, to prevent userspace from messing around with it.
>
> Signed-off-by: Mark Harmstone <maharmstone@xxxxxx>
> ---
> fs/btrfs/ioctl.c | 74 ++++++++++++++++++++++++++++++++----------------
> 1 file changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 488dcd022dea..97f7812cbf7c 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4873,7 +4873,7 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
> {
> size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args, flags);
> size_t copy_end;
> - struct btrfs_ioctl_encoded_io_args args = { 0 };
> + struct btrfs_ioctl_encoded_io_args *args;
> int ret;
> u64 disk_bytenr, disk_io_size;
> struct file *file;
> @@ -4888,6 +4888,9 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
> struct extent_state *cached_state = NULL;
> u64 start, lockend;
> void __user *sqe_addr;
> + struct io_kiocb *req = cmd_to_io_kiocb(cmd);
> + struct io_uring_cmd_data *data = req->async_data;
> + bool need_copy = false;
>
> if (!capable(CAP_SYS_ADMIN)) {
> ret = -EPERM;
> @@ -4899,34 +4902,55 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
> io_tree = &inode->io_tree;
> sqe_addr = u64_to_user_ptr(READ_ONCE(cmd->sqe->addr));
>
> + if (!data->op_data) {
> + data->op_data = kzalloc(sizeof(*args), GFP_NOFS);
> + if (!data->op_data) {
> + ret = -ENOMEM;
> + goto out_acct;
> + }
> +
> + need_copy = true;
> + }
I'd probably get rid of this need_copy variable and just do the copy
here? Might look cleaner with an btrfs_alloc_copy_foo() helper, as you
could just do:
ret = btrfs_alloc_copy_foo(...);
if (unlikely(ret))
return ret;
and hide all that ugly compat business outside the meat of this
function.
More of a style thing, the change itself looks fine.
--
Jens Axboe
