Hi,
We have an issue with drains not working due to missing copy of some
state, it's affecting 5.2/5.3/5.4. I'm attaching the patch for 5.4,
however the patch should apply to 5.2 and 5.3 as well by just removing
the last hunk. The last hunk is touching the linked code, which was
introduced with 5.4.
Can we get this queued up for stable? Thanks! Don't have an email for
Tomáš, assuming the reported-by is fine with just his name. Want to
ensure I include attribution I do have.
From: Jens Axboe <axboe@xxxxxxxxx>
Subject: [PATCH] io_uring: ensure req->submit is copied when req is deferred
There's an issue with deferred requests through drain, where if we do
need to defer, we're not copying over the sqe_submit state correctly.
This can result in using uninitialized data when we then later go and
submit the deferred request, like this check in __io_submit_sqe():
if (unlikely(s->index >= ctx->sq_entries))
return -EINVAL;
with 's' being uninitialized, we can randomly fail this check. Fix this
by copying sqe_submit state when we defer a request.
Reported-by: Andres Freund <andres@xxxxxxxxxxx>
Reported-by: Tomáš Chaloupka
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
---
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2c819c3c855d..0393545a39a7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2016,7 +2017,7 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
}
static int io_req_defer(struct io_ring_ctx *ctx, struct io_kiocb *req,
- const struct io_uring_sqe *sqe)
+ struct sqe_submit *s)
{
struct io_uring_sqe *sqe_copy;
@@ -2034,7 +2035,8 @@ static int io_req_defer(struct io_ring_ctx *ctx, struct io_kiocb *req,
return 0;
}
- memcpy(sqe_copy, sqe, sizeof(*sqe_copy));
+ memcpy(&req->submit, s, sizeof(*s));
+ memcpy(sqe_copy, s->sqe, sizeof(*sqe_copy));
req->submit.sqe = sqe_copy;
INIT_WORK(&req->work, io_sq_wq_submit_work);
@@ -2399,7 +2401,7 @@ static int io_queue_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
{
int ret;
- ret = io_req_defer(ctx, req, s->sqe);
+ ret = io_req_defer(ctx, req, s);
if (ret) {
if (ret != -EIOCBQUEUED) {
io_free_req(req);
@@ -2426,7 +2428,7 @@ static int io_queue_link_head(struct io_ring_ctx *ctx, struct io_kiocb *req,
* list.
*/
req->flags |= REQ_F_IO_DRAIN;
- ret = io_req_defer(ctx, req, s->sqe);
+ ret = io_req_defer(ctx, req, s);
if (ret) {
if (ret != -EIOCBQUEUED) {
io_free_req(req);
--
Jens Axboe
