From: Caleb Sander Mateos <csander@purestorage.com>
To: Jens Axboe <axboe@kernel.dk>, Pavel Begunkov <asml.silence@gmail.com>
Cc: Riley Thomasson <riley@purestorage.com>,
io-uring@vger.kernel.org, linux-kernel@vger.kernel.org,
Caleb Sander Mateos <csander@purestorage.com>
Subject: [PATCH 2/2] io_uring/uring_cmd: switch sqe to async_data on EAGAIN
Date: Wed, 12 Feb 2025 13:45:46 -0700 [thread overview]
Message-ID: <20250212204546.3751645-3-csander@purestorage.com> (raw)
In-Reply-To: <20250212204546.3751645-1-csander@purestorage.com>
5eff57fa9f3a ("io_uring/uring_cmd: defer SQE copying until it's needed")
moved the unconditional memcpy() of the uring_cmd SQE to async_data
to 2 cases when the request goes async:
- If REQ_F_FORCE_ASYNC is set to force the initial issue to go async
- If ->uring_cmd() returns -EAGAIN in the initial non-blocking issue
Unlike the REQ_F_FORCE_ASYNC case, in the EAGAIN case, io_uring_cmd()
copies the SQE to async_data but neglects to update the io_uring_cmd's
sqe field to point to async_data. As a result, sqe still points to the
slot in the userspace-mapped SQ. At the end of io_submit_sqes(), the
kernel advances the SQ head index, allowing userspace to reuse the slot
for a new SQE. If userspace reuses the slot before the io_uring worker
reissues the original SQE, the io_uring_cmd's SQE will be corrupted.
Introduce a helper io_uring_cmd_cache_sqes() to copy the original SQE to
the io_uring_cmd's async_data and point sqe there. Use it for both the
REQ_F_FORCE_ASYNC and EAGAIN cases. This ensures the uring_cmd doesn't
read from the SQ slot after it has been returned to userspace.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Fixes: 5eff57fa9f3a ("io_uring/uring_cmd: defer SQE copying until it's needed")
---
io_uring/uring_cmd.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index cfb22e1de0e7..bcfca18395c4 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -166,10 +166,19 @@ void io_uring_cmd_done(struct io_uring_cmd *ioucmd, ssize_t ret, u64 res2,
io_req_task_work_add(req);
}
}
EXPORT_SYMBOL_GPL(io_uring_cmd_done);
+static void io_uring_cmd_cache_sqes(struct io_kiocb *req)
+{
+ struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
+ struct io_uring_cmd_data *cache = req->async_data;
+
+ memcpy(cache->sqes, ioucmd->sqe, uring_sqe_size(req->ctx));
+ ioucmd->sqe = cache->sqes;
+}
+
static int io_uring_cmd_prep_setup(struct io_kiocb *req,
const struct io_uring_sqe *sqe)
{
struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
struct io_uring_cmd_data *cache;
@@ -177,18 +186,14 @@ static int io_uring_cmd_prep_setup(struct io_kiocb *req,
cache = io_uring_alloc_async_data(&req->ctx->uring_cache, req);
if (!cache)
return -ENOMEM;
cache->op_data = NULL;
- if (!(req->flags & REQ_F_FORCE_ASYNC)) {
- /* defer memcpy until we need it */
- ioucmd->sqe = sqe;
- return 0;
- }
-
- memcpy(cache->sqes, sqe, uring_sqe_size(req->ctx));
- ioucmd->sqe = cache->sqes;
+ ioucmd->sqe = sqe;
+ /* defer memcpy until we need it */
+ if (unlikely(req->flags & REQ_F_FORCE_ASYNC))
+ io_uring_cmd_cache_sqes(req);
return 0;
}
int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
@@ -251,11 +256,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
ret = file->f_op->uring_cmd(ioucmd, issue_flags);
if (ret == -EAGAIN) {
struct io_uring_cmd_data *cache = req->async_data;
if (ioucmd->sqe != cache->sqes)
- memcpy(cache->sqes, ioucmd->sqe, uring_sqe_size(req->ctx));
+ io_uring_cmd_cache_sqes(req);
return -EAGAIN;
} else if (ret == -EIOCBQUEUED) {
return -EIOCBQUEUED;
}
--
2.45.2
next prev parent reply other threads:[~2025-02-12 20:46 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-12 20:45 [PATCH 0/2] uring_cmd SQE corruptions Caleb Sander Mateos
2025-02-12 20:45 ` [PATCH 1/2] io_uring/uring_cmd: don't assume io_uring_cmd_data layout Caleb Sander Mateos
2025-02-12 20:45 ` Caleb Sander Mateos [this message]
2025-02-12 20:55 ` [PATCH 0/2] uring_cmd SQE corruptions Jens Axboe
2025-02-12 21:02 ` Jens Axboe
2025-02-12 21:58 ` Caleb Sander
2025-02-12 22:34 ` Jens Axboe
2025-02-12 22:52 ` Caleb Sander
2025-02-12 22:56 ` Jens Axboe
2025-02-12 21:54 ` Caleb Sander
2025-02-12 22:39 ` Jens Axboe
2025-02-12 23:07 ` Caleb Sander Mateos
2025-02-12 23:21 ` Keith Busch
2025-02-12 23:46 ` Caleb Sander Mateos
2025-02-12 23:55 ` Jens Axboe
2025-02-13 16:28 ` Pavel Begunkov
2025-02-13 16:11 ` Pavel Begunkov
2025-02-13 14:48 ` Pavel Begunkov
2025-02-13 18:13 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250212204546.3751645-3-csander@purestorage.com \
--to=csander@purestorage.com \
--cc=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=riley@purestorage.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.