Skip to content

Commit 8cede97

Browse files
keithbuschSasha Levin
authored andcommitted
io_uring: consistently use rcu semantics with sqpoll thread
[ Upstream commit c538f40 ] The sqpoll thread is dereferenced with rcu read protection in one place, so it needs to be annotated as an __rcu type, and should consistently use rcu helpers for access and assignment to make sparse happy. Since most of the accesses occur under the sqd->lock, we can use rcu_dereference_protected() without declaring an rcu read section. Provide a simple helper to get the thread from a locked context. Fixes: ac0b8b3 ("io_uring: fix use-after-free of sq->thread in __io_uring_show_fdinfo()") Signed-off-by: Keith Busch <kbusch@kernel.org> Link: https://lore.kernel.org/r/20250611205343.1821117-1-kbusch@meta.com [axboe: fold in fix for register.c] Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 8bbf420 commit 8cede97

File tree

4 files changed

+38
-15
lines changed

4 files changed

+38
-15
lines changed

io_uring/io_uring.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2913,7 +2913,7 @@ static __cold void io_ring_exit_work(struct work_struct *work)
29132913
struct task_struct *tsk;
29142914

29152915
io_sq_thread_park(sqd);
2916-
tsk = sqd->thread;
2916+
tsk = sqpoll_task_locked(sqd);
29172917
if (tsk && tsk->io_uring && tsk->io_uring->io_wq)
29182918
io_wq_cancel_cb(tsk->io_uring->io_wq,
29192919
io_cancel_ctx_cb, ctx, true);
@@ -3150,7 +3150,7 @@ __cold void io_uring_cancel_generic(bool cancel_all, struct io_sq_data *sqd)
31503150
s64 inflight;
31513151
DEFINE_WAIT(wait);
31523152

3153-
WARN_ON_ONCE(sqd && sqd->thread != current);
3153+
WARN_ON_ONCE(sqd && sqpoll_task_locked(sqd) != current);
31543154

31553155
if (!current->io_uring)
31563156
return;

io_uring/register.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ static __cold int io_register_iowq_max_workers(struct io_ring_ctx *ctx,
273273
if (ctx->flags & IORING_SETUP_SQPOLL) {
274274
sqd = ctx->sq_data;
275275
if (sqd) {
276+
struct task_struct *tsk;
277+
276278
/*
277279
* Observe the correct sqd->lock -> ctx->uring_lock
278280
* ordering. Fine to drop uring_lock here, we hold
@@ -282,8 +284,9 @@ static __cold int io_register_iowq_max_workers(struct io_ring_ctx *ctx,
282284
mutex_unlock(&ctx->uring_lock);
283285
mutex_lock(&sqd->lock);
284286
mutex_lock(&ctx->uring_lock);
285-
if (sqd->thread)
286-
tctx = sqd->thread->io_uring;
287+
tsk = sqpoll_task_locked(sqd);
288+
if (tsk)
289+
tctx = tsk->io_uring;
287290
}
288291
} else {
289292
tctx = current->io_uring;

io_uring/sqpoll.c

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ enum {
3030
void io_sq_thread_unpark(struct io_sq_data *sqd)
3131
__releases(&sqd->lock)
3232
{
33-
WARN_ON_ONCE(sqd->thread == current);
33+
WARN_ON_ONCE(sqpoll_task_locked(sqd) == current);
3434

3535
/*
3636
* Do the dance but not conditional clear_bit() because it'd race with
@@ -46,24 +46,32 @@ void io_sq_thread_unpark(struct io_sq_data *sqd)
4646
void io_sq_thread_park(struct io_sq_data *sqd)
4747
__acquires(&sqd->lock)
4848
{
49-
WARN_ON_ONCE(data_race(sqd->thread) == current);
49+
struct task_struct *tsk;
5050

5151
atomic_inc(&sqd->park_pending);
5252
set_bit(IO_SQ_THREAD_SHOULD_PARK, &sqd->state);
5353
mutex_lock(&sqd->lock);
54-
if (sqd->thread)
55-
wake_up_process(sqd->thread);
54+
55+
tsk = sqpoll_task_locked(sqd);
56+
if (tsk) {
57+
WARN_ON_ONCE(tsk == current);
58+
wake_up_process(tsk);
59+
}
5660
}
5761

5862
void io_sq_thread_stop(struct io_sq_data *sqd)
5963
{
60-
WARN_ON_ONCE(sqd->thread == current);
64+
struct task_struct *tsk;
65+
6166
WARN_ON_ONCE(test_bit(IO_SQ_THREAD_SHOULD_STOP, &sqd->state));
6267

6368
set_bit(IO_SQ_THREAD_SHOULD_STOP, &sqd->state);
6469
mutex_lock(&sqd->lock);
65-
if (sqd->thread)
66-
wake_up_process(sqd->thread);
70+
tsk = sqpoll_task_locked(sqd);
71+
if (tsk) {
72+
WARN_ON_ONCE(tsk == current);
73+
wake_up_process(tsk);
74+
}
6775
mutex_unlock(&sqd->lock);
6876
wait_for_completion(&sqd->exited);
6977
}
@@ -486,7 +494,10 @@ __cold int io_sq_offload_create(struct io_ring_ctx *ctx,
486494
goto err_sqpoll;
487495
}
488496

489-
sqd->thread = tsk;
497+
mutex_lock(&sqd->lock);
498+
rcu_assign_pointer(sqd->thread, tsk);
499+
mutex_unlock(&sqd->lock);
500+
490501
task_to_put = get_task_struct(tsk);
491502
ret = io_uring_alloc_task_context(tsk, ctx);
492503
wake_up_new_task(tsk);
@@ -514,10 +525,13 @@ __cold int io_sqpoll_wq_cpu_affinity(struct io_ring_ctx *ctx,
514525
int ret = -EINVAL;
515526

516527
if (sqd) {
528+
struct task_struct *tsk;
529+
517530
io_sq_thread_park(sqd);
518531
/* Don't set affinity for a dying thread */
519-
if (sqd->thread)
520-
ret = io_wq_cpu_affinity(sqd->thread->io_uring, mask);
532+
tsk = sqpoll_task_locked(sqd);
533+
if (tsk)
534+
ret = io_wq_cpu_affinity(tsk->io_uring, mask);
521535
io_sq_thread_unpark(sqd);
522536
}
523537

io_uring/sqpoll.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ struct io_sq_data {
88
/* ctx's that are using this sqd */
99
struct list_head ctx_list;
1010

11-
struct task_struct *thread;
11+
struct task_struct __rcu *thread;
1212
struct wait_queue_head wait;
1313

1414
unsigned sq_thread_idle;
@@ -29,3 +29,9 @@ void io_sq_thread_unpark(struct io_sq_data *sqd);
2929
void io_put_sq_data(struct io_sq_data *sqd);
3030
void io_sqpoll_wait_sq(struct io_ring_ctx *ctx);
3131
int io_sqpoll_wq_cpu_affinity(struct io_ring_ctx *ctx, cpumask_var_t mask);
32+
33+
static inline struct task_struct *sqpoll_task_locked(struct io_sq_data *sqd)
34+
{
35+
return rcu_dereference_protected(sqd->thread,
36+
lockdep_is_held(&sqd->lock));
37+
}

0 commit comments

Comments
 (0)