Hi,
Greg, can you include these in the 5.4-stable batch for the next
release? Lee reported and issue that really ended up being two
separate bugs, I fixed these last week and Lee has tested them
as good. No real upstream commits exists for these, as we fixed
them separately with refactoring and cleanup of this code.
--
Jens Axboe
From 42a9b5f649124761a4ffd260d267295056eea113 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@xxxxxxxxx>
Date: Tue, 23 May 2023 08:23:32 -0600
Subject: [PATCH 1/3] io_uring: always grab lock in io_cancel_async_work()
No upstream commit exists for this patch.
It's not necessarily safe to check the task_list locklessly, remove
this micro optimization and always grab task_lock before deeming it
empty.
Reported-and-tested-by: Lee Jones <lee@xxxxxxxxxx>
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
---
fs/io_uring.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index e8df6345a812..cc4b6635068a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3738,9 +3738,6 @@ static void io_cancel_async_work(struct io_ring_ctx *ctx,
{
struct io_kiocb *req;
- if (list_empty(&ctx->task_list))
- return;
-
spin_lock_irq(&ctx->task_lock);
list_for_each_entry(req, &ctx->task_list, task_list) {
--
2.39.2
From 66512e9596044057fe2cc173ac5c32e4fb8aed5c Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@xxxxxxxxx>
Date: Tue, 23 May 2023 08:24:31 -0600
Subject: [PATCH 2/3] io_uring: don't drop completion lock before timer is
fully initialized
No upstream commit exists for this patch.
If we drop the lock right after adding it to the timeout list, then
someone attempting to kill timeouts will find it in an indeterminate
state. That means that cancelation could attempt to cancel and remove
a timeout, and then io_timeout() proceeds to init and add the timer
afterwards.
Ensure the timeout request is fully setup before we drop the
completion lock, which guards cancelation as well.
Reported-and-tested-by: Lee Jones <lee@xxxxxxxxxx>
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
---
fs/io_uring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index cc4b6635068a..7dbc09e4c5e9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2079,12 +2079,12 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
req->sequence -= span;
add:
list_add(&req->list, entry);
- spin_unlock_irq(&ctx->completion_lock);
hrtimer_init(&req->timeout.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
req->timeout.timer.function = io_timeout_fn;
hrtimer_start(&req->timeout.timer, timespec64_to_ktime(ts),
HRTIMER_MODE_REL);
+ spin_unlock_irq(&ctx->completion_lock);
return 0;
}
--
2.39.2
From c835053c99074197d55857c6db5576a3f0ac1c08 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@xxxxxxxxx>
Date: Tue, 23 May 2023 08:26:06 -0600
Subject: [PATCH 3/3] io_uring: have io_kill_timeout() honor the request
references
No upstream commit exists for this patch.
Don't free the request unconditionally, if the request is issued async
then someone else may be holding a submit reference to it.
Reported-and-tested-by: Lee Jones <lee@xxxxxxxxxx>
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
---
fs/io_uring.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7dbc09e4c5e9..3683ddeb625a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -551,7 +551,8 @@ static void io_kill_timeout(struct io_kiocb *req)
atomic_inc(&req->ctx->cq_timeouts);
list_del(&req->list);
io_cqring_fill_event(req->ctx, req->user_data, 0);
- __io_free_req(req);
+ if (refcount_dec_and_test(&req->refs))
+ __io_free_req(req);
}
}
--
2.39.2
