linux-kernel - [PATCH] io_uring: do not recycle buffer in READV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220721131325.624788-1-dylany@fb.com>
Date:   Thu, 21 Jul 2022 06:13:25 -0700
From:   Dylan Yudaken <dylany@...com>
To:     Jens Axboe <axboe@...nel.dk>,
        Pavel Begunkov <asml.silence@...il.com>,
        <io-uring@...r.kernel.org>
CC:     <Kernel-team@...com>, <linux-kernel@...r.kernel.org>,
        Dylan Yudaken <dylany@...com>,
        Ammar Faizi <ammarfaizi2@...weeb.org>
Subject: [PATCH] io_uring: do not recycle buffer in READV

READV cannot recycle buffers as it would lose some of the data required to
reimport that buffer.

Reported-by: Ammar Faizi <ammarfaizi2@...weeb.org>
Fixes: b66e65f41426 ("io_uring: never call io_buffer_select() for a buffer re-select")
Signed-off-by: Dylan Yudaken <dylany@...com>
---

I think going forward we can probably re-enable recycling for READV, or perhaps stick
it in the opdef to make it a bit more general. However since it is late in the merge
cycle I thought the simplest approach is best.

Worth noting the initial discussed approach of stashing the data in `struct io_rw` would not
have worked as that struct is full already apparently.

Dylan

 fs/io_uring.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a01ea49f3017..b0180679584f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1737,6 +1737,14 @@ static void io_kbuf_recycle(struct io_kiocb *req, unsigned issue_flags)
 	    (req->flags & REQ_F_PARTIAL_IO))
 		return;
 
+	/*
+	 * READV uses fields in `struct io_rw` (len/addr) to stash the selected
+	 * buffer data. However if that buffer is recycled the original request
+	 * data stored in addr is lost. Therefore forbid recycling for now.
+	 */
+	if (req->opcode == IORING_OP_READV)
+		return;
+
 	/*
 	 * We don't need to recycle for REQ_F_BUFFER_RING, we can just clear
 	 * the flag and hence ensure that bl->head doesn't get incremented.

base-commit: ff6992735ade75aae3e35d16b17da1008d753d28
-- 
2.30.2