io_uring/fdinfo: grab ctx->uring_lock around io_uring_show_fdinfo()

Not everything requires locking in there, which is why the 'has_lock'
variable exists. But enough does that it's a bit unwieldy to manage.
Wrap the whole thing in a ->uring_lock trylock, and just return
with no output if we fail to grab it. The existing trylock() will
already have greatly diminished utility/output for the failure case.

This fixes an issue with reading the SQE fields, if the ring is being
actively resized at the same time.

Reported-by: Jann Horn <jannh@google.com>
Fixes: 79cfe9e59c2a ("io_uring/register: add IORING_REGISTER_RESIZE_RINGS")
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
index 9414ca6d101c0e94c8f037baf47a80c94f1c778b..e0d6a59a89fa1b23948c6271d2e9296e8b458c98 100644 (file)
--- a/io_uring/fdinfo.c
+++ b/io_uring/fdinfo.c
@@ -86,13 +86,8 @@ static inline void napi_show_fdinfo(struct io_ring_ctx *ctx,
 }
 #endif
 
-/*
- * Caller holds a reference to the file already, we don't need to do
- * anything else to get an extra reference.
- */
-__cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
+static void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, struct seq_file *m)
 {
-       struct io_ring_ctx *ctx = file->private_data;
        struct io_overflow_cqe *ocqe;
        struct io_rings *r = ctx->rings;
        struct rusage sq_usage;
@@ -106,7 +101,6 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
        unsigned int sq_entries, cq_entries;
        int sq_pid = -1, sq_cpu = -1;
        u64 sq_total_time = 0, sq_work_time = 0;
-       bool has_lock;
        unsigned int i;
 
        if (ctx->flags & IORING_SETUP_CQE32)
@@ -176,15 +170,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
                seq_printf(m, "\n");
        }
 
-       /*
-        * Avoid ABBA deadlock between the seq lock and the io_uring mutex,
-        * since fdinfo case grabs it in the opposite direction of normal use
-        * cases. If we fail to get the lock, we just don't iterate any
-        * structures that could be going away outside the io_uring mutex.
-        */
-       has_lock = mutex_trylock(&ctx->uring_lock);
-
-       if (has_lock && (ctx->flags & IORING_SETUP_SQPOLL)) {
+       if (ctx->flags & IORING_SETUP_SQPOLL) {
                struct io_sq_data *sq = ctx->sq_data;
 
                /*
@@ -206,7 +192,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
        seq_printf(m, "SqTotalTime:\t%llu\n", sq_total_time);
        seq_printf(m, "SqWorkTime:\t%llu\n", sq_work_time);
        seq_printf(m, "UserFiles:\t%u\n", ctx->file_table.data.nr);
-       for (i = 0; has_lock && i < ctx->file_table.data.nr; i++) {
+       for (i = 0; i < ctx->file_table.data.nr; i++) {
                struct file *f = NULL;
 
                if (ctx->file_table.data.nodes[i])
@@ -218,7 +204,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
                }
        }
        seq_printf(m, "UserBufs:\t%u\n", ctx->buf_table.nr);
-       for (i = 0; has_lock && i < ctx->buf_table.nr; i++) {
+       for (i = 0; i < ctx->buf_table.nr; i++) {
                struct io_mapped_ubuf *buf = NULL;
 
                if (ctx->buf_table.nodes[i])
@@ -228,7 +214,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
                else
                        seq_printf(m, "%5u: <none>\n", i);
        }
-       if (has_lock && !xa_empty(&ctx->personalities)) {
+       if (!xa_empty(&ctx->personalities)) {
                unsigned long index;
                const struct cred *cred;
 
@@ -238,7 +224,7 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
        }
 
        seq_puts(m, "PollList:\n");
-       for (i = 0; has_lock && i < (1U << ctx->cancel_table.hash_bits); i++) {
+       for (i = 0; i < (1U << ctx->cancel_table.hash_bits); i++) {
                struct io_hash_bucket *hb = &ctx->cancel_table.hbs[i];
                struct io_kiocb *req;
 
@@ -247,9 +233,6 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
                                        task_work_pending(req->tctx->task));
        }
 
-       if (has_lock)
-               mutex_unlock(&ctx->uring_lock);
-
        seq_puts(m, "CqOverflowList:\n");
        spin_lock(&ctx->completion_lock);
        list_for_each_entry(ocqe, &ctx->cq_overflow_list, list) {
@@ -262,4 +245,23 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
        spin_unlock(&ctx->completion_lock);
        napi_show_fdinfo(ctx, m);
 }
+
+/*
+ * Caller holds a reference to the file already, we don't need to do
+ * anything else to get an extra reference.
+ */
+__cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
+{
+       struct io_ring_ctx *ctx = file->private_data;
+
+       /*
+        * Avoid ABBA deadlock between the seq lock and the io_uring mutex,
+        * since fdinfo case grabs it in the opposite direction of normal use
+        * cases.
+        */
+       if (mutex_trylock(&ctx->uring_lock)) {
+               __io_uring_show_fdinfo(ctx, m);
+               mutex_unlock(&ctx->uring_lock);
+       }
+}
 #endif