On 12/17/18 12:42 PM, Jaegeuk Kim wrote: > If we don't drop caches used in old offset or block_size, we can get old data > from new offset/block_size, which gives unexpected data to user. > > For example, Martijn found a loopback bug in the below scenario. > 1) LOOP_SET_FD loads first two pages on loop file > 2) LOOP_SET_STATUS64 changes the offset on the loop file > 3) mount is failed due to the cached pages having wrong superblock > > Cc: Jens Axboe <[email protected]> > Cc: [email protected] > Reported-by: Martijn Coenen <[email protected]> > Signed-off-by: Jaegeuk Kim <[email protected]> > --- > > v1 to v2: > - cover block_size change > > drivers/block/loop.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index cb0cc8685076..382557c81674 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -1154,6 +1154,12 @@ loop_set_status(struct loop_device *lo, const struct > loop_info64 *info) > > if (lo->lo_offset != info->lo_offset || > lo->lo_sizelimit != info->lo_sizelimit) { > + struct block_device *bdev = lo->lo_device; > + > + /* drop stale caches used in old offset */ > + sync_blockdev(bdev); > + kill_bdev(bdev); > + > if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit)) { > err = -EFBIG; > goto exit; > @@ -1388,6 +1394,15 @@ static int loop_set_block_size(struct loop_device *lo, > unsigned long arg) > blk_queue_io_min(lo->lo_queue, arg); > loop_update_dio(lo); > > + /* Don't change the size if it is same as current */ > + if (lo->lo_queue->limits.logical_block_size != arg) { > + struct block_device *bdev = lo->lo_device; > + > + /* drop stale caches likewise set_blocksize */ > + sync_blockdev(bdev); > + kill_bdev(bdev); > + } > + > blk_mq_unfreeze_queue(lo->lo_queue); > > return 0;
Looks fine to me, my only worry would be verifying that we're fine calling sync/kill from those contexts. The queue is frozen at this point, what happens if we do need to flush out dirty data? -- Jens Axboe
