[stgt] [PATCH 4/4] fix field length checks in spc_pr_register_and_move()

[nezhinsky at gmail.com]

From: Alexander Nezhinsky <nezhinsky at gmail.com>

REGISTER_AND_MOVE defines (SPC-3, 6.14.4) an extended (not basic) data-out format.
- It must be at least 24 bytes long, possibly extended by the Transport ID parameter.
- Transport parameter data length should be at least 24, and if greater, a multiple of 4.
- Expected data transfer length must be at least the declared parameter list length
  (this is a WR cmd, so the entire data must be transferred by the initiator).
- Parameter list length must account correctly for the transport parameter
  data length, so that no truncation occurs.

Except correcting the above checks enforcement, transport id comparison is fixed.
It was done using strncmp(), which is incorrect here, as transport id format
starts with a few binary fields potentially containing zeros which would terminate
string comparison prematurely.
memcmp() is used instead of strncmp(), and the reported id length is used instead
of the value returned by strlen().

Signed-off-by: Alexander Nezhinsky <nezhinsky at gmail.com>
---
 usr/spc.c |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/usr/spc.c b/usr/spc.c
index 0decaf3..25153d9 100644
--- a/usr/spc.c
+++ b/usr/spc.c
@@ -1418,24 +1418,26 @@ static int spc_pr_register_and_move(int host_no, struct scsi_cmd *cmd)
 {
 	uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
 	uint8_t key = ILLEGAL_REQUEST;
+	uint32_t param_list_len;
 	char *buf;
-	uint8_t unreg;
+	uint8_t unreg, aptpl;
 	uint64_t res_key, sa_res_key;
-	uint32_t addlen, idlen;
+	uint32_t tpid_data_len, idlen;
 	struct registration *reg, *dst;
-	uint16_t len = 24;
 	int (*id)(int, uint64_t, char *, int);
 	char tpid[300]; /* large enough? */
 
-	if (get_unaligned_be16(cmd->scb + 7) < len)
+	param_list_len = get_unaligned_be32(&cmd->scb[5]);
+	if (param_list_len < 24)
 		goto sense;
 
-	if (scsi_get_out_length(cmd) < len)
+	if (scsi_get_out_length(cmd) < param_list_len)
 		goto sense;
 
 	buf = scsi_get_out_buffer(cmd);
 
-	if (buf[17] & 0x01)
+	aptpl = buf[17] & 0x01;
+	if (aptpl) /* not reported in capabilities */
 		goto sense;
 
 	unreg = buf[17] & 0x02;
@@ -1443,7 +1445,11 @@ static int spc_pr_register_and_move(int host_no, struct scsi_cmd *cmd)
 	res_key = get_unaligned_be64(buf);
 	sa_res_key = get_unaligned_be64(buf + 8);
 
-	addlen = get_unaligned_be32(buf + 24);
+	tpid_data_len = get_unaligned_be32(&buf[20]);
+	if (tpid_data_len < 24 || tpid_data_len % 4 != 0)
+		goto sense;
+	if (param_list_len - 24 < tpid_data_len)
+		goto sense;
 
 	reg = lookup_registration_by_nexus(cmd->dev, cmd->it_nexus);
 	if (!reg) {
@@ -1474,10 +1480,8 @@ found:
 	if (id) {
 		memset(tpid, 0, sizeof(tpid));
 		idlen = id(cmd->dev->tgt->tid, dst->nexus_id, tpid, sizeof(tpid));
-		if (addlen) {
-			if (strncmp(tpid, buf + 28, strlen(tpid)))
-				goto sense;
-		}
+		if (tpid_data_len != idlen || memcmp(tpid, &buf[24], idlen))
+			goto sense;
 	}
 
 	cmd->dev->pr_holder = dst;
-- 
1.7.9.6

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