kevin/linux
1
Fork 0
Code

io_uring: use fget/fput consistently

Browse Source 
Normally within a syscall it's fine to use fdget/fdput for grabbing a
file from the file table, and it's fine within io_uring as well. We do
that via io_uring_enter(2), io_uring_register(2), and then also for
cancel which is invoked from the latter. io_uring cannot close its own
file descriptors as that is explicitly rejected, and for the cancel
side of things, the file itself is just used as a lookup cookie.

However, it is more prudent to ensure that full references are always
grabbed. For anything threaded, either explicitly in the application
itself or through use of the io-wq worker threads, this is what happens
anyway. Generalize it and use fget/fput throughout.

Also see the below link for more details.

Link: https://lore.kernel.org/io-uring/CAG48ez1htVSO3TqmrF8QcX2WFuYTRM-VZ_N10i-VZgbtg=NNqw@mail.gmail.com/
Suggested-by: Jann Horn <jannh@google.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
Jens Axboe 2023-11-28 10:29:58 -07:00
parent 5cf4f52e6d
commit 73363c262d
2 changed files with 24 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
io_uring/cancel.c
View File

@ -273,7 +273,7 @@ int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg)
}; };
ktime_t timeout = KTIME_MAX; ktime_t timeout = KTIME_MAX;
struct io_uring_sync_cancel_reg sc; struct io_uring_sync_cancel_reg sc;
struct fd f = { }; struct file *file = NULL;
DEFINE_WAIT(wait); DEFINE_WAIT(wait);
int ret, i; int ret, i;
@ -295,10 +295,10 @@ int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg)
/* we can grab a normal file descriptor upfront */ /* we can grab a normal file descriptor upfront */
if ((cd.flags & IORING_ASYNC_CANCEL_FD) && if ((cd.flags & IORING_ASYNC_CANCEL_FD) &&
!(cd.flags & IORING_ASYNC_CANCEL_FD_FIXED)) { !(cd.flags & IORING_ASYNC_CANCEL_FD_FIXED)) {
f = fdget(sc.fd); file = fget(sc.fd);
if (!f.file) if (!file)
return -EBADF; return -EBADF;
cd.file = f.file; cd.file = file;
} }
ret = __io_sync_cancel(current->io_uring, &cd, sc.fd); ret = __io_sync_cancel(current->io_uring, &cd, sc.fd);
@ -348,6 +348,7 @@ int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg)
if (ret == -ENOENT || ret > 0) if (ret == -ENOENT || ret > 0)
ret = 0; ret = 0;
out: out:
fdput(f); if (file)
fput(file);
return ret; return ret;
} }

36
io_uring/io_uring.c
View File

@ -3652,7 +3652,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
size_t, argsz) size_t, argsz)
{ {
struct io_ring_ctx *ctx; struct io_ring_ctx *ctx;
struct fd f; struct file *file;
long ret; long ret;
if (unlikely(flags & ~(IORING_ENTER_GETEVENTS | IORING_ENTER_SQ_WAKEUP | if (unlikely(flags & ~(IORING_ENTER_GETEVENTS | IORING_ENTER_SQ_WAKEUP |
@ -3670,20 +3670,19 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX)) if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX))
return -EINVAL; return -EINVAL;
fd = array_index_nospec(fd, IO_RINGFD_REG_MAX); fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
f.file = tctx->registered_rings[fd]; file = tctx->registered_rings[fd];
f.flags = 0; if (unlikely(!file))
if (unlikely(!f.file))
return -EBADF; return -EBADF;
} else { } else {
f = fdget(fd); file = fget(fd);
if (unlikely(!f.file)) if (unlikely(!file))
return -EBADF; return -EBADF;
ret = -EOPNOTSUPP; ret = -EOPNOTSUPP;
if (unlikely(!io_is_uring_fops(f.file))) if (unlikely(!io_is_uring_fops(file)))
goto out; goto out;
} }
ctx = f.file->private_data; ctx = file->private_data;
ret = -EBADFD; ret = -EBADFD;
if (unlikely(ctx->flags & IORING_SETUP_R_DISABLED)) if (unlikely(ctx->flags & IORING_SETUP_R_DISABLED))
goto out; goto out;
@ -3777,7 +3776,8 @@ iopoll_locked:
} }
} }
out: out:
fdput(f); if (!(flags & IORING_ENTER_REGISTERED_RING))
fput(file);
return ret; return ret;
} }
@ -4618,7 +4618,7 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
{ {
struct io_ring_ctx *ctx; struct io_ring_ctx *ctx;
long ret = -EBADF; long ret = -EBADF;
struct fd f; struct file *file;
bool use_registered_ring; bool use_registered_ring;
use_registered_ring = !!(opcode & IORING_REGISTER_USE_REGISTERED_RING); use_registered_ring = !!(opcode & IORING_REGISTER_USE_REGISTERED_RING);
@ -4637,27 +4637,27 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX)) if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX))
return -EINVAL; return -EINVAL;
fd = array_index_nospec(fd, IO_RINGFD_REG_MAX); fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
f.file = tctx->registered_rings[fd]; file = tctx->registered_rings[fd];
f.flags = 0; if (unlikely(!file))
if (unlikely(!f.file))
return -EBADF; return -EBADF;
} else { } else {
f = fdget(fd); file = fget(fd);
if (unlikely(!f.file)) if (unlikely(!file))
return -EBADF; return -EBADF;
ret = -EOPNOTSUPP; ret = -EOPNOTSUPP;
if (!io_is_uring_fops(f.file)) if (!io_is_uring_fops(file))
goto out_fput; goto out_fput;
} }
ctx = f.file->private_data; ctx = file->private_data;
mutex_lock(&ctx->uring_lock); mutex_lock(&ctx->uring_lock);
ret = __io_uring_register(ctx, opcode, arg, nr_args); ret = __io_uring_register(ctx, opcode, arg, nr_args);
mutex_unlock(&ctx->uring_lock); mutex_unlock(&ctx->uring_lock);
trace_io_uring_register(ctx, opcode, ctx->nr_user_files, ctx->nr_user_bufs, ret); trace_io_uring_register(ctx, opcode, ctx->nr_user_files, ctx->nr_user_bufs, ret);
out_fput: out_fput:
fdput(f); if (!use_registered_ring)
fput(file);
return ret; return ret;
} }