On Fri, Oct 21 2016 at 4:18P -0400, Mikulas Patocka <[email protected]> wrote:
> > > On Fri, 21 Oct 2016, Mike Snitzer wrote: > > > On Fri, Oct 21 2016 at 2:33pm -0400, > > Mikulas Patocka <[email protected]> wrote: > > > > > Hi > > > > > > I found a bug in dm regarding the BLKFLSBUF ioctl. > > > > > > The BLKFLSBUF ioctl can be called on a block device and it flushes the > > > buffer cache. There is one exception - when it is called on ramdisk, it > > > actually destroys all ramdisk data (it works like a discard on the full > > > device). > > > > > > The device mapper passes this ioctl down to the underlying device, so > > > when > > > the ioctl is called on a logical volume, it can be used to destroy the > > > underlying volume group if the volume group is on ramdisk. > > > > > > For example: > > > # modprobe brd rd_size=1048576 > > > # pvcreate /dev/ram0 > > > # vgcreate ram_vg /dev/ram0 > > > # lvcreate -L 16M -n ram_lv ram_vg > > > # blockdev --flushbufs /dev/ram_vg/ram_lv > > > --- and now the whole volume group is gone, all data on the > > > ramdisk were replaced with zeroes > > > > > > The BLKFLSBUF ioctl is only allowed with CAP_SYS_ADMIN, so there > > > shouldn't > > > be security implications with this. > > > > > > Whan to do with it? The best thing would be to drop this special ramdisk > > > behavior and make the BLKFLSBUF ioctl flush the buffer cache on ramdisk > > > like on all other block devices. But there may be many users having > > > scripts that depend on this special behavior. > > > > > > Another possibility is to stop the device mapper from passing the > > > BLKFLSBUF ioctl down. > > > > If anything DM is being consistent with what the underlying device is > > meant to do. > > > > brd_ioctl() destroys the data in response to BLKFLSBUF.. I'm missing why > > this is a DM-specific problem. > > The problem is that if we call it on a logical volume, it destroys all > logical volumes on the give physical volume. Yeah, pretty awful. But this isn't a DM-specific concern. Could easily happen with normal block partitions too right? We _could_ add a DM-specific hack like this but it feels wrong to me: diff --git a/drivers/md/dm.c b/drivers/md/dm.c index ec513ee..e91607f 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -10,6 +10,8 @@ #include "dm-uevent.h" #include <linux/init.h> +#include <linux/fs.h> +#include <linux/major.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/blkpg.h> @@ -470,6 +472,16 @@ static int dm_blk_ioctl(struct block_device *bdev, fmode_t mode, * a logical partition of the parent bdev; so extra * validation is needed. */ + if (MAJOR(disk_devt(bdev->bd_disk)) == RAMDISK_MAJOR && + cmd == BLKFLSBUF) { + /* + * Disallow BLKFLSBUF on ramdisk because it can easily + * destroy data outside of this logical volume. + */ + r = -EIO; + goto out; + } + r = scsi_verify_blk_ioctl(NULL, cmd); if (r) goto out; -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
