loop: use READ_ONCE() to read lo->lo_state without locking

When lo->lo_mutex is not held, direct access may read stale data. This
patch uses READ_ONCE() to read lo->lo_state and data_race() to silence
code checkers, and changes all assignments to use WRITE_ONCE().

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 272bc608e528244761f3406799f5fc16e04f1844..32a3a5b138029e65e97f331fb64577ceedbdf59c 100644 (file)
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1082,7 +1082,7 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
        /* Order wrt reading lo_state in loop_validate_file(). */
        wmb();
 
-       lo->lo_state = Lo_bound;
+       WRITE_ONCE(lo->lo_state, Lo_bound);
        if (part_shift)
                lo->lo_flags |= LO_FLAGS_PARTSCAN;
        partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
@@ -1179,7 +1179,7 @@ static void __loop_clr_fd(struct loop_device *lo)
        if (!part_shift)
                set_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
        mutex_lock(&lo->lo_mutex);
-       lo->lo_state = Lo_unbound;
+       WRITE_ONCE(lo->lo_state, Lo_unbound);
        mutex_unlock(&lo->lo_mutex);
 
        /*
@@ -1218,7 +1218,7 @@ static int loop_clr_fd(struct loop_device *lo)
 
        lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
        if (disk_openers(lo->lo_disk) == 1)
-               lo->lo_state = Lo_rundown;
+               WRITE_ONCE(lo->lo_state, Lo_rundown);
        loop_global_unlock(lo, true);
 
        return 0;
@@ -1743,7 +1743,7 @@ static void lo_release(struct gendisk *disk)
 
        mutex_lock(&lo->lo_mutex);
        if (lo->lo_state == Lo_bound && (lo->lo_flags & LO_FLAGS_AUTOCLEAR))
-               lo->lo_state = Lo_rundown;
+               WRITE_ONCE(lo->lo_state, Lo_rundown);
 
        need_clear = (lo->lo_state == Lo_rundown);
        mutex_unlock(&lo->lo_mutex);
@@ -1858,7 +1858,7 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 
        blk_mq_start_request(rq);
 
-       if (lo->lo_state != Lo_bound)
+       if (data_race(READ_ONCE(lo->lo_state)) != Lo_bound)
                return BLK_STS_IOERR;
 
        switch (req_op(rq)) {
@@ -2016,7 +2016,7 @@ static int loop_add(int i)
        lo->worker_tree = RB_ROOT;
        INIT_LIST_HEAD(&lo->idle_worker_list);
        timer_setup(&lo->timer, loop_free_idle_workers_timer, TIMER_DEFERRABLE);
-       lo->lo_state = Lo_unbound;
+       WRITE_ONCE(lo->lo_state, Lo_unbound);
 
        err = mutex_lock_killable(&loop_ctl_mutex);
        if (err)
@@ -2174,7 +2174,7 @@ static int loop_control_remove(int idx)
                goto mark_visible;
        }
        /* Mark this loop device as no more bound, but not quite unbound yet */
-       lo->lo_state = Lo_deleting;
+       WRITE_ONCE(lo->lo_state, Lo_deleting);
        mutex_unlock(&lo->lo_mutex);
 
        loop_remove(lo);
@@ -2197,8 +2197,12 @@ static int loop_control_get_free(int idx)
        if (ret)
                return ret;
        idr_for_each_entry(&loop_index_idr, lo, id) {
-               /* Hitting a race results in creating a new loop device which is harmless. */
-               if (lo->idr_visible && data_race(lo->lo_state) == Lo_unbound)
+               /*
+                * Hitting a race results in creating a new loop device
+                * which is harmless.
+                */
+               if (lo->idr_visible &&
+                   data_race(READ_ONCE(lo->lo_state)) == Lo_unbound)
                        goto found;
        }
        mutex_unlock(&loop_ctl_mutex);