Re: [PATCH v2 3/3] btrfs: don't read from userspace twice in btrfs_uring_encoded_read()

Pavel Begunkov
2024-12-13T21:17:48Z

On 12/13/24 18:44, 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@fb.com>
> ---
> Refactored slightly from the first version, in order to eliminate the
> need_copy variable.
>
> fs/btrfs/ioctl.c | 75 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index dc5faa89cdba..684c1541105e 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4882,7 +4882,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;
> @@ -4897,6 +4897,8 @@ 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);

Oh no, please don't. In general cmd implementations should never
use helpers that are not explicitly given to them in
include/linux/io_uring/*

> + struct io_uring_cmd_data *data = req->async_data;

Just add a helper to io_uring/cmd.h,

>
> if (!capable(CAP_SYS_ADMIN)) {
> ret = -EPERM;
...

--
Pavel Begunkov