Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1206525 > unrolled thread

[PATCH] target: Fix handling of small allocation lengths in REPORT LUNS

Started bySpencer Baugh <sbaugh@catern.com>
First post2015-08-13 08:10 +0200
Last post2015-08-14 07:10 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] target: Fix handling of small allocation lengths in REPORT LUNS Spencer Baugh <sbaugh@catern.com> - 2015-08-13 08:10 +0200
    Re: [PATCH] target: Fix handling of small allocation lengths in  REPORT LUNS "Nicholas A. Bellinger" <nab@linux-iscsi.org> - 2015-08-14 01:30 +0200
      [PATCH v2] target: Fix handling of small allocation lengths in REPORT LUNS Spencer Baugh <sbaugh@catern.com> - 2015-08-14 07:10 +0200

#1206525 — [PATCH] target: Fix handling of small allocation lengths in REPORT LUNS

FromSpencer Baugh <sbaugh@catern.com>
Date2015-08-13 08:10 +0200
Subject[PATCH] target: Fix handling of small allocation lengths in REPORT LUNS
Message-ID<pWTS9-3aP-1@gated-at.bofh.it>
From: Roland Dreier <roland@purestorage.com>

REPORT LUNS should not fail just because the allocation length is less
than 16.  The relevant section of SPC-4 is:

  4.2.5.6 Allocation length

  The ALLOCATION LENGTH field specifies the maximum number of bytes or
  blocks that an application client has allocated in the Data-In
  Buffer. The ALLOCATION LENGTH field specifies bytes unless a
  different requirement is stated in the command definition.

  An allocation length of zero specifies that no data shall be
  transferred. This condition shall not be considered an error.

So we should just truncate our response rather than return an error.

Signed-off-by: Roland Dreier <roland@purestorage.com>
Signed-off-by: Spencer Baugh <sbaugh@catern.com>
---
 drivers/target/target_core_spc.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index b5ba1ec..67487e1 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -1203,17 +1203,13 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 	struct se_dev_entry *deve;
 	struct se_session *sess = cmd->se_sess;
 	struct se_node_acl *nacl;
+	struct scsi_lun slun;
 	unsigned char *buf;
 	u32 lun_count = 0, offset = 8;
-
-	if (cmd->data_length < 16) {
-		pr_warn("REPORT LUNS allocation length %u too small\n",
-			cmd->data_length);
-		return TCM_INVALID_CDB_FIELD;
-	}
+ 	__be32 len;
 
 	buf = transport_kmap_data_sg(cmd);
-	if (!buf)
+	if (cmd->data_length && !buf)
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	/*
@@ -1222,7 +1218,10 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 	 * a $FABRIC_MOD.  In that case, report LUN=0 only.
 	 */
 	if (!sess) {
-		int_to_scsilun(0, (struct scsi_lun *)&buf[offset]);
+		int_to_scsilun(0, &slun);
+		if (cmd->data_length > 8)
+			memcpy(buf + offset, &slun,
+			       min(8u, cmd->data_length - offset));
 		lun_count = 1;
 		goto done;
 	}
@@ -1236,10 +1235,12 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 		 * See SPC2-R20 7.19.
 		 */
 		lun_count++;
-		if ((offset + 8) > cmd->data_length)
+		if (offset >= cmd->data_length)
 			continue;
 
-		int_to_scsilun(deve->mapped_lun, (struct scsi_lun *)&buf[offset]);
+		int_to_scsilun(deve->mapped_lun, &slun);
+		memcpy(buf + offset, &slun,
+		       min(8u, cmd->data_length - offset));
 		offset += 8;
 	}
 	rcu_read_unlock();
@@ -1248,12 +1249,11 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 	 * See SPC3 r07, page 159.
 	 */
 done:
-	lun_count *= 8;
-	buf[0] = ((lun_count >> 24) & 0xff);
-	buf[1] = ((lun_count >> 16) & 0xff);
-	buf[2] = ((lun_count >> 8) & 0xff);
-	buf[3] = (lun_count & 0xff);
-	transport_kunmap_data_sg(cmd);
+	if (buf) {
+		len = cpu_to_be32(lun_count * 8);
+		memcpy(buf, &len, min_t(int, sizeof len, cmd->data_length));
+		transport_kunmap_data_sg(cmd);
+	}
 
 	target_complete_cmd_with_length(cmd, GOOD, 8 + lun_count * 8);
 	return 0;
-- 
2.5.0.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1207238 — Re: [PATCH] target: Fix handling of small allocation lengths in REPORT LUNS

From"Nicholas A. Bellinger" <nab@linux-iscsi.org>
Date2015-08-14 01:30 +0200
SubjectRe: [PATCH] target: Fix handling of small allocation lengths in REPORT LUNS
Message-ID<pXa6B-1d6-5@gated-at.bofh.it>
In reply to#1206525
Hi Spencer & Co,

