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

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data
@ 2018-11-19 21:06 David Disseldorp
  2018-11-20 16:47 ` Christoph Hellwig
  2018-11-20 17:15 ` Bart Van Assche
  0 siblings, 2 replies; 3+ messages in thread
From: David Disseldorp @ 2018-11-19 21:06 UTC (permalink / raw)
  To: target-devel

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data
  2018-11-19 21:06 [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
@ 2018-11-20 16:47 ` Christoph Hellwig
  2018-11-20 17:15 ` Bart Van Assche
  1 sibling, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2018-11-20 16:47 UTC (permalink / raw)
  To: target-devel

Looks good,

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data
  2018-11-19 21:06 [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
  2018-11-20 16:47 ` Christoph Hellwig
@ 2018-11-20 17:15 ` Bart Van Assche
  1 sibling, 0 replies; 3+ messages in thread
From: Bart Van Assche @ 2018-11-20 17:15 UTC (permalink / raw)
  To: target-devel

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-11-20 17:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-19 21:06 [PATCH v3 1/4] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
2018-11-20 16:47 ` Christoph Hellwig
2018-11-20 17:15 ` Bart Van Assche

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.