Re: [PATCH 5.10 491/491] io_uring/poll: correctly handle io_poll_add() return value on update

Ben Hutchings
2026-04-19T15:46:02Z

[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]

On Mon, 2026-04-13 at 18:02 +0200, Greg Kroah-Hartman wrote:
> 5.10-stable review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Jens Axboe <axboe@kernel.dk>
>
> Commit 84230ad2d2afbf0c44c32967e525c0ad92e26b4e upstream.
>
> When the core of io_uring was updated to handle completions
> consistently and with fixed return codes, the POLL_REMOVE opcode
> with updates got slightly broken. If a POLL_ADD is pending and
> then POLL_REMOVE is used to update the events of that request, if that
> update causes the POLL_ADD to now trigger, then that completion is lost
> and a CQE is never posted.
>
> Additionally, ensure that if an update does cause an existing POLL_ADD
> to complete, that the completion value isn't always overwritten with
> -ECANCELED. For that case, whatever io_poll_add() set the value to
> should just be retained.

This backport is very different from the upstream version, and I have
some questions about that (inline below).

> Cc: stable@vger.kernel.org
> Fixes: 97b388d70b53 ("io_uring: handle completions in the core")
> Reported-by: syzbot+641eec6b7af1f62f2b99@syzkaller.appspotmail.com
> Tested-by: syzbot+641eec6b7af1f62f2b99@syzkaller.appspotmail.com
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> io_uring/io_uring.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -5980,7 +5980,7 @@ static int io_poll_add_prep(struct io_ki
> return 0;
> }
>
> -static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
> +static int __io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
> {
> struct io_poll_iocb *poll = &req->poll;
> struct io_poll_table ipt;
> @@ -5992,11 +5992,21 @@ static int io_poll_add(struct io_kiocb *
> if (!ret && ipt.error)
> req_set_fail(req);
> ret = ret ?: ipt.error;
> - if (ret)
> + if (ret > 0) {
> __io_req_complete(req, issue_flags, ret, 0);
> + return ret;
> + }
> return 0;
> }
>
> +static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + int ret;
> +
> + ret = __io_poll_add(req, issue_flags);
> + return ret < 0 ? ret : 0;

__io_poll_add() still never returns a negative result, so why is there a
check for that here?

> +}
> +
> static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags)
> {
> struct io_ring_ctx *ctx = req->ctx;
> @@ -6012,6 +6022,7 @@ static int io_poll_update(struct io_kioc
> ret = preq ? -EALREADY : -ENOENT;
> goto out;
> }
> + preq->result = -ECANCELED;
> spin_unlock(&ctx->completion_lock);
>
> if (req->poll_update.update_events || req->poll_update.update_user_data) {
> @@ -6024,16 +6035,17 @@ static int io_poll_update(struct io_kioc
> if (req->poll_update.update_user_data)
> preq->user_data = req->poll_update.new_user_data;
>
> - ret2 = io_poll_add(preq, issue_flags);
> + ret2 = __io_poll_add(preq, issue_flags);
> /* successfully updated, don't complete poll request */
> if (!ret2)
> goto out;
> + preq->result = ret2;
> +
> }
> - req_set_fail(preq);
> - io_req_complete(preq, -ECANCELED);
> + if (preq->result < 0)
> + req_set_fail(preq);
> + io_req_complete(preq, preq->result);

If __io_poll_add() returned an events mask then it completed preq, but
then we also complete preq here. Is that really correct?

Ben.

> out:
> - if (ret < 0)
> - req_set_fail(req);
> /* complete update request, we're done with it */
> io_req_complete(req, ret);
> io_ring_submit_unlock(ctx, !(issue_flags & IO_URING_F_NONBLOCK));
>
>

--
Ben Hutchings
Any smoothly functioning technology is indistinguishable
from a rigged demo.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]