Re: initiator can write to the write-protected device in demo mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2015-09-10 at 12:16 -0700, Andy Grover wrote:
> On 09/10/2015 11:18 AM, Andy Grover wrote:
> > On 09/10/2015 03:53 AM, joeue Deng wrote:
> >> Using 'targetcli /backstores/block/ create name dev [readonly=true]' to
> >> set block with "ro write-thru" stat.
> >>
> >> however, the initiator can still write to the iscsi device.
> >>
> >> and I also find something similar,
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1036052
> >>
> >> Is this not supported current?
> >>
> >> the version i am using is :
> >>
> >> targetcli-2.1.fb37-3.el7.src.rpm
> >> python-configshell-1.1.fb14-1.el7.src.rpm
> >> python-rtslib-2.1.fb50-1.el7.src.rpm
> >
> >> targetcli-fb-devel mailing list
> >
> > [CCing target-devel for their thoughts]
> >
> > This is because iblock's readonly parameter just results in the block
> > device being opened without FMODE_WRITE flag set. If the underlying
> > block device is actually read-only then iblock will pass them down and
> > they will fail in blkdev_write_iter(), like they should.
> >
> > But if the iblock device is set read-only but the underlying device is
> > not then we get this situation, because opening without FMODE_WRITE does
> > nothing to prevent write bios from being performed.
> >
> > I see two solutions:
> >
> > 1) Change iblock to fail write/write_same/discard requests to devices it
> > has as read-only, before they get to the backing block device
> >
> > 2) Change targetcli/rtslib so read-only is not settable by the user, but
> > instead is based on the state of the backing device (by checking
> > /sys/block/<x>/ro).
> >
> > I lean towards #2. I tried #1 and it works, but the original goal (see
> > 44bfd0185) wasn't to add a read-only feature to lio exports, it was to
> > get read-only block devices working.
> 
> And of course we also have write protect on the target side.. Maybe what 
> we do is #2, and then userspace can also set write protect on tpg luns 
> if backed by a read-only block device.

After thinking about this a bit more, I'd prefer the following to
propagate up a read-only bit using se_device->dev_flags, and
subsequently override the passed lun_access in core_tpg_add_lun().

WDYT..?

diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 5a9982f..0f19e11 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -105,6 +105,8 @@ static int iblock_configure_device(struct se_device *dev)
 	mode = FMODE_READ|FMODE_EXCL;
 	if (!ib_dev->ibd_readonly)
 		mode |= FMODE_WRITE;
+	else
+		dev->dev_flags |= DF_READ_ONLY;
 
 	bd = blkdev_get_by_path(ib_dev->ibd_udev_path, mode, ib_dev);
 	if (IS_ERR(bd)) {
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 2d0381d..5fb9dd7 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -668,7 +668,10 @@ int core_tpg_add_lun(
 	list_add_tail(&lun->lun_dev_link, &dev->dev_sep_list);
 	spin_unlock(&dev->se_port_lock);
 
-	lun->lun_access = lun_access;
+	if (dev->dev_flags & DF_READ_ONLY)
+		lun->lun_access = TRANSPORT_LUNFLAGS_READ_ONLY;
+	else
+		lun->lun_access = lun_access;
 	if (!(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE))
 		hlist_add_head_rcu(&lun->link, &tpg->tpg_lun_hlist);
 	mutex_unlock(&tpg->tpg_lun_mutex);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index ac9bf1c..5f48754 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -730,6 +730,7 @@ struct se_device {
 #define DF_EMULATED_VPD_UNIT_SERIAL		0x00000004
 #define DF_USING_UDEV_PATH			0x00000008
 #define DF_USING_ALIAS				0x00000010
+#define DF_READ_ONLY				0x00000020
 	/* Physical device queue depth */
 	u32			queue_depth;
 	/* Used for SPC-2 reservations enforce of ISIDs */

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux