FAILED: patch "[PATCH] io_uring: Clean up a false-positive warning from GCC 9.3.0" failed to apply to 5.10-stable tree
The patch below does not apply to the 5.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
Possible dependencies:
0d7c1153d929 ("io_uring: Clean up a false-positive warning from GCC 9.3.0") d1fd1c201d75 ("io_uring: simplify selected buf handling") 3648e5265cfa ("io_uring: move up io_put_kbuf() and io_put_rw_kbuf()") 04c76b41ca97 ("io_uring: add option to skip CQE posting") 913a571affed ("io_uring: clean cqe filling functions") 7297ce3d5944 ("io_uring: improve send/recv error handling") 54daa9b2d80a ("io_uring: correct fill events helpers types") 867f8fa5aeb7 ("io_uring: inline io_req_needs_clean()") d17e56eb4907 ("io_uring: remove struct io_completion") d886e185a128 ("io_uring: control ->async_data with a REQ_F flag") fff4e40e3094 ("io_uring: delay req queueing into compl-batch list") 51d48dab62ed ("io_uring: add more likely/unlikely() annotations") 7e3709d57651 ("io_uring: optimise kiocb layout") 30d51dd4ad20 ("io_uring: clean up buffer select") a1cdbb4cb5f7 ("io_uring: comment why inline complete calls io_clean_op()") ef05d9ebcc92 ("io_uring: kill off ->inflight_entry field") 6962980947e2 ("io_uring: restructure submit sqes to_submit checks") d9f9d2842c91 ("io_uring: reshuffle queue_sqe completion handling") f5ed3bcd5b11 ("io_uring: optimise batch completion") b3fa03fd1b17 ("io_uring: convert iopoll_completed to store_release")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 0d7c1153d9291197c1dc473cfaade77acb874b4b Mon Sep 17 00:00:00 2001 From: Alviro Iskandar Setiawan alviro.iskandar@gmail.com Date: Mon, 7 Feb 2022 21:05:33 +0700 Subject: [PATCH] io_uring: Clean up a false-positive warning from GCC 9.3.0
In io_recv(), if import_single_range() fails, the @flags variable is uninitialized, then it will goto out_free.
After the goto, the compiler doesn't know that (ret < min_ret) is always true, so it thinks the "if ((flags & MSG_WAITALL) ..." path could be taken.
The complaint comes from gcc-9 (Debian 9.3.0-22) 9.3.0: ``` fs/io_uring.c:5238 io_recvfrom() error: uninitialized symbol 'flags' ``` Fix this by bypassing the @ret and @flags check when import_single_range() fails.
Reasons: 1. import_single_range() only returns -EFAULT when it fails. 2. At that point, @flags is uninitialized and shouldn't be read.
Reported-by: kernel test robot lkp@intel.com Reported-by: Dan Carpenter dan.carpenter@oracle.com Reported-by: "Chen, Rong A" rong.a.chen@intel.com Link: https://lore.gnuweeb.org/timl/d33bb5a9-8173-f65b-f653-51fc0681c6d6@intel.com... Cc: Pavel Begunkov asml.silence@gmail.com Suggested-by: Ammar Faizi ammarfaizi2@gnuweeb.org Fixes: 7297ce3d59449de49d3c9e1f64ae25488750a1fc ("io_uring: improve send/recv error handling") Signed-off-by: Alviro Iskandar Setiawan alviro.iskandar@gmail.com Signed-off-by: Ammar Faizi ammarfaizi2@gnuweeb.org Link: https://lore.kernel.org/r/20220207140533.565411-1-ammarfaizi2@gnuweeb.org Signed-off-by: Jens Axboe axboe@kernel.dk
diff --git a/fs/io_uring.c b/fs/io_uring.c index 2e04f718319d..3445c4da0153 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5228,7 +5228,6 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags) min_ret = iov_iter_count(&msg.msg_iter);
ret = sock_recvmsg(sock, &msg, flags); -out_free: if (ret < min_ret) { if (ret == -EAGAIN && force_nonblock) return -EAGAIN; @@ -5236,9 +5235,9 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags) ret = -EINTR; req_set_fail(req); } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) { +out_free: req_set_fail(req); } - __io_req_complete(req, issue_flags, ret, io_put_kbuf(req)); return 0; }
On Sun, Jan 22, 2023 at 9:44 PM gregkh@linuxfoundation.org wrote:
The patch below does not apply to the 5.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
That uninitialized reading is living in 5.10.y branch now https://github.com/gregkh/linux/blob/v5.10.162/io_uring/io_uring.c#L4989-L50...
If this:
ret = import_single_range(RE AD, buf, sr->len, &iov, &msg.msg_iter);
fails, this one (flags & MSG_WAITALL) may read an uninitialized variable because @flags is uninitialized.
Fortunately, if import_single_range() fails, (ret < min_ret) is always true, so this:
ret < min_ret || ((flags & MSG_WAITALL)
will always short circuit. But no one tells the compiler if @ret is always less than @min_ret in that case. So it can't prove that @flags is never actually read. That still falls to undefined behavior anyway, the compiler may emit "ud2" or similar trap for that or behave randomly. IDK...
On 1/22/23 8:43 AM, Alviro Iskandar Setiawan wrote:
On Sun, Jan 22, 2023 at 9:44 PM gregkh@linuxfoundation.org wrote:
The patch below does not apply to the 5.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
That uninitialized reading is living in 5.10.y branch now https://github.com/gregkh/linux/blob/v5.10.162/io_uring/io_uring.c#L4989-L50...
If this:
ret = import_single_range(RE AD, buf, sr->len, &iov, &msg.msg_iter);
fails, this one (flags & MSG_WAITALL) may read an uninitialized variable because @flags is uninitialized.
Fortunately, if import_single_range() fails, (ret < min_ret) is always true, so this:
ret < min_ret || ((flags & MSG_WAITALL)will always short circuit. But no one tells the compiler if @ret is always less than @min_ret in that case. So it can't prove that @flags is never actually read. That still falls to undefined behavior anyway, the compiler may emit "ud2" or similar trap for that or behave randomly. IDK...
Now handled for both trees.
linux-stable-mirror@lists.linaro.org
-
Alviro Iskandar Setiawan -
gregkh＠linuxfoundation.org -
Jens Axboe