On 5/1/26 2:54 AM, Fedor Pchelkin wrote:
> Hi Jens,
> 
> the patch has some issues even after "[PATCH 2/2] io_uring/poll: fix
> backport of io_poll_add() changes" has been applied.  Please see below.
> 
> Jens Axboe wrote:
>> 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.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 97b388d70b53 ("io_uring: handle completions in the core")
> 
> This commit is not present in 5.10/5.15 in any form, to my mind.  That
> is, io_uring changes were imported in big chunks there without preserving
> upstream git history in some places but still I can't find whether the
> changes of the mentioned Fixes-commit are present in those old kernels.
> 
> So either the Fixes tag is not completely correct or this patch can just
> be reverted from 5.10/5.15 stables.

The fixes tag is wrong.

>> 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;
>> +}
>> +
>>  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);

This should all be handled in the fixup patch - yes this one ended up
being broken, but that's why there's the followup fix.

Now this is all pretty broken because some patches ended up in 5.15 and
some in 5.10 and honestly I've almost lost track at this point. Sasha
spotted some that were dropped in some broken commit from Greg. For
5.15, the two attached are what I recently asked for to be added. 5.10
should ALWAYS get the exact same patches as 5.15, because of the whole
sale backport that was done years ago. I always ask for that explicitly
in the emails. But looks like that wasn't always done...

5.10 doesn't look like it ever got what is sha
349ef5d2e7bfb292e7000e6041a984ab56eccf28 in 5.15-stable, hence the fixup
can be merged with queueing that backport.

Sigh...

-- 
Jens Axboe