[PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data

[PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data

[David Disseldorp]

spc5r17.pdf specifies:
  4.3.1 ASCII data field requirements
  ASCII data fields shall contain only ASCII printable characters (i.e.,
  code values 20h to 7Eh) and may be terminated with one or more ASCII
  null (00h) characters.
  ASCII data fields described as being left-aligned shall have any
  unused bytes at the end of the field (i.e., highest offset) and the
  unused bytes shall be filled with ASCII space characters (20h).

LIO currently space-pads the T10 VENDOR IDENTIFICATION and PRODUCT
IDENTIFICATION fields in the standard INQUIRY data. However, the
PRODUCT REVISION LEVEL field in the standard INQUIRY data as well as the
T10 VENDOR IDENTIFICATION field in the INQUIRY Device Identification VPD
Page are zero-terminated/zero-padded.

Fix this inconsistency by using space-padding for all of the above
fields.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 drivers/target/target_core_spc.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index f459118bc11b..c37dd36ec77d 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -108,12 +108,17 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
 
 	buf[7] = 0x2; /* CmdQue=1 */
 
-	memcpy(&buf[8], "LIO-ORG ", 8);
-	memset(&buf[16], 0x20, 16);
+	/*
+	 * ASCII data fields described as being left-aligned shall have any
+	 * unused bytes at the end of the field (i.e., highest offset) and the
+	 * unused bytes shall be filled with ASCII space characters (20h).
+	 */
+	memset(&buf[8], 0x20, 8 + 16 + 4);
+	memcpy(&buf[8], "LIO-ORG", sizeof("LIO-ORG") - 1);
 	memcpy(&buf[16], dev->t10_wwn.model,
-	       min_t(size_t, strlen(dev->t10_wwn.model), 16));
+	       strnlen(dev->t10_wwn.model, 16));
 	memcpy(&buf[32], dev->t10_wwn.revision,
-	       min_t(size_t, strlen(dev->t10_wwn.revision), 4));
+	       strnlen(dev->t10_wwn.revision, 4));
 	buf[4] = 31; /* Set additional length to 31 */
 
 	return 0;
@@ -251,7 +256,9 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf)
 	buf[off] = 0x2; /* ASCII */
 	buf[off+1] = 0x1; /* T10 Vendor ID */
 	buf[off+2] = 0x0;
-	memcpy(&buf[off+4], "LIO-ORG", 8);
+	/* left align Vendor ID and pad with spaces */
+	memset(&buf[off+4], 0x20, 8);
+	memcpy(&buf[off+4], "LIO-ORG", sizeof("LIO-ORG") - 1);
 	/* Extra Byte for NULL Terminator */
 	id_len++;
 	/* Identifier Length */
-- 
2.13.7

