io_uring: fix racy REQ_F_LINK_TIMEOUT clearing
authorPavel Begunkov <asml.silence@gmail.com>
Mon, 19 Oct 2020 15:39:16 +0000 (16:39 +0100)
committerJens Axboe <axboe@kernel.dk>
Mon, 19 Oct 2020 19:29:51 +0000 (13:29 -0600)
io_link_timeout_fn() removes REQ_F_LINK_TIMEOUT from the link head's
flags, it's not atomic and may race with what the head is doing.

If io_link_timeout_fn() doesn't clear the flag, as forced by this patch,
then it may happen that for "req -> link_timeout1 -> link_timeout2",
__io_kill_linked_timeout() would find link_timeout2 and try to cancel
it, so miscounting references. Teach it to ignore such double timeouts
by marking the active one with a new flag in io_prep_linked_timeout().

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
fs/io_uring.c

index bcd4439..09e7a5f 100644 (file)
@@ -585,6 +585,7 @@ enum {
        REQ_F_BUFFER_SELECTED_BIT,
        REQ_F_NO_FILE_TABLE_BIT,
        REQ_F_WORK_INITIALIZED_BIT,
+       REQ_F_LTIMEOUT_ACTIVE_BIT,
 
        /* not a real bit, just to check we're not overflowing the space */
        __REQ_F_LAST_BIT,
@@ -614,7 +615,7 @@ enum {
        REQ_F_CUR_POS           = BIT(REQ_F_CUR_POS_BIT),
        /* must not punt to workers */
        REQ_F_NOWAIT            = BIT(REQ_F_NOWAIT_BIT),
-       /* has linked timeout */
+       /* has or had linked timeout */
        REQ_F_LINK_TIMEOUT      = BIT(REQ_F_LINK_TIMEOUT_BIT),
        /* regular file */
        REQ_F_ISREG             = BIT(REQ_F_ISREG_BIT),
@@ -628,6 +629,8 @@ enum {
        REQ_F_NO_FILE_TABLE     = BIT(REQ_F_NO_FILE_TABLE_BIT),
        /* io_wq_work is initialized */
        REQ_F_WORK_INITIALIZED  = BIT(REQ_F_WORK_INITIALIZED_BIT),
+       /* linked timeout is active, i.e. prepared by link's head */
+       REQ_F_LTIMEOUT_ACTIVE   = BIT(REQ_F_LTIMEOUT_ACTIVE_BIT),
 };
 
 struct async_poll {
@@ -1871,6 +1874,12 @@ static bool __io_kill_linked_timeout(struct io_kiocb *req)
        link = list_first_entry(&req->link_list, struct io_kiocb, link_list);
        if (link->opcode != IORING_OP_LINK_TIMEOUT)
                return false;
+       /*
+        * Can happen if a linked timeout fired and link had been like
+        * req -> link t-out -> link t-out [-> ...]
+        */
+       if (!(link->flags & REQ_F_LTIMEOUT_ACTIVE))
+               return false;
 
        list_del_init(&link->link_list);
        wake_ev = io_link_cancel_timeout(link);
@@ -6106,10 +6115,9 @@ static enum hrtimer_restart io_link_timeout_fn(struct hrtimer *timer)
        if (!list_empty(&req->link_list)) {
                prev = list_entry(req->link_list.prev, struct io_kiocb,
                                  link_list);
-               if (refcount_inc_not_zero(&prev->refs)) {
+               if (refcount_inc_not_zero(&prev->refs))
                        list_del_init(&req->link_list);
-                       prev->flags &= ~REQ_F_LINK_TIMEOUT;
-               } else
+               else
                        prev = NULL;
        }
 
@@ -6166,6 +6174,7 @@ static struct io_kiocb *io_prep_linked_timeout(struct io_kiocb *req)
        if (!nxt || nxt->opcode != IORING_OP_LINK_TIMEOUT)
                return NULL;
 
+       nxt->flags |= REQ_F_LTIMEOUT_ACTIVE;
        req->flags |= REQ_F_LINK_TIMEOUT;
        return nxt;
 }