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

[Fedor Pchelkin]

On Fri, 01. May 15:33, Jens Axboe wrote:
> >> @@ -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...

Oh, for the fixup patch - it is in the 5.10-queue (or at least was when I
wrote up report today).  It was taken into consideration.  The concern
for the Fixes tag is resolved now by you, thanks for clarification.

But that `if (preq->result) < 0` thing is not covered by the fixup patch.
This check is not correct in 5.10/5.15 because preq->result is unsigned.

Taken that double complete is OK (I had doubts about that), the following
change should do the job now:

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 8b0dfea96ee0..b17a26b1b5e1 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -6006,7 +6006,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags)
 {
        struct io_ring_ctx *ctx = req->ctx;
        struct io_kiocb *preq;
-       int ret2, ret = 0;
+       int ret2 = -ECANCELED, ret = 0;
 
        io_ring_submit_lock(ctx, !(issue_flags & IO_URING_F_NONBLOCK));
 
@@ -6037,7 +6037,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags)
                preq->result = ret2;
 
        }
-       if (preq->result < 0)
+       if (ret2 < 0)
                req_set_fail(preq);
        io_req_complete(preq, preq->result);
 out:


The fixup patch may be updated with this if the changes look OK.

--
Thanks,
Fedor