linux-kernel - Re: [PATCH 1/4] loop: Refactor size calculation.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b546eff4-616a-8488-fc11-9b7e23d44bcf@acm.org>
Date:   Mon, 20 Apr 2020 06:22:12 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Martijn Coenen <maco@...roid.com>, axboe@...nel.dk, hch@....de,
        ming.lei@...hat.com
Cc:     narayan@...gle.com, zezeozue@...gle.com, kernel-team@...roid.com,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        maco@...gle.com, Chaitanya.Kulkarni@....com,
        Jaegeuk Kim <jaegeuk@...nel.org>
Subject: Re: [PATCH 1/4] loop: Refactor size calculation.

On 4/20/20 1:04 AM, Martijn Coenen wrote:
> +/**
> + * loop_set_size - sets device size and notifies userspace
> + * @lo: struct loop_device to set the size for
> + * @size: new size of the loop device
> + *
> + * Callers must validate that the size passed into this function fits into
> + * a sector_t.
> + */
> +static void loop_set_size(struct loop_device *lo, loff_t size)
> +{
> +	struct block_device *bdev = lo->lo_device;
> +
> +	set_capacity(lo->lo_disk, size);
> +	bd_set_size(bdev, size << 9);
> +	/* let user-space know about the new size */
> +	kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
> +}

How about using the SECTOR_SHIFT constant instead of "9"? I think the 
following has been added recently in include/linux/blkdev.h:

#define SECTOR_SHIFT 9

> @@ -1295,6 +1321,15 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
>   	/* I/O need to be drained during transfer transition */
>   	blk_mq_freeze_queue(lo->lo_queue);
>   
> +	if (size_changed && lo->lo_device->bd_inode->i_mapping->nrpages) {
> +		/* kill_bdev should have truncated all the pages */
> +		err = -EAGAIN;
> +		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
> +			__func__, lo->lo_number, lo->lo_file_name,
> +			lo->lo_device->bd_inode->i_mapping->nrpages);
> +		goto out_unfreeze;
> +	}
> +
>   	err = loop_release_xfer(lo);
>   	if (err)
>   		goto out_unfreeze;

Please Cc Jaegeuk for any changes in this code (see also 
https://lore.kernel.org/linux-block/20190518005304.GA19446@jaegeuk-macbookpro.roam.corp.google.com/).

Please also change the "kill_bdev should have truncated all the pages" 
comment into something like "return -EAGAIN if any pages have been 
dirtied after kill_bdev() returned".

Thanks,

Bart.