[PATCH for-6.1 0/4] minor poll fixes

[PATCH for-6.1 0/4] minor poll fixes

[Pavel Begunkov]

First two patches fix poll missing events.
3 and 4 fix leaks of multishot apoll requests.

Pavel Begunkov (4):
  io_uring: update res mask in io_poll_check_events
  io_uring: fix tw losing poll events
  io_uring: fix multishot accept request leaks
  io_uring: fix multishot recv request leaks

 include/linux/io_uring.h |  3 +++
 io_uring/io_uring.c      |  2 +-
 io_uring/io_uring.h      |  4 ++--
 io_uring/net.c           | 23 +++++++++--------------
 io_uring/poll.c          | 10 ++++++++++
 5 files changed, 25 insertions(+), 17 deletions(-)

-- 
2.38.1

[PATCH for-6.1 1/4] io_uring: update res mask in io_poll_check_events

[Pavel Begunkov]

When io_poll_check_events() collides with someone attempting to queue a
task work, it'll spin for one more time. However, it'll continue to use
the mask from the first iteration instead of updating it. For example,
if the first wake up was a EPOLLIN and the second EPOLLOUT, the
userspace will not get EPOLLOUT in time.

Clear the mask for all subsequent iterations to force vfs_poll().

Cc: stable@vger.kernel.org
Fixes: aa43477b04025 ("io_uring: poll rework")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/poll.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/io_uring/poll.c b/io_uring/poll.c
index f500506984ec..90920abf91ff 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -258,6 +258,9 @@ static int io_poll_check_events(struct io_kiocb *req, bool *locked)
 				return ret;
 		}
 
+		/* force the next iteration to vfs_poll() */
+		req->cqe.res = 0;
+
 		/*
 		 * Release all references, retry if someone tried to restart
 		 * task_work while we were executing it.
-- 
2.38.1

[PATCH for-6.1 2/4] io_uring: fix tw losing poll events

[Pavel Begunkov]

We may never try to process a poll wake and its mask if there was
multiple wake ups racing for queueing up a tw. Force
io_poll_check_events() to update the mask by vfs_poll().

Cc: stable@vger.kernel.org
Fixes: aa43477b04025 ("io_uring: poll rework")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/poll.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/io_uring/poll.c b/io_uring/poll.c
index 90920abf91ff..c34019b18211 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -228,6 +228,13 @@ static int io_poll_check_events(struct io_kiocb *req, bool *locked)
 			return IOU_POLL_DONE;
 		if (v & IO_POLL_CANCEL_FLAG)
 			return -ECANCELED;
+		/*
+		 * cqe.res contains only events of the first wake up
+		 * and all others are be lost. Redo vfs_poll() to get
+		 * up to date state.
+		 */
+		if ((v & IO_POLL_REF_MASK) != 1)
+			req->cqe.res = 0;
 
 		/* the mask was stashed in __io_poll_execute */
 		if (!req->cqe.res) {
-- 
2.38.1

[PATCH for-6.1 3/4] io_uring: fix multishot accept request leaks

[Pavel Begunkov]

Having REQ_F_POLLED set doesn't guarantee that the request is
executed as a multishot from the polling path. Fortunately for us, if
the code thinks it's multishot issue when it's not, it can only ask to
skip completion so leaking the request. Use issue_flags to mark
multipoll issues.

Cc: stable@vger.kernel.org
Fixes: 390ed29b5e425 ("io_uring: add IORING_ACCEPT_MULTISHOT for accept")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/io_uring.h | 3 +++
 io_uring/io_uring.c      | 2 +-
 io_uring/io_uring.h      | 4 ++--
 io_uring/net.c           | 7 ++-----
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 43bc8a2edccf..0ded9e271523 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -16,6 +16,9 @@ enum io_uring_cmd_flags {
 	IO_URING_F_SQE128		= 4,
 	IO_URING_F_CQE32		= 8,
 	IO_URING_F_IOPOLL		= 16,
+
+	/* the request is executed from poll, it should not be freed */
+	IO_URING_F_MULTISHOT		= 32,
 };
 
 struct io_uring_cmd {
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 4a1e482747cc..8840cf3e20f2 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1768,7 +1768,7 @@ int io_poll_issue(struct io_kiocb *req, bool *locked)
 	io_tw_lock(req->ctx, locked);
 	if (unlikely(req->task->flags & PF_EXITING))
 		return -EFAULT;
-	return io_issue_sqe(req, IO_URING_F_NONBLOCK);
+	return io_issue_sqe(req, IO_URING_F_NONBLOCK|IO_URING_F_MULTISHOT);
 }
 
 struct io_wq_work *io_wq_free_work(struct io_wq_work *work)
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index e99a79f2df9b..cef5ff924e63 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -17,8 +17,8 @@ enum {
 	IOU_ISSUE_SKIP_COMPLETE	= -EIOCBQUEUED,
 
 	/*
-	 * Intended only when both REQ_F_POLLED and REQ_F_APOLL_MULTISHOT
-	 * are set to indicate to the poll runner that multishot should be
+	 * Intended only when both IO_URING_F_MULTISHOT is passed
+	 * to indicate to the poll runner that multishot should be
 	 * removed and the result is set on req->cqe.res.
 	 */
 	IOU_STOP_MULTISHOT	= -ECANCELED,
diff --git a/io_uring/net.c b/io_uring/net.c
index 15dea91625e2..a390d3ea486c 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1289,8 +1289,7 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 			 * return EAGAIN to arm the poll infra since it
 			 * has already been done
 			 */
-			if ((req->flags & IO_APOLL_MULTI_POLLED) ==
-			    IO_APOLL_MULTI_POLLED)
+			if (issue_flags & IO_URING_F_MULTISHOT)
 				ret = IOU_ISSUE_SKIP_COMPLETE;
 			return ret;
 		}
@@ -1315,9 +1314,7 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 		goto retry;
 
 	io_req_set_res(req, ret, 0);
-	if (req->flags & REQ_F_POLLED)
-		return IOU_STOP_MULTISHOT;
-	return IOU_OK;
+	return (issue_flags & IO_URING_F_MULTISHOT) ? IOU_STOP_MULTISHOT : IOU_OK;
 }
 
 int io_socket_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
