Re: [PATCH 3/7] block: Add config option to not allow writing to mounted devices

[Jan Kara <jack@suse.cz>]

On Wed 20-12-23 11:26:38, Li Lingfeng wrote:
> > @@ -773,6 +803,10 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> >   	if (ret)
> >   		goto free_handle;
> > +	/* Blocking writes requires exclusive opener */
> > +	if (mode & BLK_OPEN_RESTRICT_WRITES && !holder)
> > +		return ERR_PTR(-EINVAL);
> > +
> >   	bdev = blkdev_get_no_open(dev);
> >   	if (!bdev) {
> >   		ret = -ENXIO;
> > @@ -800,12 +834,21 @@ struct bdev_handle *bdev_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> >   		goto abort_claiming;
> >   	if (!try_module_get(disk->fops->owner))
> >   		goto abort_claiming;
> > +	ret = -EBUSY;
> > +	if (!blkdev_open_compatible(bdev, mode))
> > +		goto abort_claiming;
> >   	if (bdev_is_partition(bdev))
> >   		ret = blkdev_get_part(bdev, mode);
> >   	else
> >   		ret = blkdev_get_whole(bdev, mode);
> >   	if (ret)
> >   		goto put_module;
> > +	if (!bdev_allow_write_mounted) {
> > +		if (mode & BLK_OPEN_RESTRICT_WRITES)
> > +			bdev_block_writes(bdev);
> 
> When a partition device is mounted, I think maybe it's better to block
> writes on the whole device at same time.
> 
> Allowing the whole device to be opened for writing when mounting a partition
> device, did you have any special considerations before?

Yes, we were considering this. But the truth is that:

a) It is *very* hard to stop all the possibilities of corrupting data on
the device - e.g. with device mapper / loop device / malicious partition
table you can construct many block devices pointing to the same storage,
you can use e.g. SG_IO to corrupt on disk data etc. So special-casing
partitions is providing little additional benefit.

b) It is difficult to then correctly handle valid cases of multiple
writeably mounted partitions on the same device - you'd need to track used
block numbers for each device which gets difficult in presence of device
mapper etc.

c) To stop filesystem crashes, it is enough to stop modifications of buffer
cache of that one block device. Because filesystems have to validate data
they are loading into buffer cache anyway to handle faulty device, fs
corruption etc.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR