[PATCH 4/7] crash bug in spc_mode_sense

[Ronnie Sahlberg]

If an initiator/application is requesting mode_sense and specifying a very
small allocation_length (2,8 or so bytes) but the mode page itself
is big,  then the memcpy() in build_mode_page() will overwrite other memory
and cause tgtd to crash.

be more careful when writing to the data buffer.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
---
 usr/spc.c |   63 ++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/usr/spc.c b/usr/spc.c
index e3e4d98..6707778 100644
--- a/usr/spc.c
+++ b/usr/spc.c
@@ -335,22 +335,28 @@ int spc_test_unit(int host_no, struct scsi_cmd *cmd)
  *
  * Returns number of bytes copied.
  */
-static int build_mode_page(uint8_t *data, struct mode_pg *pg, int update)
+static uint8_t *build_mode_page(uint8_t *data, uint8_t *p, int alloc_len,
+				struct mode_pg *pg)
 {
-	uint8_t *p;
-	int len;
+	int i;
+
+	if (alloc_len > (p-data))
+		*p = pg->pcode;
+	p++;
+
+
+	if (alloc_len > (p-data))
+		*p = pg->pcode_size;
+	p++;
+

-	len = pg->pcode_size;
-	if (update) {
-		data[0] = pg->pcode;
-		data[1] = len;
+	for (i = 0; i < pg->pcode_size; i++) {
+		if (alloc_len > (p-data))
+			*p = pg->mode_data[i];
+		p++;
 	}
-	p = &data[2];
-	len += 2;
-	if (update)
-		memcpy(p, pg->mode_data, pg->pcode_size);

-	return len;
+	return p;
 }

 /**
@@ -362,12 +368,13 @@ static int build_mode_page(uint8_t *data, struct
mode_pg *pg, int update)
  */
 int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
 {
-	int len = 0;
 	uint8_t *data = NULL, *scb, mode6, dbd, pcode, subpcode;
+	uint8_t *p;
 	uint16_t alloc_len;
 	unsigned char key = ILLEGAL_REQUEST;
 	uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
 	struct mode_pg *pg;
+	int i;

 	scb = cmd->scb;
 	mode6 = (scb[0] == 0x1a);
@@ -383,10 +390,10 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)

 	if (mode6) {
 		alloc_len = scb[4];
-		len = 4;
+		p = &data[4];
 	} else {
 		alloc_len = (scb[7] << 8) + scb[8];
-		len = 8;
+		p = &data[8];
 	}

 	if (scsi_get_in_length(cmd) < alloc_len)
@@ -394,10 +401,11 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
 	memset(data, 0, alloc_len);

 	if (!dbd) {
-		if (alloc_len >= len)
-			memcpy(data + len, cmd->dev->mode_block_descriptor,
-			       BLOCK_DESCRIPTOR_LEN);
-		len += BLOCK_DESCRIPTOR_LEN;
+		for (i = 0; i < BLOCK_DESCRIPTOR_LEN; i++) {
+			if (alloc_len > (p-data))
+				*p = cmd->dev->mode_block_descriptor[i];
+			p++;
+		}
 	}

 	if (pcode == 0x3f) {
@@ -405,25 +413,26 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
 		for (i = 0; i < ARRAY_SIZE(cmd->dev->mode_pgs); i++) {
 			pg = cmd->dev->mode_pgs[i];
 			if (pg)
-				len += build_mode_page(data + len, pg,
-						       alloc_len >= len);
+				p = build_mode_page(data, p, alloc_len, pg);
 		}
 	} else {
 		pg = cmd->dev->mode_pgs[pcode];
 		if (!pg)
 			goto sense;
-		len += build_mode_page(data + len, pg, alloc_len >= len);
+		p = build_mode_page(data, p, alloc_len, pg);
 	}

 	if (mode6) {
-		data[0] = len - 1;
-		data[3] = dbd ? 0 : BLOCK_DESCRIPTOR_LEN;
+		data[0] = (p - data) - 1;
+		if (alloc_len > 3)
+			data[3] = dbd ? 0 : BLOCK_DESCRIPTOR_LEN;
 	} else {
-		*(uint16_t *)(data) = __cpu_to_be16(len - 2);
-		data[7] = dbd ? 0 : BLOCK_DESCRIPTOR_LEN;
+		*(uint16_t *)(data) = __cpu_to_be16((p - data)-2);
+		if (alloc_len > 7)
+			data[7] = dbd ? 0 : BLOCK_DESCRIPTOR_LEN;
 	}

-	scsi_set_in_resid_by_actual(cmd, len);
+	scsi_set_in_resid_by_actual(cmd, p - data);
 	return SAM_STAT_GOOD;
 sense:
 	scsi_set_in_resid_by_actual(cmd, 0);
-- 
1.5.5