Skip to content

Commit 2c32395

Browse files
isilenceaxboe
authored andcommitted
io_uring: fix __tctx_task_work() ctx race
There is an unlikely but possible race using a freed context. That's because req->task_work.func() can free a request, but we won't necessarily find a completion in submit_state.comp and so all ctx refs may be put by the time we do mutex_lock(&ctx->uring_ctx); There are several reasons why it can miss going through submit_state.comp: 1) req->task_work.func() didn't complete it itself, but punted to iowq (e.g. reissue) and it got freed later, or a similar situation with it overflowing and getting flushed by someone else, or being submitted to IRQ completion, 2) As we don't hold the uring_lock, someone else can do io_submit_flush_completions() and put our ref. 3) Bugs and code obscurities, e.g. failing to propagate issue_flags properly. One example is as follows CPU1 | CPU2 ======================================================================= @req->task_work.func() | -> @Req overflwed, | so submit_state.comp,nr==0 | | flush overflows, and free @Req | ctx refs == 0, free it ctx is dead, but we do | lock + flush + unlock | So take a ctx reference for each new ctx we see in __tctx_task_work(), and do release it until we do all our flushing. Fixes: 65453d1 ("io_uring: enable req cache for task_work items") Reported-by: syzbot+a157ac7c03a56397f553@syzkaller.appspotmail.com Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> [axboe: fold in my one-liner and fix ref mismatch] Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 0d30b3e commit 2c32395

1 file changed

Lines changed: 19 additions & 17 deletions

File tree

fs/io_uring.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,6 +1800,18 @@ static inline struct io_kiocb *io_req_find_next(struct io_kiocb *req)
18001800
return __io_req_find_next(req);
18011801
}
18021802

1803+
static void ctx_flush_and_put(struct io_ring_ctx *ctx)
1804+
{
1805+
if (!ctx)
1806+
return;
1807+
if (ctx->submit_state.comp.nr) {
1808+
mutex_lock(&ctx->uring_lock);
1809+
io_submit_flush_completions(&ctx->submit_state.comp, ctx);
1810+
mutex_unlock(&ctx->uring_lock);
1811+
}
1812+
percpu_ref_put(&ctx->refs);
1813+
}
1814+
18031815
static bool __tctx_task_work(struct io_uring_task *tctx)
18041816
{
18051817
struct io_ring_ctx *ctx = NULL;
@@ -1817,30 +1829,20 @@ static bool __tctx_task_work(struct io_uring_task *tctx)
18171829
node = list.first;
18181830
while (node) {
18191831
struct io_wq_work_node *next = node->next;
1820-
struct io_ring_ctx *this_ctx;
18211832
struct io_kiocb *req;
18221833

18231834
req = container_of(node, struct io_kiocb, io_task_work.node);
1824-
this_ctx = req->ctx;
1825-
req->task_work.func(&req->task_work);
1826-
node = next;
1827-
1828-
if (!ctx) {
1829-
ctx = this_ctx;
1830-
} else if (ctx != this_ctx) {
1831-
mutex_lock(&ctx->uring_lock);
1832-
io_submit_flush_completions(&ctx->submit_state.comp, ctx);
1833-
mutex_unlock(&ctx->uring_lock);
1834-
ctx = this_ctx;
1835+
if (req->ctx != ctx) {
1836+
ctx_flush_and_put(ctx);
1837+
ctx = req->ctx;
1838+
percpu_ref_get(&ctx->refs);
18351839
}
1836-
}
18371840

1838-
if (ctx && ctx->submit_state.comp.nr) {
1839-
mutex_lock(&ctx->uring_lock);
1840-
io_submit_flush_completions(&ctx->submit_state.comp, ctx);
1841-
mutex_unlock(&ctx->uring_lock);
1841+
req->task_work.func(&req->task_work);
1842+
node = next;
18421843
}
18431844

1845+
ctx_flush_and_put(ctx);
18441846
return list.first != NULL;
18451847
}
18461848

0 commit comments

Comments
 (0)