-- 
2.38.1

[PATCH for-6.1 4/4] io_uring: fix multishot recv request leaks

[Pavel Begunkov]

Having REQ_F_POLLED set doesn't guarantee that the request is
executed as a multishot from the polling path. Fortunately for us, if
the code thinks it's multishot issue when it's not, it can only ask to
skip completion so leaking the request. Use issue_flags to mark
multipoll issues.

Cc: stable@vger.kernel.org
Fixes: 1300ebb20286b ("io_uring: multishot recv")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index a390d3ea486c..ab83da7e80f0 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -67,8 +67,6 @@ struct io_sr_msg {
 	struct io_kiocb 		*notif;
 };
 
-#define IO_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
-
 int io_shutdown_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_shutdown *shutdown = io_kiocb_to_cmd(req, struct io_shutdown);
@@ -591,7 +589,8 @@ static inline void io_recv_prep_retry(struct io_kiocb *req)
  * again (for multishot).
  */
 static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
-				  unsigned int cflags, bool mshot_finished)
+				  unsigned int cflags, bool mshot_finished,
+				  unsigned issue_flags)
 {
 	if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
 		io_req_set_res(req, *ret, cflags);
@@ -614,7 +613,7 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
 
 	io_req_set_res(req, *ret, cflags);
 
-	if (req->flags & REQ_F_POLLED)
+	if (issue_flags & IO_URING_F_MULTISHOT)
 		*ret = IOU_STOP_MULTISHOT;
 	else
 		*ret = IOU_OK;
@@ -773,8 +772,7 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
 	if (ret < min_ret) {
 		if (ret == -EAGAIN && force_nonblock) {
 			ret = io_setup_async_msg(req, kmsg, issue_flags);
-			if (ret == -EAGAIN && (req->flags & IO_APOLL_MULTI_POLLED) ==
-					       IO_APOLL_MULTI_POLLED) {
+			if (ret == -EAGAIN && (issue_flags & IO_URING_F_MULTISHOT)) {
 				io_kbuf_recycle(req, issue_flags);
 				return IOU_ISSUE_SKIP_COMPLETE;
 			}
@@ -803,7 +801,7 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
 	if (kmsg->msg.msg_inq)
 		cflags |= IORING_CQE_F_SOCK_NONEMPTY;
 
-	if (!io_recv_finish(req, &ret, cflags, mshot_finished))
+	if (!io_recv_finish(req, &ret, cflags, mshot_finished, issue_flags))
 		goto retry_multishot;
 
 	if (mshot_finished) {
@@ -869,7 +867,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 	ret = sock_recvmsg(sock, &msg, flags);
 	if (ret < min_ret) {
 		if (ret == -EAGAIN && force_nonblock) {
-			if ((req->flags & IO_APOLL_MULTI_POLLED) == IO_APOLL_MULTI_POLLED) {
+			if (issue_flags & IO_URING_F_MULTISHOT) {
 				io_kbuf_recycle(req, issue_flags);
 				return IOU_ISSUE_SKIP_COMPLETE;
 			}
@@ -902,7 +900,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 	if (msg.msg_inq)
 		cflags |= IORING_CQE_F_SOCK_NONEMPTY;
 
-	if (!io_recv_finish(req, &ret, cflags, ret <= 0))
+	if (!io_recv_finish(req, &ret, cflags, ret <= 0, issue_flags))
 		goto retry_multishot;
 
 	return ret;
-- 
2.38.1

Re: [PATCH for-6.1 0/4] minor poll fixes

[Jens Axboe]

On Thu, 17 Nov 2022 18:40:13 +0000, Pavel Begunkov wrote:
> First two patches fix poll missing events.
> 3 and 4 fix leaks of multishot apoll requests.
> 
> Pavel Begunkov (4):
>   io_uring: update res mask in io_poll_check_events
>   io_uring: fix tw losing poll events
>   io_uring: fix multishot accept request leaks
>   io_uring: fix multishot recv request leaks
> 
> [...]

Applied, thanks!

[1/4] io_uring: update res mask in io_poll_check_events
      (no commit info)
[2/4] io_uring: fix tw losing poll events
      (no commit info)
[3/4] io_uring: fix multishot accept request leaks
      (no commit info)
[4/4] io_uring: fix multishot recv request leaks
      (no commit info)

Best regards,
-- 
Jens Axboe

Re: [PATCH for-6.1 1/4] io_uring: update res mask in io_poll_check_events

[Gabriel Krisman Bertazi]

Pavel Begunkov <asml.silence@gmail.com> writes:

> When io_poll_check_events() collides with someone attempting to queue a
> task work, it'll spin for one more time. However, it'll continue to use
> the mask from the first iteration instead of updating it. For example,
> if the first wake up was a EPOLLIN and the second EPOLLOUT, the
> userspace will not get EPOLLOUT in time.
>
> Clear the mask for all subsequent iterations to force vfs_poll().

Do we have a reproducer for it in liburing?  Either way, I checked the
code, it looks good to me.

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

>
> Cc: stable@vger.kernel.org
> Fixes: aa43477b04025 ("io_uring: poll rework")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  io_uring/poll.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/io_uring/poll.c b/io_uring/poll.c
> index f500506984ec..90920abf91ff 100644
> --- a/io_uring/poll.c
> +++ b/io_uring/poll.c
> @@ -258,6 +258,9 @@ static int io_poll_check_events(struct io_kiocb *req, bool *locked)
>  				return ret;
>  		}
>  
> +		/* force the next iteration to vfs_poll() */
> +		req->cqe.res = 0;
> +
>  		/*
>  		 * Release all references, retry if someone tried to restart
>  		 * task_work while we were executing it.

-- 
Gabriel Krisman Bertazi