* [PATCH 5.11 0/2] syzbot fixes
@ 2021-01-26 15:28 Pavel Begunkov
2021-01-26 15:28 ` [PATCH 1/2] io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE Pavel Begunkov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-01-26 15:28 UTC (permalink / raw)
To: Jens Axboe, io-uring
2/2 is a reincarnated issue found by syzbot, fixed by cancel-all-reqs
patches before.
Pavel Begunkov (2):
io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE
io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE
fs/io_uring.c | 52 ++++++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 23 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE 2021-01-26 15:28 [PATCH 5.11 0/2] syzbot fixes Pavel Begunkov @ 2021-01-26 15:28 ` Pavel Begunkov 2021-01-26 15:28 ` [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE Pavel Begunkov 2021-01-26 15:54 ` [PATCH 5.11 0/2] syzbot fixes Jens Axboe 2 siblings, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2021-01-26 15:28 UTC (permalink / raw) To: Jens Axboe, io-uring; +Cc: stable If the tctx inflight number haven't changed because of cancellation, __io_uring_task_cancel() will continue leaving the task in TASK_UNINTERRUPTIBLE state, that's not expected by __io_uring_files_cancel(). Ensure we always call finish_wait() before retrying. Cc: stable@vger.kernel.org # 5.9+ Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- fs/io_uring.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 2166c469789d..09aada153a71 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -9124,16 +9124,15 @@ void __io_uring_task_cancel(void) prepare_to_wait(&tctx->wait, &wait, TASK_UNINTERRUPTIBLE); /* - * If we've seen completions, retry. This avoids a race where - * a completion comes in before we did prepare_to_wait(). + * If we've seen completions, retry without waiting. This + * avoids a race where a completion comes in before we did + * prepare_to_wait(). */ - if (inflight != tctx_inflight(tctx)) - continue; - schedule(); + if (inflight == tctx_inflight(tctx)) + schedule(); finish_wait(&tctx->wait, &wait); } while (1); - finish_wait(&tctx->wait, &wait); atomic_dec(&tctx->in_idle); io_uring_remove_task_files(tctx); -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE 2021-01-26 15:28 [PATCH 5.11 0/2] syzbot fixes Pavel Begunkov 2021-01-26 15:28 ` [PATCH 1/2] io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE Pavel Begunkov @ 2021-01-26 15:28 ` Pavel Begunkov 2021-01-26 15:56 ` Pavel Begunkov 2021-01-26 15:54 ` [PATCH 5.11 0/2] syzbot fixes Jens Axboe 2 siblings, 1 reply; 6+ messages in thread From: Pavel Begunkov @ 2021-01-26 15:28 UTC (permalink / raw) To: Jens Axboe, io-uring; +Cc: stable, syzbot+f655445043a26a7cfab8 do not call blocking ops when !TASK_RUNNING; state=2 set at [<00000000ced9dbfc>] prepare_to_wait+0x1f4/0x3b0 kernel/sched/wait.c:262 WARNING: CPU: 1 PID: 19888 at kernel/sched/core.c:7853 __might_sleep+0xed/0x100 kernel/sched/core.c:7848 RIP: 0010:__might_sleep+0xed/0x100 kernel/sched/core.c:7848 Call Trace: __mutex_lock_common+0xc4/0x2ef0 kernel/locking/mutex.c:935 __mutex_lock kernel/locking/mutex.c:1103 [inline] mutex_lock_nested+0x1a/0x20 kernel/locking/mutex.c:1118 io_wq_submit_work+0x39a/0x720 fs/io_uring.c:6411 io_run_cancel fs/io-wq.c:856 [inline] io_wqe_cancel_pending_work fs/io-wq.c:990 [inline] io_wq_cancel_cb+0x614/0xcb0 fs/io-wq.c:1027 io_uring_cancel_files fs/io_uring.c:8874 [inline] io_uring_cancel_task_requests fs/io_uring.c:8952 [inline] __io_uring_files_cancel+0x115d/0x19e0 fs/io_uring.c:9038 io_uring_files_cancel include/linux/io_uring.h:51 [inline] do_exit+0x2e6/0x2490 kernel/exit.c:780 do_group_exit+0x168/0x2d0 kernel/exit.c:922 get_signal+0x16b5/0x2030 kernel/signal.c:2770 arch_do_signal_or_restart+0x8e/0x6a0 arch/x86/kernel/signal.c:811 handle_signal_work kernel/entry/common.c:147 [inline] exit_to_user_mode_loop kernel/entry/common.c:171 [inline] exit_to_user_mode_prepare+0xac/0x1e0 kernel/entry/common.c:201 __syscall_exit_to_user_mode_work kernel/entry/common.c:291 [inline] syscall_exit_to_user_mode+0x48/0x190 kernel/entry/common.c:302 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Rewrite io_uring_cancel_files() to mimic __io_uring_task_cancel()'s counting scheme, so it does all the heavy work before setting TASK_UNINTERRUPTIBLE. Cc: stable@vger.kernel.org # 5.9+ Reported-by: syzbot+f655445043a26a7cfab8@syzkaller.appspotmail.com Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- fs/io_uring.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 09aada153a71..f3f2b37e7021 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -8873,30 +8873,33 @@ static void io_cancel_defer_files(struct io_ring_ctx *ctx, } } +static int io_uring_count_inflight(struct io_ring_ctx *ctx, + struct task_struct *task, + struct files_struct *files) +{ + struct io_kiocb *req; + int cnt = 0; + + spin_lock_irq(&ctx->inflight_lock); + list_for_each_entry(req, &ctx->inflight_list, inflight_entry) { + if (!io_match_task(req, task, files)) + cnt++; + } + spin_unlock_irq(&ctx->inflight_lock); + return cnt; +} + static void io_uring_cancel_files(struct io_ring_ctx *ctx, struct task_struct *task, struct files_struct *files) { while (!list_empty_careful(&ctx->inflight_list)) { struct io_task_cancel cancel = { .task = task, .files = files }; - struct io_kiocb *req; DEFINE_WAIT(wait); - bool found = false; + int inflight; - spin_lock_irq(&ctx->inflight_lock); - list_for_each_entry(req, &ctx->inflight_list, inflight_entry) { - if (!io_match_task(req, task, files)) - continue; - found = true; - break; - } - if (found) - prepare_to_wait(&task->io_uring->wait, &wait, - TASK_UNINTERRUPTIBLE); - spin_unlock_irq(&ctx->inflight_lock); - - /* We need to keep going until we don't find a matching req */ - if (!found) + inflight = io_uring_count_inflight(ctx, task, files); + if (!inflight) break; io_wq_cancel_cb(ctx->io_wq, io_cancel_task_cb, &cancel, true); @@ -8905,7 +8908,11 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx, io_cqring_overflow_flush(ctx, true, task, files); /* cancellations _may_ trigger task work */ io_run_task_work(); - schedule(); + + prepare_to_wait(&task->io_uring->wait, &wait, + TASK_UNINTERRUPTIBLE); + if (inflight == io_uring_count_inflight(ctx, task, files)) + schedule(); finish_wait(&task->io_uring->wait, &wait); } } -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE 2021-01-26 15:28 ` [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE Pavel Begunkov @ 2021-01-26 15:56 ` Pavel Begunkov 2021-01-26 16:01 ` Jens Axboe 0 siblings, 1 reply; 6+ messages in thread From: Pavel Begunkov @ 2021-01-26 15:56 UTC (permalink / raw) To: Jens Axboe, io-uring; +Cc: stable, syzbot+f655445043a26a7cfab8 On 26/01/2021 15:28, Pavel Begunkov wrote: > do not call blocking ops when !TASK_RUNNING; state=2 set at > [<00000000ced9dbfc>] prepare_to_wait+0x1f4/0x3b0 > kernel/sched/wait.c:262 > WARNING: CPU: 1 PID: 19888 at kernel/sched/core.c:7853 > __might_sleep+0xed/0x100 kernel/sched/core.c:7848 > RIP: 0010:__might_sleep+0xed/0x100 kernel/sched/core.c:7848 > Call Trace: > __mutex_lock_common+0xc4/0x2ef0 kernel/locking/mutex.c:935 > __mutex_lock kernel/locking/mutex.c:1103 [inline] > mutex_lock_nested+0x1a/0x20 kernel/locking/mutex.c:1118 > io_wq_submit_work+0x39a/0x720 fs/io_uring.c:6411 > io_run_cancel fs/io-wq.c:856 [inline] > io_wqe_cancel_pending_work fs/io-wq.c:990 [inline] > io_wq_cancel_cb+0x614/0xcb0 fs/io-wq.c:1027 > io_uring_cancel_files fs/io_uring.c:8874 [inline] > io_uring_cancel_task_requests fs/io_uring.c:8952 [inline] > __io_uring_files_cancel+0x115d/0x19e0 fs/io_uring.c:9038 > io_uring_files_cancel include/linux/io_uring.h:51 [inline] > do_exit+0x2e6/0x2490 kernel/exit.c:780 > do_group_exit+0x168/0x2d0 kernel/exit.c:922 > get_signal+0x16b5/0x2030 kernel/signal.c:2770 > arch_do_signal_or_restart+0x8e/0x6a0 arch/x86/kernel/signal.c:811 > handle_signal_work kernel/entry/common.c:147 [inline] > exit_to_user_mode_loop kernel/entry/common.c:171 [inline] > exit_to_user_mode_prepare+0xac/0x1e0 kernel/entry/common.c:201 > __syscall_exit_to_user_mode_work kernel/entry/common.c:291 [inline] > syscall_exit_to_user_mode+0x48/0x190 kernel/entry/common.c:302 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Rewrite io_uring_cancel_files() to mimic __io_uring_task_cancel()'s > counting scheme, so it does all the heavy work before setting > TASK_UNINTERRUPTIBLE. > > Cc: stable@vger.kernel.org # 5.9+ > Reported-by: syzbot+f655445043a26a7cfab8@syzkaller.appspotmail.com > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > fs/io_uring.c | 41 ++++++++++++++++++++++++----------------- > 1 file changed, 24 insertions(+), 17 deletions(-) > > diff --git a/fs/io_uring.c b/fs/io_uring.c > index 09aada153a71..f3f2b37e7021 100644 > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -8873,30 +8873,33 @@ static void io_cancel_defer_files(struct io_ring_ctx *ctx, > } > } > > +static int io_uring_count_inflight(struct io_ring_ctx *ctx, > + struct task_struct *task, > + struct files_struct *files) > +{ > + struct io_kiocb *req; > + int cnt = 0; > + > + spin_lock_irq(&ctx->inflight_lock); > + list_for_each_entry(req, &ctx->inflight_list, inflight_entry) { > + if (!io_match_task(req, task, files)) This condition should be inversed. Jens, please drop it p.s. I wonder how tests didn't catch that > + cnt++; > + } > + spin_unlock_irq(&ctx->inflight_lock); > + return cnt; > +} > + > static void io_uring_cancel_files(struct io_ring_ctx *ctx, > struct task_struct *task, > struct files_struct *files) > { > while (!list_empty_careful(&ctx->inflight_list)) { > struct io_task_cancel cancel = { .task = task, .files = files }; > - struct io_kiocb *req; > DEFINE_WAIT(wait); > - bool found = false; > + int inflight; > > - spin_lock_irq(&ctx->inflight_lock); > - list_for_each_entry(req, &ctx->inflight_list, inflight_entry) { > - if (!io_match_task(req, task, files)) > - continue; > - found = true; > - break; > - } > - if (found) > - prepare_to_wait(&task->io_uring->wait, &wait, > - TASK_UNINTERRUPTIBLE); > - spin_unlock_irq(&ctx->inflight_lock); > - > - /* We need to keep going until we don't find a matching req */ > - if (!found) > + inflight = io_uring_count_inflight(ctx, task, files); > + if (!inflight) > break; > > io_wq_cancel_cb(ctx->io_wq, io_cancel_task_cb, &cancel, true); > @@ -8905,7 +8908,11 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx, > io_cqring_overflow_flush(ctx, true, task, files); > /* cancellations _may_ trigger task work */ > io_run_task_work(); > - schedule(); > + > + prepare_to_wait(&task->io_uring->wait, &wait, > + TASK_UNINTERRUPTIBLE); > + if (inflight == io_uring_count_inflight(ctx, task, files)) > + schedule(); > finish_wait(&task->io_uring->wait, &wait); > } > } > -- Pavel Begunkov ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE 2021-01-26 15:56 ` Pavel Begunkov @ 2021-01-26 16:01 ` Jens Axboe 0 siblings, 0 replies; 6+ messages in thread From: Jens Axboe @ 2021-01-26 16:01 UTC (permalink / raw) To: Pavel Begunkov, io-uring; +Cc: stable, syzbot+f655445043a26a7cfab8 On 1/26/21 8:56 AM, Pavel Begunkov wrote: > On 26/01/2021 15:28, Pavel Begunkov wrote: >> do not call blocking ops when !TASK_RUNNING; state=2 set at >> [<00000000ced9dbfc>] prepare_to_wait+0x1f4/0x3b0 >> kernel/sched/wait.c:262 >> WARNING: CPU: 1 PID: 19888 at kernel/sched/core.c:7853 >> __might_sleep+0xed/0x100 kernel/sched/core.c:7848 >> RIP: 0010:__might_sleep+0xed/0x100 kernel/sched/core.c:7848 >> Call Trace: >> __mutex_lock_common+0xc4/0x2ef0 kernel/locking/mutex.c:935 >> __mutex_lock kernel/locking/mutex.c:1103 [inline] >> mutex_lock_nested+0x1a/0x20 kernel/locking/mutex.c:1118 >> io_wq_submit_work+0x39a/0x720 fs/io_uring.c:6411 >> io_run_cancel fs/io-wq.c:856 [inline] >> io_wqe_cancel_pending_work fs/io-wq.c:990 [inline] >> io_wq_cancel_cb+0x614/0xcb0 fs/io-wq.c:1027 >> io_uring_cancel_files fs/io_uring.c:8874 [inline] >> io_uring_cancel_task_requests fs/io_uring.c:8952 [inline] >> __io_uring_files_cancel+0x115d/0x19e0 fs/io_uring.c:9038 >> io_uring_files_cancel include/linux/io_uring.h:51 [inline] >> do_exit+0x2e6/0x2490 kernel/exit.c:780 >> do_group_exit+0x168/0x2d0 kernel/exit.c:922 >> get_signal+0x16b5/0x2030 kernel/signal.c:2770 >> arch_do_signal_or_restart+0x8e/0x6a0 arch/x86/kernel/signal.c:811 >> handle_signal_work kernel/entry/common.c:147 [inline] >> exit_to_user_mode_loop kernel/entry/common.c:171 [inline] >> exit_to_user_mode_prepare+0xac/0x1e0 kernel/entry/common.c:201 >> __syscall_exit_to_user_mode_work kernel/entry/common.c:291 [inline] >> syscall_exit_to_user_mode+0x48/0x190 kernel/entry/common.c:302 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> Rewrite io_uring_cancel_files() to mimic __io_uring_task_cancel()'s >> counting scheme, so it does all the heavy work before setting >> TASK_UNINTERRUPTIBLE. >> >> Cc: stable@vger.kernel.org # 5.9+ >> Reported-by: syzbot+f655445043a26a7cfab8@syzkaller.appspotmail.com >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> --- >> fs/io_uring.c | 41 ++++++++++++++++++++++++----------------- >> 1 file changed, 24 insertions(+), 17 deletions(-) >> >> diff --git a/fs/io_uring.c b/fs/io_uring.c >> index 09aada153a71..f3f2b37e7021 100644 >> --- a/fs/io_uring.c >> +++ b/fs/io_uring.c >> @@ -8873,30 +8873,33 @@ static void io_cancel_defer_files(struct io_ring_ctx *ctx, >> } >> } >> >> +static int io_uring_count_inflight(struct io_ring_ctx *ctx, >> + struct task_struct *task, >> + struct files_struct *files) >> +{ >> + struct io_kiocb *req; >> + int cnt = 0; >> + >> + spin_lock_irq(&ctx->inflight_lock); >> + list_for_each_entry(req, &ctx->inflight_list, inflight_entry) { >> + if (!io_match_task(req, task, files)) > > This condition should be inversed. Jens, please drop it > > p.s. I wonder how tests didn't catch that Probably just make it cnt += io_match_task(req, task_files) to simplify it. But yes, it looks reversed. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5.11 0/2] syzbot fixes 2021-01-26 15:28 [PATCH 5.11 0/2] syzbot fixes Pavel Begunkov 2021-01-26 15:28 ` [PATCH 1/2] io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE Pavel Begunkov 2021-01-26 15:28 ` [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE Pavel Begunkov @ 2021-01-26 15:54 ` Jens Axboe 2 siblings, 0 replies; 6+ messages in thread From: Jens Axboe @ 2021-01-26 15:54 UTC (permalink / raw) To: Pavel Begunkov, io-uring On 1/26/21 8:28 AM, Pavel Begunkov wrote: > 2/2 is a reincarnated issue found by syzbot, fixed by cancel-all-reqs > patches before. > > Pavel Begunkov (2): > io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE > io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE > > fs/io_uring.c | 52 ++++++++++++++++++++++++++++----------------------- > 1 file changed, 29 insertions(+), 23 deletions(-) LGTM, applied. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-26 16:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-26 15:28 [PATCH 5.11 0/2] syzbot fixes Pavel Begunkov 2021-01-26 15:28 ` [PATCH 1/2] io_uring: fix __io_uring_files_cancel() with TASK_UNINTERRUPTIBLE Pavel Begunkov 2021-01-26 15:28 ` [PATCH 2/2] io_uring: fix cancellation taking mutex while TASK_UNINTERRUPTIBLE Pavel Begunkov 2021-01-26 15:56 ` Pavel Begunkov 2021-01-26 16:01 ` Jens Axboe 2021-01-26 15:54 ` [PATCH 5.11 0/2] syzbot fixes Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.