io_uring: use fget/fput consistently

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>

diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index 3c19cccb1aecf938185b6392580c8017e9bc3e31..8a8b07dfc444cde6181e2e5f86b6aa799ab5980b 100644 (file)
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -273,7 +273,7 @@ int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg)
        };
        ktime_t timeout = KTIME_MAX;
        struct io_uring_sync_cancel_reg sc;
-       struct fd f = { };
+       struct file *file = NULL;
        DEFINE_WAIT(wait);
        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 */
        if ((cd.flags & IORING_ASYNC_CANCEL_FD) &&
           !(cd.flags & IORING_ASYNC_CANCEL_FD_FIXED)) {
-               f = fdget(sc.fd);
-               if (!f.file)
+               file = fget(sc.fd);
+               if (!file)
                        return -EBADF;
-               cd.file = f.file;
+               cd.file = file;
        }
 
        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)
                ret = 0;
 out:
-       fdput(f);
+       if (file)
+               fput(file);
        return ret;
 }

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 05f933dddfde9f11c7e7b1901e21969992cd5dee..aba5657d287ef6d0f4a549ac5f27bcd5a30ab64b 100644 (file)
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3652,7 +3652,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
                size_t, argsz)
 {
        struct io_ring_ctx *ctx;
-       struct fd f;
+       struct file *file;
        long ret;
 
        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))
                        return -EINVAL;
                fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
-               f.file = tctx->registered_rings[fd];
-               f.flags = 0;
-               if (unlikely(!f.file))
+               file = tctx->registered_rings[fd];
+               if (unlikely(!file))
                        return -EBADF;
        } else {
-               f = fdget(fd);
-               if (unlikely(!f.file))
+               file = fget(fd);
+               if (unlikely(!file))
                        return -EBADF;
                ret = -EOPNOTSUPP;
-               if (unlikely(!io_is_uring_fops(f.file)))
+               if (unlikely(!io_is_uring_fops(file)))
                        goto out;
        }
 
-       ctx = f.file->private_data;
+       ctx = file->private_data;
        ret = -EBADFD;
        if (unlikely(ctx->flags & IORING_SETUP_R_DISABLED))
                goto out;
@@ -3777,7 +3776,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
                }
        }
 out:
-       fdput(f);
+       if (!(flags & IORING_ENTER_REGISTERED_RING))
+               fput(file);
        return ret;
 }
 
@@ -4618,7 +4618,7 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
 {
        struct io_ring_ctx *ctx;
        long ret = -EBADF;
-       struct fd f;
+       struct file *file;
        bool 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))
                        return -EINVAL;
                fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
-               f.file = tctx->registered_rings[fd];
-               f.flags = 0;
-               if (unlikely(!f.file))
+               file = tctx->registered_rings[fd];
+               if (unlikely(!file))
                        return -EBADF;
        } else {
-               f = fdget(fd);
-               if (unlikely(!f.file))
+               file = fget(fd);
+               if (unlikely(!file))
                        return -EBADF;
                ret = -EOPNOTSUPP;
-               if (!io_is_uring_fops(f.file))
+               if (!io_is_uring_fops(file))
                        goto out_fput;
        }
 
-       ctx = f.file->private_data;
+       ctx = file->private_data;
 
        mutex_lock(&ctx->uring_lock);
        ret = __io_uring_register(ctx, opcode, arg, nr_args);
        mutex_unlock(&ctx->uring_lock);
        trace_io_uring_register(ctx, opcode, ctx->nr_user_files, ctx->nr_user_bufs, ret);
 out_fput:
-       fdput(f);
+       if (!use_registered_ring)
+               fput(file);
        return ret;
 }