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

Jens Axboe
2026-04-21T22:18:51Z

On 4/19/26 9:45 AM, Ben Hutchings wrote:
> 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).

It is, was quite painful.

>> 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?

Let me take a closer look, I do agree with you that the final result
does not look entirely correct.

--
Jens Axboe