[Patch 0/0] Fix two issues with WRITE_SAME*

[Patch 0/0] Fix two issues with WRITE_SAME*

[Ronnie Sahlberg]

Fix two issues with WRITE_SAME* found using the libiscsi test suite.
When TL is set to 0 this means that the command will cover all blocks
from the specified LBA until end of device.
If DataOut is provided then it must be exactly one block in size.
(DataOut may be omitted for unmap operations)

 sbc.c |   18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

[PATCH 1/2] Validate the DataOut length in WRITE_SAME_1*

[Ronnie Sahlberg]

WRITE_SAME_1* takes either no data at all or exactly one block of data
as DataOut. Check for this and fail the cdb if the DataOut size differs.

(Some initiators use DataOut size == 0 (primarily together with unmap)
to mean "use a block all filled with 0" so that is why I think we
should be liberal and allow size==0 too here.)

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 usr/sbc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/usr/sbc.c b/usr/sbc.c
index 8dbcd78..e58a233 100644
--- a/usr/sbc.c
+++ b/usr/sbc.c
@@ -245,6 +245,7 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
 	int ret;
 	uint64_t lba;
 	uint32_t tl;
+	size_t blocksize = 1 << cmd->dev->blk_shift;
 	unsigned char key = ILLEGAL_REQUEST;
 	uint16_t asc = ASC_LUN_NOT_SUPPORTED;
 	struct scsi_lu *lu = cmd->dev;
@@ -306,6 +307,13 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
 			asc = ASC_INVALID_FIELD_IN_CDB;
 			goto sense;
 		}
+		/* Fail of DataOut is neither ==0 or ==blocksize */
+		if (scsi_get_out_length(cmd) &&
+		    blocksize != scsi_get_out_length(cmd)) {
+			key = ILLEGAL_REQUEST;
+			asc = ASC_PARAMETER_LIST_LENGTH_ERR;
+			goto sense;
+		}
 		break;
 	}
 
-- 
2.5.0

Re: [PATCH 1/2] Validate the DataOut length in WRITE_SAME_1*

[FUJITA Tomonori]

On Sun,  2 Oct 2016 09:04:01 -0700
Ronnie Sahlberg <ronniesahlberg@gmail.com> wrote:

> WRITE_SAME_1* takes either no data at all or exactly one block of data
> as DataOut. Check for this and fail the cdb if the DataOut size differs.
> 
> (Some initiators use DataOut size == 0 (primarily together with unmap)
> to mean "use a block all filled with 0" so that is why I think we
> should be liberal and allow size==0 too here.)
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  usr/sbc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Applied both, thanks!

[PATCH 2/2] WRITE_SAME_* TL==0 means all blocks from LBA until the end of device.

[Ronnie Sahlberg]

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 usr/sbc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/usr/sbc.c b/usr/sbc.c
index e58a233..779dacd 100644
--- a/usr/sbc.c
+++ b/usr/sbc.c
@@ -260,6 +260,9 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
 		goto sense;
 	}
 
+	lba = scsi_rw_offset(cmd->scb);
+	tl  = scsi_rw_count(cmd->scb);
+
 	switch (cmd->scb[0]) {
 	case READ_10:
 	case READ_12:
@@ -314,6 +317,10 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
 			asc = ASC_PARAMETER_LIST_LENGTH_ERR;
 			goto sense;
 		}
+		/* TL == 0 means all LBAs until end of device */
+		if (tl == 0)
+			tl = (lu->size >> cmd->dev->blk_shift) - lba;
+
 		break;
 	}
 
@@ -339,9 +346,6 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
 		}
 	}
 
-	lba = scsi_rw_offset(cmd->scb);
-	tl  = scsi_rw_count(cmd->scb);
-
 	/* Verify that we are not doing i/o beyond
 	   the end-of-lun */
 	if (tl) {
-- 
2.5.0