[PATCH 0/3] multishot improvements

[PATCH 0/3] multishot improvements

[Pavel Begunkov]

Fix accept with the first patch, and make multishot handling a bit
more saner. As mentioned, changes in Patch 3 will be used later to
further cleaning it up.

Pavel Begunkov (3):
  io_uring/net: fix accept multishot handling
  io_uring/net: canonise accept mshot handling
  io_uring: make io_poll_issue() sturdier

 io_uring/io_uring.c | 40 +++++++++++++++++++++++++++++++++-------
 io_uring/net.c      | 15 ++++++---------
 2 files changed, 39 insertions(+), 16 deletions(-)

-- 
2.48.1

[PATCH 1/3] io_uring/net: fix accept multishot handling

[Pavel Begunkov]

REQ_F_APOLL_MULTISHOT doesn't guarantee it's executed from the multishot
context, so a multishot accept may get executed inline, fail
io_req_post_cqe(), and ask the core code to kill the request with
-ECANCELED by returning IOU_STOP_MULTISHOT even when a socket has been
accepted and installed.

Cc: stable@vger.kernel.org
Fixes: 390ed29b5e425 ("io_uring: add IORING_ACCEPT_MULTISHOT for accept")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/net.c b/io_uring/net.c
index 000dc70d08d0..657b8f5341cf 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1718,6 +1718,8 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	}
 
 	io_req_set_res(req, ret, cflags);
+	if (!(issue_flags & IO_URING_F_MULTISHOT))
+		return IOU_OK;
 	return IOU_STOP_MULTISHOT;
 }
 
-- 
2.48.1

[PATCH 2/3] io_uring/net: canonise accept mshot handling

[Pavel Begunkov]

Use a more recongnisable pattern for mshot accept, first try to post an
mshot cqe if needed and after do terminating handling.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 657b8f5341cf..d22fa61539a3 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1689,7 +1689,6 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 		}
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
-		req_set_fail(req);
 	} else if (!fixed) {
 		fd_install(fd, file);
 		ret = fd;
@@ -1702,14 +1701,8 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	if (!arg.is_empty)
 		cflags |= IORING_CQE_F_SOCK_NONEMPTY;
 
-	if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
-		io_req_set_res(req, ret, cflags);
-		return IOU_OK;
-	}
-
-	if (ret < 0)
-		return ret;
-	if (io_req_post_cqe(req, ret, cflags | IORING_CQE_F_MORE)) {
+	if (ret >= 0 && (req->flags & REQ_F_APOLL_MULTISHOT) &&
+	    io_req_post_cqe(req, ret, cflags | IORING_CQE_F_MORE)) {
 		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || arg.is_empty == -1)
 			goto retry;
 		if (issue_flags & IO_URING_F_MULTISHOT)
@@ -1718,6 +1711,8 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	}
 
 	io_req_set_res(req, ret, cflags);
+	if (ret < 0)
+		req_set_fail(req);
 	if (!(issue_flags & IO_URING_F_MULTISHOT))
 		return IOU_OK;
 	return IOU_STOP_MULTISHOT;
-- 
2.48.1

[PATCH 3/3] io_uring: make io_poll_issue() sturdier

[Pavel Begunkov]

io_poll_issue() forwards the call to io_issue_sqe() and thus inherits
some of the handling. That's not particularly failure resistant, as for
example returning an innocently looking IOU_OK from a multishot issue
will lead to severe bugs.

Reimplement io_poll_issue() without io_issue_sqe()'s request completion
logic. Remove extra checks as we know that req->file is already set,
linked timeout are armed, and iopoll is not supported. Also cover it
with warnings for now.

The patch should be useful by itself, but it's also preparing the
codebase for other future clean ups.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 58528bf61638..a7f5794c1930 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1721,15 +1721,13 @@ static bool io_assign_file(struct io_kiocb *req, const struct io_issue_def *def,
 	return !!req->file;
 }
 
-static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
+static inline int __io_issue_sqe(struct io_kiocb *req,
+				 unsigned int issue_flags,
+				 const struct io_issue_def *def)
 {
-	const struct io_issue_def *def = &io_issue_defs[req->opcode];
 	const struct cred *creds = NULL;
 	int ret;
 
-	if (unlikely(!io_assign_file(req, def, issue_flags)))
-		return -EBADF;
-
 	if (unlikely((req->flags & REQ_F_CREDS) && req->creds != current_cred()))
 		creds = override_creds(req->creds);
 
@@ -1744,6 +1742,19 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 	if (creds)
 		revert_creds(creds);
 
+	return ret;
+}
+
+static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
+{
+	const struct io_issue_def *def = &io_issue_defs[req->opcode];
+	int ret;
+
+	if (unlikely(!io_assign_file(req, def, issue_flags)))
+		return -EBADF;
+
+	ret = __io_issue_sqe(req, issue_flags, def);
+
 	if (ret == IOU_OK) {
 		if (issue_flags & IO_URING_F_COMPLETE_DEFER)
 			io_req_complete_defer(req);
@@ -1766,9 +1777,24 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 
 int io_poll_issue(struct io_kiocb *req, io_tw_token_t tw)
 {
+	const unsigned int issue_flags = IO_URING_F_NONBLOCK |
+					 IO_URING_F_MULTISHOT |
+					 IO_URING_F_COMPLETE_DEFER;
+	int ret;
+
 	io_tw_lock(req->ctx, tw);
-	return io_issue_sqe(req, IO_URING_F_NONBLOCK|IO_URING_F_MULTISHOT|
-				 IO_URING_F_COMPLETE_DEFER);
+
+	WARN_ON_ONCE(!req->file);
+	if (WARN_ON_ONCE(req->ctx->flags & IORING_SETUP_IOPOLL))
+		return -EFAULT;
+
+	ret = __io_issue_sqe(req, issue_flags, &io_issue_defs[req->opcode]);
+
+	WARN_ON_ONCE(ret == IOU_OK);
+
+	if (ret == IOU_ISSUE_SKIP_COMPLETE)
+		ret = 0;
+	return ret;
 }
 
 struct io_wq_work *io_wq_free_work(struct io_wq_work *work)
-- 
2.48.1

Re: [PATCH 0/3] multishot improvements

[Jens Axboe]

On Sun, 23 Feb 2025 17:22:28 +0000, Pavel Begunkov wrote:
> Fix accept with the first patch, and make multishot handling a bit
> more saner. As mentioned, changes in Patch 3 will be used later to
> further cleaning it up.
> 
> Pavel Begunkov (3):
>   io_uring/net: fix accept multishot handling
>   io_uring/net: canonise accept mshot handling
>   io_uring: make io_poll_issue() sturdier
> 
> [...]

Applied, thanks!

[1/3] io_uring/net: fix accept multishot handling
      commit: 6a1ac63e91a70c9c35f2fb7ab4f0b74acb8a903c
[2/3] io_uring/net: canonise accept mshot handling
      commit: f474b328b527616d72419a23029fb8bd7e2bea52
[3/3] io_uring: make io_poll_issue() sturdier
      commit: 0243b56ffd3c69b030b90e046ebc38d5e54249e2

Best regards,
-- 
Jens Axboe