[PATCH 5.19 0/3] poll fixes

[PATCH 5.19 0/3] poll fixes

[Pavel Begunkov]

Several poll and apoll fixes for 5.19. I don't know if problems in 2-3
were occuring prior to "io_uring: poll rework", but let's at least
back port it to that point.

I'll also be sending another clean up series for 5.20.

Pavel Begunkov (3):
  io_uring: fail links when poll fails
  io_uring: fix wrong arm_poll error handling
  io_uring: fix double poll leak on repolling

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

-- 
2.36.1

[PATCH 5.19 1/3] io_uring: fail links when poll fails

[Pavel Begunkov]

Don't forget to cancel all linked requests of poll request when
__io_arm_poll_handler() failed.

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

diff --git a/fs/io_uring.c b/fs/io_uring.c
index dffa85d4dc7a..d5ea3c6167b5 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7405,6 +7405,8 @@ static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
 	ipt.pt._qproc = io_poll_queue_proc;
 
 	ret = __io_arm_poll_handler(req, &req->poll, &ipt, poll->events);
+	if (!ret && ipt.error)
+		req_set_fail(req);
 	ret = ret ?: ipt.error;
 	if (ret)
 		__io_req_complete(req, issue_flags, ret, 0);
-- 
2.36.1

Re: [PATCH 5.19 1/3] io_uring: fail links when poll fails

[Jens Axboe]

On 6/21/22 5:00 PM, Pavel Begunkov wrote:
> Don't forget to cancel all linked requests of poll request when
> __io_arm_poll_handler() failed.
> 
> Fixes: aa43477b04025 ("io_uring: poll rework")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  fs/io_uring.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index dffa85d4dc7a..d5ea3c6167b5 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7405,6 +7405,8 @@ static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
>  	ipt.pt._qproc = io_poll_queue_proc;
>  
>  	ret = __io_arm_poll_handler(req, &req->poll, &ipt, poll->events);
> +	if (!ret && ipt.error)
> +		req_set_fail(req);
>  	ret = ret ?: ipt.error;
>  	if (ret)
>  		__io_req_complete(req, issue_flags, ret, 0);

We could make this:

	if (!ret && ipt.error) {
		req_set_fail(req);
		ret = ipt.error;
	}

and kill that ternary, but we could also then go a bit further with the
completion. So let's just leave that for 5.20.


-- 
Jens Axboe

[PATCH 5.19 2/3] io_uring: fix wrong arm_poll error handling

[Pavel Begunkov]

Leaving ip.error set when a request was punted to task_work execution is
problematic, don't forget to clear it.

Fixes: aa43477b04025 ("io_uring: poll rework")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d5ea3c6167b5..cb719a53b8bd 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7145,6 +7145,7 @@ static int __io_arm_poll_handler(struct io_kiocb *req,
 		if (unlikely(ipt->error || !ipt->nr_entries)) {
 			poll->events |= EPOLLONESHOT;
 			req->apoll_events |= EPOLLONESHOT;
+			ipt->error = 0;
 		}
 		__io_poll_execute(req, mask, poll->events);
 		return 0;
-- 
2.36.1

[PATCH 5.19 3/3] io_uring: fix double poll leak on repolling

[Pavel Begunkov]

We have re-polling for partial IO, so a request can be polled twice. If
it used two poll entries the first time then on the second
io_arm_poll_handler() it will find the old apoll entry and NULL
kmalloc()'ed second entry, i.e. apoll->double_poll, so leaking it.

Fixes: 10c873334feba ("io_uring: allow re-poll if we made progress")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index cb719a53b8bd..5c95755619e2 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7208,6 +7208,7 @@ static int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags)
 		mask |= EPOLLEXCLUSIVE;
 	if (req->flags & REQ_F_POLLED) {
 		apoll = req->apoll;
+		kfree(apoll->double_poll);
 	} else if (!(issue_flags & IO_URING_F_UNLOCKED) &&
 		   !list_empty(&ctx->apoll_cache)) {
 		apoll = list_first_entry(&ctx->apoll_cache, struct async_poll,
-- 
2.36.1

Re: [PATCH 5.19 3/3] io_uring: fix double poll leak on repolling

[Jens Axboe]

On 6/21/22 5:00 PM, Pavel Begunkov wrote:
> We have re-polling for partial IO, so a request can be polled twice. If
> it used two poll entries the first time then on the second
> io_arm_poll_handler() it will find the old apoll entry and NULL
> kmalloc()'ed second entry, i.e. apoll->double_poll, so leaking it.
> 
> Fixes: 10c873334feba ("io_uring: allow re-poll if we made progress")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  fs/io_uring.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index cb719a53b8bd..5c95755619e2 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7208,6 +7208,7 @@ static int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags)
>  		mask |= EPOLLEXCLUSIVE;
>  	if (req->flags & REQ_F_POLLED) {
>  		apoll = req->apoll;
> +		kfree(apoll->double_poll);
>  	} else if (!(issue_flags & IO_URING_F_UNLOCKED) &&
>  		   !list_empty(&ctx->apoll_cache)) {
>  		apoll = list_first_entry(&ctx->apoll_cache, struct async_poll,

Looks good, my only question was on whether we should clear ->double_poll
here, but we do that right below so all's good.


-- 
Jens Axboe

Re: [PATCH 5.19 0/3] poll fixes

[Jens Axboe]

On 6/21/22 5:00 PM, Pavel Begunkov wrote:
> Several poll and apoll fixes for 5.19. I don't know if problems in 2-3
> were occuring prior to "io_uring: poll rework", but let's at least
> back port it to that point.
> 
> I'll also be sending another clean up series for 5.20.

Looks good to me, a few comments in the patches. As mentioned privately,
I think we should write a dummy char driver that allows us to exercise
all types of poll (single, poll, pollfree, etc) and be able to trigger
the error paths with it too. I'll give that a shot. We can probably
just make it a debugfs file or something like that, and have it be
configurable too as a debug option for test purposes.

-- 
Jens Axboe

Re: [PATCH 5.19 0/3] poll fixes

[Jens Axboe]

On Wed, 22 Jun 2022 00:00:34 +0100, Pavel Begunkov wrote:
> Several poll and apoll fixes for 5.19. I don't know if problems in 2-3
> were occuring prior to "io_uring: poll rework", but let's at least
> back port it to that point.
> 
> I'll also be sending another clean up series for 5.20.
> 
> Pavel Begunkov (3):
>   io_uring: fail links when poll fails
>   io_uring: fix wrong arm_poll error handling
>   io_uring: fix double poll leak on repolling
> 
> [...]

Applied, thanks!

[1/3] io_uring: fail links when poll fails
      commit: c487a5ad48831afa6784b368ec40d0ee50f2fe1b
[2/3] io_uring: fix wrong arm_poll error handling
      commit: 9d2ad2947a53abf5e5e6527a9eeed50a3a4cbc72
[3/3] io_uring: fix double poll leak on repolling
      commit: c0737fa9a5a5cf5a053bcc983f72d58919b997c6

Best regards,
-- 
Jens Axboe