Skip to content

Commit c21b89d

Browse files
maharmstonekdave
authored andcommitted
btrfs: don't read from userspace twice in btrfs_uring_encoded_read()
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 iovs, and the iter the first time, as by the time we are called the second time these may have gone out of scope. Reported-by: Jens Axboe <[email protected]> Fixes: 34310c4 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)") Signed-off-by: Mark Harmstone <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent b0af20d commit c21b89d

File tree

1 file changed

+65
-57
lines changed

1 file changed

+65
-57
lines changed

fs/btrfs/ioctl.c

Lines changed: 65 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4879,25 +4879,29 @@ static int btrfs_uring_read_extent(struct kiocb *iocb, struct iov_iter *iter,
48794879
return ret;
48804880
}
48814881

4882+
struct btrfs_uring_encoded_data {
4883+
struct btrfs_ioctl_encoded_io_args args;
4884+
struct iovec iovstack[UIO_FASTIOV];
4885+
struct iovec *iov;
4886+
struct iov_iter iter;
4887+
};
4888+
48824889
static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue_flags)
48834890
{
48844891
size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args, flags);
48854892
size_t copy_end;
4886-
struct btrfs_ioctl_encoded_io_args args = { 0 };
48874893
int ret;
48884894
u64 disk_bytenr, disk_io_size;
48894895
struct file *file;
48904896
struct btrfs_inode *inode;
48914897
struct btrfs_fs_info *fs_info;
48924898
struct extent_io_tree *io_tree;
4893-
struct iovec iovstack[UIO_FASTIOV];
4894-
struct iovec *iov = iovstack;
4895-
struct iov_iter iter;
48964899
loff_t pos;
48974900
struct kiocb kiocb;
48984901
struct extent_state *cached_state = NULL;
48994902
u64 start, lockend;
49004903
void __user *sqe_addr;
4904+
struct btrfs_uring_encoded_data *data = io_uring_cmd_get_async_data(cmd)->op_data;
49014905

