[PATCH] io_uring/net: commit all consumed buffers on bundle recv retry

[PATCH] io_uring/net: commit all consumed buffers on bundle recv retry

[Aohan Mei]

From: Aohan Mei <henrymei@tencent.com>

For a bundle receive (IORING_RECVSEND_BUNDLE) using a provided
buffer ring, the initial issue peeks N buffers and defers the
commit via REQ_F_BUFFERS_COMMIT. If MSG_WAITALL (or a short read
on a streaming socket) yields a partial result,
io_net_kbuf_recyle() commits only the buffers covering that first
partial round and clears REQ_F_BUFFERS_COMMIT. On subsequent poll
armed retries, sel.buf_list is reset to NULL while
REQ_F_BUFFER_RING prevents re-selection, so neither the retry
rounds nor the final io_recv_finish() -> io_put_kbufs() ever
commit the remaining buffers that have already been written to.

This leaves bl->head accounting for only the first partial round
rather than all consumed buffers. Buffers that already hold
received data stay visible in the ring and are handed out again
to later requests, corrupting or dropping application data.

Defer the commit entirely: do not commit in io_net_kbuf_recyle(),
keeping REQ_F_BUFFERS_COMMIT set for the final completion; recover
the buffer list on retry so the final commit has it available; and
derive the number of committed buffers from the total received
bytes rather than the final round, so a short last read cannot
under-count.

Fixes: 41b70df5b38b ("io_uring/net: commit partial buffers on retry")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
---
 io_uring/kbuf.c | 4 ++--
 io_uring/kbuf.h | 2 ++
 io_uring/net.c  | 7 ++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 7c309173dd19..c4bae8f00173 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -76,8 +76,8 @@ bool io_kbuf_commit(struct io_kiocb *req,
 	return true;
 }
 
-static inline struct io_buffer_list *io_buffer_get_list(struct io_ring_ctx *ctx,
-							unsigned int bgid)
+struct io_buffer_list *io_buffer_get_list(struct io_ring_ctx *ctx,
+					  unsigned int bgid)
 {
 	lockdep_assert_held(&ctx->uring_lock);
 
diff --git a/io_uring/kbuf.h b/io_uring/kbuf.h
index 401773e1ef80..7079f2dec9e0 100644
--- a/io_uring/kbuf.h
+++ b/io_uring/kbuf.h
@@ -71,6 +71,8 @@ struct io_br_sel io_buffer_select(struct io_kiocb *req, size_t *len,
 				  unsigned buf_group, unsigned int issue_flags);
 int io_buffers_select(struct io_kiocb *req, struct buf_sel_arg *arg,
 		      struct io_br_sel *sel, unsigned int issue_flags);
+struct io_buffer_list *io_buffer_get_list(struct io_ring_ctx *ctx,
+					  unsigned int bgid);
 int io_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
 		    struct io_br_sel *sel);
 void io_destroy_buffers(struct io_ring_ctx *ctx);
diff --git a/io_uring/net.c b/io_uring/net.c
index fbe719d86c46..bff4514c0100 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -483,8 +483,6 @@ static int io_net_kbuf_recyle(struct io_kiocb *req, struct io_buffer_list *bl,
 			      struct io_async_msghdr *kmsg, int len)
 {
 	req->flags |= REQ_F_BL_NO_RECYCLE;
-	if (req->flags & REQ_F_BUFFERS_COMMIT)
-		io_kbuf_commit(req, bl, len, io_bundle_nbufs(kmsg, len));
 	return IOU_RETRY;
 }
 
@@ -877,7 +875,8 @@ static inline bool io_recv_finish(struct io_kiocb *req,
 	if (sr->flags & IORING_RECVSEND_BUNDLE) {
 		size_t this_ret = sel->val - sr->done_io;
 
-		cflags |= io_put_kbufs(req, this_ret, sel->buf_list, io_bundle_nbufs(kmsg, this_ret));
+		cflags |= io_put_kbufs(req, this_ret, sel->buf_list,
+				       io_bundle_nbufs(kmsg, sel->val));
 		if (sr->flags & IORING_RECV_RETRY)
 			cflags = req->cqe.flags | (cflags & CQE_F_MASK);
 		if (sr->mshot_len && sel->val >= sr->mshot_len)
@@ -1221,6 +1220,8 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 			goto out_free;
 		}
 		sr->buf = NULL;
+	} else if (req->flags & REQ_F_BUFFER_RING) {
+		sel.buf_list = io_buffer_get_list(req->ctx, sr->buf_group);
 	}
 
 	kmsg->msg.msg_flags = 0;
-- 
2.43.7

Re: [PATCH] io_uring/net: commit all consumed buffers on bundle recv retry

[Gabriel Krisman Bertazi]

Aohan Mei <ljp1205831794@gmail.com> writes:

> This leaves bl->head accounting for only the first partial round
> rather than all consumed buffers. Buffers that already hold
> received data stay visible in the ring and are handed out again
> to later requests, corrupting or dropping application data.

Hi, Aohan. Do you have a reproducer for this issue?

-- 
Gabriel Krisman Bertazi