Commit 54891a96 authored by Yongpeng Yang's avatar Yongpeng Yang Committed by Jens Axboe
Browse files

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: default avatarDamien Le Moal <dlemoal@kernel.org>
Signed-off-by: default avatarYongpeng Yang <yangyongpeng@xiaomi.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 9869d3a6

drivers/block/loop.c

+13 −9
Original line number Diff line number Diff line
@@ -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);