Re: [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data

[Bart Van Assche]

On Mon, 2018-11-19 at 22:06 +-0100, David Disseldorp wrote:
+AD4 diff --git a/drivers/target/target+AF8-core+AF8-spc.c b/drivers/target/target+AF8-core+AF8-spc.c
+AD4 index f459118bc11b..c37dd36ec77d 100644
+AD4 --- a/drivers/target/target+AF8-core+AF8-spc.c
+AD4 +-+-+- b/drivers/target/target+AF8-core+AF8-spc.c
+AD4 +AEAAQA -108,12 +-108,17 +AEAAQA spc+AF8-emulate+AF8-inquiry+AF8-std(struct se+AF8-cmd +ACo-cmd, unsigned char +ACo-buf)
+AD4  
+AD4  	buf+AFs-7+AF0 +AD0 0x2+ADs /+ACo CmdQue+AD0-1 +ACo-/
+AD4  
+AD4 -	memcpy(+ACY-buf+AFs-8+AF0, +ACI-LIO-ORG +ACI, 8)+ADs
+AD4 -	memset(+ACY-buf+AFs-16+AF0, 0x20, 16)+ADs
+AD4 +-	/+ACo
+AD4 +-	 +ACo ASCII data fields described as being left-aligned shall have any
+AD4 +-	 +ACo unused bytes at the end of the field (i.e., highest offset) and the
+AD4 +-	 +ACo unused bytes shall be filled with ASCII space characters (20h).
+AD4 +-	 +ACo-/
+AD4 +-	memset(+ACY-buf+AFs-8+AF0, 0x20, 8 +- 16 +- 4)+ADs
+AD4 +-	memcpy(+ACY-buf+AFs-8+AF0, +ACI-LIO-ORG+ACI, sizeof(+ACI-LIO-ORG+ACI) - 1)+ADs
+AD4  	memcpy(+ACY-buf+AFs-16+AF0, dev-+AD4-t10+AF8-wwn.model,
+AD4 -	       min+AF8-t(size+AF8-t, strlen(dev-+AD4-t10+AF8-wwn.model), 16))+ADs
+AD4 +-	       strnlen(dev-+AD4-t10+AF8-wwn.model, 16))+ADs
+AD4  	memcpy(+ACY-buf+AFs-32+AF0, dev-+AD4-t10+AF8-wwn.revision,
+AD4 -	       min+AF8-t(size+AF8-t, strlen(dev-+AD4-t10+AF8-wwn.revision), 4))+ADs
+AD4 +-	       strnlen(dev-+AD4-t10+AF8-wwn.revision, 4))+ADs
+AD4  	buf+AFs-4+AF0 +AD0 31+ADs /+ACo Set additional length to 31 +ACo-/
+AD4  
+AD4  	return 0+ADs
+AD4 +AEAAQA -251,7 +-256,9 +AEAAQA spc+AF8-emulate+AF8-evpd+AF8-83(struct se+AF8-cmd +ACo-cmd, unsigned char +ACo-buf)
+AD4  	buf+AFs-off+AF0 +AD0 0x2+ADs /+ACo ASCII +ACo-/
+AD4  	buf+AFs-off+-1+AF0 +AD0 0x1+ADs /+ACo T10 Vendor ID +ACo-/
+AD4  	buf+AFs-off+-2+AF0 +AD0 0x0+ADs
+AD4 -	memcpy(+ACY-buf+AFs-off+-4+AF0, +ACI-LIO-ORG+ACI, 8)+ADs
+AD4 +-	/+ACo left align Vendor ID and pad with spaces +ACo-/
+AD4 +-	memset(+ACY-buf+AFs-off+-4+AF0, 0x20, 8)+ADs
+AD4 +-	memcpy(+ACY-buf+AFs-off+-4+AF0, +ACI-LIO-ORG+ACI, sizeof(+ACI-LIO-ORG+ACI) - 1)+ADs
+AD4  	/+ACo Extra Byte for NULL Terminator +ACo-/
+AD4  	id+AF8-len+-+-+ADs
+AD4  	/+ACo Identifier Length +ACo-/

A helper function that accepts a '+AFw-0'-terminated string and a field length as
input and that copies the input string and pads it with spaces would be welcome.
From the SCST source code:

	/+ACo
	 +ACo 8 byte ASCII Vendor Identification of the target
	 +ACo - left aligned.
	 +ACo-/
	scst+AF8-copy+AF8-and+AF8-fill(+ACY-buf+AFs-8+AF0, virt+AF8-dev-+AD4-t10+AF8-vend+AF8-id, 8)+ADs

	/+ACo
	 +ACo 16 byte ASCII Product Identification of the target - left
	 +ACo aligned.
	 +ACo-/
	scst+AF8-copy+AF8-and+AF8-fill(+ACY-buf+AFs-16+AF0, virt+AF8-dev-+AD4-prod+AF8-id, 16)+ADs

	/+ACo
	 +ACo 4 byte ASCII Product Revision Level of the target - left
	 +ACo aligned.
	 +ACo-/
	scst+AF8-copy+AF8-and+AF8-fill(+ACY-buf+AFs-32+AF0, virt+AF8-dev-+AD4-prod+AF8-rev+AF8-lvl, 4)+ADs

/+ACo
 +ACo Copy a zero-terminated string into a fixed-size byte array and fill the
 +ACo trailing bytes with +AEA-fill+AF8-byte.
 +ACo-/
static void scst+AF8-copy+AF8-and+AF8-fill+AF8-b(char +ACo-dst, const char +ACo-src, int len,
				 uint8+AF8-t fill+AF8-byte)
+AHs
	int cpy+AF8-len +AD0 min+AF8-t(int, strlen(src), len)+ADs

	memcpy(dst, src, cpy+AF8-len)+ADs
	memset(dst +- cpy+AF8-len, fill+AF8-byte, len - cpy+AF8-len)+ADs
+AH0

/+ACo
 +ACo Copy a zero-terminated string into a fixed-size char array and fill the
 +ACo trailing characters with spaces.
 +ACo-/
static void scst+AF8-copy+AF8-and+AF8-fill(char +ACo-dst, const char +ACo-src, int len)
+AHs
	scst+AF8-copy+AF8-and+AF8-fill+AF8-b(dst, src, len, ' ')+ADs
+AH0

Bart.