From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Snitzer Subject: Re: device mapper and the BLKFLSBUF ioctl Date: Mon, 24 Oct 2016 11:57:56 -0400 Message-ID: <20161024155756.GA48306@redhat.com> References: <20161021200022.GA12580@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Mikulas Patocka Cc: dm-devel@redhat.com, "Alasdair G. Kergon" List-Id: dm-devel.ids On Fri, Oct 21 2016 at 4:18P -0400, Mikulas Patocka wrote: > > > On Fri, 21 Oct 2016, Mike Snitzer wrote: > > > On Fri, Oct 21 2016 at 2:33pm -0400, > > Mikulas Patocka 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 +#include +#include #include #include #include @@ -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;