io_uring: fix race around poll update and poll triggering

Joakim reports that in some conditions he sees a multishot poll request
being canceled, and that it coincides with getting -EALREADY on
modification. As part of the poll update procedure, there's a small window
where the request is marked as canceled, and if this coincides with the
event actually triggering, then we can get a spurious -ECANCELED and
termination of the multishot request.

Don't mark the poll request as being canceled for update. We also don't
care if we race on removal unless it's a one-shot request, we can safely
updated for either case.

Fixes: b69de288e913 ("io_uring: allow events and user_data update of running poll requests")
Reported-by: Joakim Hassila <joj@mac.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2be6f3f9578fede4aa47b8e4f9838c11ad538378..c417708bc441cc24143a4a6c354376c2329041c4 100644 (file)
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5213,14 +5213,15 @@ static bool io_arm_poll_handler(struct io_kiocb *req)
 }
 
 static bool __io_poll_remove_one(struct io_kiocb *req,
-                                struct io_poll_iocb *poll)
+                                struct io_poll_iocb *poll, bool do_cancel)
 {
        bool do_complete = false;
 
        if (!poll->head)
                return false;
        spin_lock(&poll->head->lock);
-       WRITE_ONCE(poll->canceled, true);
+       if (do_cancel)
+               WRITE_ONCE(poll->canceled, true);
        if (!list_empty(&poll->wait.entry)) {
                list_del_init(&poll->wait.entry);
                do_complete = true;
@@ -5237,12 +5238,12 @@ static bool io_poll_remove_waitqs(struct io_kiocb *req)
        io_poll_remove_double(req);
 
        if (req->opcode == IORING_OP_POLL_ADD) {
-               do_complete = __io_poll_remove_one(req, &req->poll);
+               do_complete = __io_poll_remove_one(req, &req->poll, true);
        } else {
                struct async_poll *apoll = req->apoll;
 
                /* non-poll requests have submit ref still */
-               do_complete = __io_poll_remove_one(req, &apoll->poll);
+               do_complete = __io_poll_remove_one(req, &apoll->poll, true);
                if (do_complete) {
                        io_put_req(req);
                        kfree(apoll->double_poll);
@@ -5451,10 +5452,11 @@ static int io_poll_update(struct io_kiocb *req)
                ret = -EACCES;
                goto err;
        }
-       if (!__io_poll_remove_one(preq, &preq->poll)) {
-               /* in process of completing/removal */
-               ret = -EALREADY;
-               goto err;
+       if (!__io_poll_remove_one(preq, &preq->poll, false)) {
+               if (preq->poll.events & EPOLLONESHOT) {
+                       ret = -EALREADY;
+                       goto err;
+               }
        }
        /* we now have a detached poll request. reissue. */
        ret = 0;