Re: [PATCH v3] loop: drop caches if offset or block_size are changed

From: Jaegeuk Kim
Date: Wed Jan 09 2019 - 00:22:43 EST


Hi Jens,

May I ask whether this patch is acceptable?

Thanks,

On 12/18, 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 <axboe@xxxxxxxxx>
> Cc: linux-block@xxxxxxxxxxxxxxx
> Reported-by: Martijn Coenen <maco@xxxxxxxxxx>
> Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> ---
> v2 -> v3:
> - avoid to submit IOs on frozen mq
>
> Jens, how about this?
>
> Thanks,
>
> drivers/block/loop.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index cb0cc8685076..6b03121d62aa 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1126,6 +1126,12 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
> if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
> return -EINVAL;
>
> + if (lo->lo_offset != info->lo_offset ||
> + lo->lo_sizelimit != info->lo_sizelimit) {
> + sync_blockdev(lo->lo_device);
> + kill_bdev(lo->lo_device);
> + }
> +
> /* I/O need to be drained during transfer transition */
> blk_mq_freeze_queue(lo->lo_queue);
>
> @@ -1154,6 +1160,11 @@ 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) {
> + /* kill_bdev should have truncated all the pages */
> + if (lo->lo_device->bd_inode->i_mapping->nrpages) {
> + err = -EAGAIN;
> + goto exit;
> + }
> if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit)) {
> err = -EFBIG;
> goto exit;
> @@ -1375,22 +1386,36 @@ static int loop_set_dio(struct loop_device *lo, unsigned long arg)
>
> static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
> {
> + int err = 0;
> +
> if (lo->lo_state != Lo_bound)
> return -ENXIO;
>
> if (arg < 512 || arg > PAGE_SIZE || !is_power_of_2(arg))
> return -EINVAL;
>
> + if (lo->lo_queue->limits.logical_block_size != arg) {
> + sync_blockdev(lo->lo_device);
> + kill_bdev(lo->lo_device);
> + }
> +
> blk_mq_freeze_queue(lo->lo_queue);
>
> + /* kill_bdev should have truncated all the pages */
> + if (lo->lo_queue->limits.logical_block_size != arg &&
> + lo->lo_device->bd_inode->i_mapping->nrpages) {
> + err = -EAGAIN;
> + goto out;
> + }
> +
> blk_queue_logical_block_size(lo->lo_queue, arg);
> blk_queue_physical_block_size(lo->lo_queue, arg);
> blk_queue_io_min(lo->lo_queue, arg);
> loop_update_dio(lo);
> -
> +out:
> blk_mq_unfreeze_queue(lo->lo_queue);
>
> - return 0;
> + return err;
> }
>
> static int lo_ioctl(struct block_device *bdev, fmode_t mode,
> --
> 2.19.0.605.g01d371f741-goog