On Wed, 2015-08-12 at 23:05 -0700, Spencer Baugh wrote:
> From: Roland Dreier <roland@purestorage.com>
> 
> REPORT LUNS should not fail just because the allocation length is less
> than 16.  The relevant section of SPC-4 is:
> 
>   4.2.5.6 Allocation length
> 
>   The ALLOCATION LENGTH field specifies the maximum number of bytes or
>   blocks that an application client has allocated in the Data-In
>   Buffer. The ALLOCATION LENGTH field specifies bytes unless a
>   different requirement is stated in the command definition.
> 
>   An allocation length of zero specifies that no data shall be
>   transferred. This condition shall not be considered an error.
> 
> So we should just truncate our response rather than return an error.
> 
> Signed-off-by: Roland Dreier <roland@purestorage.com>
> Signed-off-by: Spencer Baugh <sbaugh@catern.com>

This patch looks fine, but it currently doesn't apply to
target-pending/master code due Roland's earlier patch:

target: REPORT LUNS should return LUN 0 even for dynamic ACLs
https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?id=9c395170a559d3b23dad100b01fc4a89d661c698

Would you be so kind to respin atop current target-pending/master..?

Thank you,

--nab

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1207335 — [PATCH v2] target: Fix handling of small allocation lengths in REPORT LUNS

FromSpencer Baugh <sbaugh@catern.com>
Date2015-08-14 07:10 +0200
Subject[PATCH v2] target: Fix handling of small allocation lengths in REPORT LUNS
Message-ID<pXfpD-Ah-5@gated-at.bofh.it>
In reply to#1207238
From: Roland Dreier <roland@purestorage.com>

REPORT LUNS should not fail just because the allocation length is less
than 16.  The relevant section of SPC-4 is:

  4.2.5.6 Allocation length

  The ALLOCATION LENGTH field specifies the maximum number of bytes or
  blocks that an application client has allocated in the Data-In
  Buffer. The ALLOCATION LENGTH field specifies bytes unless a
  different requirement is stated in the command definition.

  An allocation length of zero specifies that no data shall be
  transferred. This condition shall not be considered an error.

So we should just truncate our response rather than return an error.

Signed-off-by: Roland Dreier <roland@purestorage.com>
Signed-off-by: Spencer Baugh <sbaugh@catern.com>
---
 drivers/target/target_core_spc.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index da6130a..43a27bf 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -1210,17 +1210,13 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 	struct se_dev_entry *deve;
 	struct se_session *sess = cmd->se_sess;
 	struct se_node_acl *nacl;
+	struct scsi_lun slun;
 	unsigned char *buf;
 	u32 lun_count = 0, offset = 8;
-
-	if (cmd->data_length < 16) {
-		pr_warn("REPORT LUNS allocation length %u too small\n",
-			cmd->data_length);
-		return TCM_INVALID_CDB_FIELD;
-	}
+ 	__be32 len;
 
 	buf = transport_kmap_data_sg(cmd);
-	if (!buf)
+	if (cmd->data_length && !buf)
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	/*
@@ -1241,10 +1237,12 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 		 * See SPC2-R20 7.19.
 		 */
 		lun_count++;
-		if ((offset + 8) > cmd->data_length)
+		if (offset >= cmd->data_length)
 			continue;
 
-		int_to_scsilun(deve->mapped_lun, (struct scsi_lun *)&buf[offset]);
+		int_to_scsilun(deve->mapped_lun, &slun);
+		memcpy(buf + offset, &slun,
+		       min(8u, cmd->data_length - offset));
 		offset += 8;
 	}
 	rcu_read_unlock();
@@ -1257,16 +1255,18 @@ done:
 	 * If no LUNs are accessible, report virtual LUN 0.
 	 */
 	if (lun_count == 0) {
-		int_to_scsilun(0, (struct scsi_lun *)&buf[offset]);
+		int_to_scsilun(0, &slun);
+		if (cmd->data_length > 8)
+			memcpy(buf + offset, &slun,
+			       min(8u, cmd->data_length - offset));
 		lun_count = 1;
 	}
 
-	lun_count *= 8;
-	buf[0] = ((lun_count >> 24) & 0xff);
-	buf[1] = ((lun_count >> 16) & 0xff);
-	buf[2] = ((lun_count >> 8) & 0xff);
-	buf[3] = (lun_count & 0xff);
-	transport_kunmap_data_sg(cmd);
+	if (buf) {
+		len = cpu_to_be32(lun_count * 8);
+		memcpy(buf, &len, min_t(int, sizeof len, cmd->data_length));
+		transport_kunmap_data_sg(cmd);
+	}
 
 	target_complete_cmd_with_length(cmd, GOOD, 8 + lun_count * 8);
 	return 0;
-- 
2.5.0.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web