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

[Pavel Begunkov <asml.silence@gmail.com>]

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