49024906
if (!capable(CAP_SYS_ADMIN)) {
49034907
ret = -EPERM;
@@ -4911,43 +4915,64 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
49114915

49124916
if (issue_flags & IO_URING_F_COMPAT) {
49134917
#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
4914-
struct btrfs_ioctl_encoded_io_args_32 args32;
4915-
49164918
copy_end = offsetofend(struct btrfs_ioctl_encoded_io_args_32, flags);
4917-
if (copy_from_user(&args32, sqe_addr, copy_end)) {
4918-
ret = -EFAULT;
4919-
goto out_acct;
4920-
}
4921-
args.iov = compat_ptr(args32.iov);
4922-
args.iovcnt = args32.iovcnt;
4923-
args.offset = args32.offset;
4924-
args.flags = args32.flags;
49254919
#else
49264920
return -ENOTTY;
49274921
#endif
49284922
} else {
49294923
copy_end = copy_end_kernel;
4930-
if (copy_from_user(&args, sqe_addr, copy_end)) {
4931-
ret = -EFAULT;
4924+
}
4925+
4926+
if (!data) {
4927+
data = kzalloc(sizeof(*data), GFP_NOFS);
4928+
if (!data) {
4929+
ret = -ENOMEM;
49324930
goto out_acct;
49334931
}
4934-
}
49354932

4936-
if (args.flags != 0)
4937-
return -EINVAL;
4933+
io_uring_cmd_get_async_data(cmd)->op_data = data;
49384934

4939-
ret = import_iovec(ITER_DEST, args.iov, args.iovcnt, ARRAY_SIZE(iovstack),
4940-
&iov, &iter);
4941-
if (ret < 0)
4942-
goto out_acct;
4935+
if (issue_flags & IO_URING_F_COMPAT) {
4936+
#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
4937+
struct btrfs_ioctl_encoded_io_args_32 args32;
49434938

4944-
if (iov_iter_count(&iter) == 0) {
4945-
ret = 0;
4946-
goto out_free;
4939+
if (copy_from_user(&args32, sqe_addr, copy_end)) {
4940+
ret = -EFAULT;
4941+
goto out_acct;
4942+
}
4943+
4944+
data->args.iov = compat_ptr(args32.iov);
4945+
data->args.iovcnt = args32.iovcnt;
4946+
data->args.offset = args32.offset;
4947+
data->args.flags = args32.flags;
4948+
#endif
4949+
} else {
4950+
if (copy_from_user(&data->args, sqe_addr, copy_end)) {
4951+
ret = -EFAULT;
4952+
goto out_acct;
4953+
}
4954+
}
4955+
4956+
if (data->args.flags != 0) {
4957+
ret = -EINVAL;
4958+
goto out_acct;
4959+
}
4960+
4961+
data->iov = data->iovstack;
4962+
ret = import_iovec(ITER_DEST, data->args.iov, data->args.iovcnt,
4963+
ARRAY_SIZE(data->iovstack), &data->iov,
4964+
&data->iter);
4965+
if (ret < 0)
4966+
goto out_acct;
4967+
4968+
if (iov_iter_count(&data->iter) == 0) {
4969+
ret = 0;
4970+
goto out_free;
4971+
}
49474972
}
49484973

4949-
pos = args.offset;
4950-
ret = rw_verify_area(READ, file, &pos, args.len);
4974+
pos = data->args.offset;
4975+
ret = rw_verify_area(READ, file, &pos, data->args.len);
49514976
if (ret < 0)
49524977
goto out_free;
49534978

@@ -4960,15 +4985,16 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
49604985
start = ALIGN_DOWN(pos, fs_info->sectorsize);
49614986
lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
49624987

4963-
ret = btrfs_encoded_read(&kiocb, &iter, &args, &cached_state,
4988+
ret = btrfs_encoded_read(&kiocb, &data->iter, &data->args, &cached_state,
49644989
&disk_bytenr, &disk_io_size);
49654990
if (ret < 0 && ret != -EIOCBQUEUED)
49664991
goto out_free;
49674992

49684993
file_accessed(file);
49694994

4970-
if (copy_to_user(sqe_addr + copy_end, (const char *)&args + copy_end_kernel,
4971-
sizeof(args) - copy_end_kernel)) {
4995+
if (copy_to_user(sqe_addr + copy_end,
4996+
(const char *)&data->args + copy_end_kernel,
4997+
sizeof(data->args) - copy_end_kernel)) {
49724998
if (ret == -EIOCBQUEUED) {
49734999
unlock_extent(io_tree, start, lockend, &cached_state);
49745000
btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
@@ -4978,40 +5004,22 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
49785004
}
49795005

49805006
if (ret == -EIOCBQUEUED) {
4981-
u64 count;
4982-
4983-
/*
4984-
* If we've optimized things by storing the iovecs on the stack,
4985-
* undo this.
4986-
*/
4987-
if (!iov) {
4988-
iov = kmalloc(sizeof(struct iovec) * args.iovcnt, GFP_NOFS);
4989-
if (!iov) {
4990-
unlock_extent(io_tree, start, lockend, &cached_state);
4991-
btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
4992-
ret = -ENOMEM;
4993-
goto out_acct;
4994-
}
4995-
4996-
memcpy(iov, iovstack, sizeof(struct iovec) * args.iovcnt);
4997-
}
4998-
4999-
count = min_t(u64, iov_iter_count(&iter), disk_io_size);
5007+
u64 count = min_t(u64, iov_iter_count(&data->iter), disk_io_size);
50005008

50015009
/* Match ioctl by not returning past EOF if uncompressed. */
5002-
if (!args.compression)
5003-
count = min_t(u64, count, args.len);
5010+
if (!data->args.compression)
5011+
count = min_t(u64, count, data->args.len);
50045012

5005-
ret = btrfs_uring_read_extent(&kiocb, &iter, start, lockend,
5006-
cached_state, disk_bytenr,
5007-
disk_io_size, count,
5008-
args.compression, iov, cmd);
5013+
ret = btrfs_uring_read_extent(&kiocb, &data->iter, start, lockend,
5014+
cached_state, disk_bytenr, disk_io_size,
5015+
count, data->args.compression,
5016+
data->iov, cmd);
50095017

50105018
goto out_acct;
50115019
}
50125020

50135021
out_free:
5014-
kfree(iov);
5022+
kfree(data->iov);
50155023

50165024
out_acct:
50175025
if (ret > 0)

0 commit comments

Comments
 (0)