8742 potential iscsi panic

Reviewed by: Dan McDonald <danmcd@joyent.com>
Reviewed by: Richard Lowe <richlowe@richlowe.net>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Albert Lee <trisk@forkgnu.org>

3 files changed, 163 insertions, 22 deletions

diff --git a/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c b/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c
index 32fb80218d..b8d31d00c2 100644
--- a/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c
+++ b/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c
@@ -22,6 +22,7 @@
  * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
  *
  * Copyright 2014, 2015 Nexenta Systems, Inc. All rights reserved.
+ * Copyright (c) 2017, Joyent, Inc.  All rights reserved.
  */
 
 #include <sys/cpuvar.h>
@@ -124,7 +125,7 @@ static idm_status_t iscsit_init(dev_info_t *dip);
 static idm_status_t iscsit_enable_svc(iscsit_hostinfo_t *hostinfo);
 static void iscsit_disable_svc(void);
 
-static int
+static boolean_t
 iscsit_check_cmdsn_and_queue(idm_pdu_t *rx_pdu);
 
 static void
@@ -1541,8 +1542,9 @@ iscsit_xfer_scsi_data(scsi_task_t *task, stmf_data_buf_t *dbuf,
 	/*
 	 * If it's not immediate data then start the transfer
 	 */
-	ASSERT(ibuf->ibuf_is_immed == B_FALSE);
 	if (dbuf->db_flags & DB_DIRECTION_TO_RPORT) {
+		if (ibuf->ibuf_is_immed)
+			return (iscsit_idm_to_stmf(IDM_STATUS_SUCCESS));
 		/*
 		 * The DB_SEND_STATUS_GOOD flag in the STMF data buffer allows
 		 * the port provider to phase-collapse, i.e. send the status
@@ -1566,6 +1568,7 @@ iscsit_xfer_scsi_data(scsi_task_t *task, stmf_data_buf_t *dbuf,
 
 		return (iscsit_idm_to_stmf(idm_rc));
 	} else if (dbuf->db_flags & DB_DIRECTION_FROM_RPORT) {
+		ASSERT(ibuf->ibuf_is_immed == B_FALSE);
 		/* Grab the SN lock (see comment above) */
 		mutex_enter(&ict_sess->ist_sn_mutex);
 		idm_rc = idm_buf_rx_from_ini(iscsit_task->it_idm_task,
@@ -3115,8 +3118,10 @@ iscsit_cmdsn_in_window(iscsit_conn_t *ict, uint32_t cmdsn)
  * CmdSN order. So out-of-order non-immediate commands are queued up on a
  * session-wide wait queue. Duplicate commands are ignored.
  *
+ * returns B_TRUE for commands which can be executed immediately (are
+ * non-deferred), B_FALSE for cases where a command was deferred or invalid.
  */
-static int
+static boolean_t
 iscsit_check_cmdsn_and_queue(idm_pdu_t *rx_pdu)
 {
 	idm_conn_t		*ic = rx_pdu->isp_ic;
@@ -3130,9 +3135,15 @@ iscsit_check_cmdsn_and_queue(idm_pdu_t *rx_pdu)
 		DTRACE_PROBE2(immediate__cmd, iscsit_sess_t *, ist,
 		    idm_pdu_t *, rx_pdu);
 		mutex_exit(&ist->ist_sn_mutex);
-		return (ISCSIT_CMDSN_EQ_EXPCMDSN);
+		return (B_TRUE);
 	}
-	if (iscsit_sna_lt(ist->ist_expcmdsn, ntohl(hdr->cmdsn))) {
+	/*
+	 * See RFC3270 3.1.1.2: non-immediate commands outside of the
+	 * expected window (from expcmdsn to maxcmdsn, inclusive)
+	 * should be silently ignored.
+	 */
+	if (iscsit_sna_lt(ist->ist_expcmdsn, ntohl(hdr->cmdsn)) &&
+	    iscsit_sna_lt(ntohl(hdr->cmdsn), ist->ist_maxcmdsn)) {
 		/*
 		 * Out-of-order commands (cmdSN higher than ExpCmdSN)
 		 * are staged on a fixed-size circular buffer until
@@ -3144,15 +3155,24 @@ iscsit_check_cmdsn_and_queue(idm_pdu_t *rx_pdu)
 		rx_pdu->isp_queue_time = gethrtime();
 		iscsit_add_pdu_to_queue(ist, rx_pdu);
 		mutex_exit(&ist->ist_sn_mutex);
-		return (ISCSIT_CMDSN_GT_EXPCMDSN);
-	} else if (iscsit_sna_lt(ntohl(hdr->cmdsn), ist->ist_expcmdsn)) {
+		return (B_FALSE);
+	} else if (iscsit_sna_lt(ntohl(hdr->cmdsn), ist->ist_expcmdsn) ||
+	    iscsit_sna_lt(ist->ist_maxcmdsn, ntohl(hdr->cmdsn))) {
+		/*
+		 * See above, this command is outside of our acceptable
+		 * window, we need to discard/complete.
+		 */
 		DTRACE_PROBE3(cmdsn__lt__expcmdsn, iscsit_sess_t *, ist,
 		    iscsit_conn_t *, ict, idm_pdu_t *, rx_pdu);
 		mutex_exit(&ist->ist_sn_mutex);
-		return (ISCSIT_CMDSN_LT_EXPCMDSN);
+		idm_pdu_complete(rx_pdu, IDM_STATUS_SUCCESS);
+		/*
+		 * tell our callers that the PDU "finished."
+		 */
+		return (B_FALSE);
 	} else {
 		mutex_exit(&ist->ist_sn_mutex);
-		return (ISCSIT_CMDSN_EQ_EXPCMDSN);
+		return (B_TRUE);
 	}
 }
 
diff --git a/usr/src/uts/common/io/comstar/port/iscsit/iscsit.h b/usr/src/uts/common/io/comstar/port/iscsit/iscsit.h
index b6e6b1183a..4718e98e77 100644
--- a/usr/src/uts/common/io/comstar/port/iscsit/iscsit.h
+++ b/usr/src/uts/common/io/comstar/port/iscsit/iscsit.h
@@ -22,6 +22,7 @@
 /*
  * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright (c) 2017, Joyent, Inc.  All rights reserved.
  */
 
 #ifndef _ISCSIT_H_
@@ -61,9 +62,6 @@
 #define	ISCSIT_MAX_WINDOW	1024
 #define	ISCSIT_RXPDU_QUEUE_LEN	2048
 
-#define	ISCSIT_CMDSN_LT_EXPCMDSN	-1
-#define	ISCSIT_CMDSN_EQ_EXPCMDSN	1
-#define	ISCSIT_CMDSN_GT_EXPCMDSN	0
 /*
  * MC/S: A timeout is maintained to recover from lost CmdSN (holes in the
  * CmdSN ordering). When the timeout is reached, the ExpCmdSN is advanced
diff --git a/usr/src/uts/common/io/idm/idm_so.c b/usr/src/uts/common/io/idm/idm_so.c
index 61fd0df123..e2d29e55ef 100644
--- a/usr/src/uts/common/io/idm/idm_so.c
+++ b/usr/src/uts/common/io/idm/idm_so.c
@@ -24,6 +24,7 @@
  */
 /*
  * Copyright (c) 2013 by Delphix. All rights reserved.
+ * Copyright (c) 2017, Joyent, Inc.  All rights reserved.
  */
 
 #include <sys/conf.h>
@@ -732,6 +733,103 @@ n2h24(const uchar_t *ptr)
 	return ((ptr[0] << 16) | (ptr[1] << 8) | ptr[2]);
 }
 
+static boolean_t
+idm_dataseglenokay(idm_conn_t *ic, idm_pdu_t *pdu)
+{
+	iscsi_hdr_t	*bhs;
+
+	if (ic->ic_conn_type == CONN_TYPE_TGT &&
+	    pdu->isp_datalen > ic->ic_conn_params.max_recv_dataseglen) {
+		IDM_CONN_LOG(CE_WARN,
+		    "idm_dataseglenokay: exceeded the max data segment length");
+		return (B_FALSE);
+	}
+
+	bhs = pdu->isp_hdr;
+	/*
+	 * Filter out any RFC3720 data-size violations.
+	 */
+	switch (IDM_PDU_OPCODE(pdu)) {
+	case ISCSI_OP_SCSI_TASK_MGT_MSG:
+	case ISCSI_OP_SCSI_TASK_MGT_RSP:
+	case ISCSI_OP_RTT_RSP:
+	case ISCSI_OP_LOGOUT_CMD:
+		/*
+		 * Data-segment not allowed and additional headers not allowed.
+		 * (both must be zero according to the RFC3720.)
+		 */
+		if (bhs->hlength != 0 || pdu->isp_datalen != 0)
+			return (B_FALSE);
+		break;
+	case ISCSI_OP_NOOP_OUT:
+	case ISCSI_OP_LOGIN_CMD:
+	case ISCSI_OP_TEXT_CMD:
+	case ISCSI_OP_SNACK_CMD:
+	case ISCSI_OP_NOOP_IN:
+	case ISCSI_OP_SCSI_RSP:
+	case ISCSI_OP_LOGIN_RSP:
+	case ISCSI_OP_TEXT_RSP:
+	case ISCSI_OP_SCSI_DATA_RSP:
+	case ISCSI_OP_LOGOUT_RSP:
+	case ISCSI_OP_ASYNC_EVENT:
+	case ISCSI_OP_REJECT_MSG:
+		/*
+		 * Additional headers not allowed.
+		 * (must be zero according to RFC3720.)
+		 */
+		if (bhs->hlength)
+			return (B_FALSE);
+		break;
+	case ISCSI_OP_SCSI_CMD:
+		/*
+		 * See RFC3720, section 10.3
+		 *
+		 * For pure read cmds, data-segment-length must be zero.
+		 * For non-final transfers, data-size must be even number of
+		 * 4-byte words.
+		 * For any transfer, an expected byte count must be provided.
+		 * For bidirectional transfers, an additional-header must be
+		 * provided (for the read byte-count.)
+		 */
+		if (pdu->isp_datalen) {
+			if ((bhs->flags & (ISCSI_FLAG_CMD_READ |
+			    ISCSI_FLAG_CMD_WRITE)) == ISCSI_FLAG_CMD_READ)
+				return (B_FALSE);
+			if ((bhs->flags & ISCSI_FLAG_FINAL) == 0 &&
+			    ((pdu->isp_datalen & 0x3) != 0))
+				return (B_FALSE);
+		}
+		if (bhs->flags & (ISCSI_FLAG_CMD_READ |
+		    ISCSI_FLAG_CMD_WRITE)) {
+			iscsi_scsi_cmd_hdr_t *cmdhdr =
+			    (iscsi_scsi_cmd_hdr_t *)bhs;
+			/*
+			 * we're transfering some data, we must have a
+			 * byte count
+			 */
+			if (cmdhdr->data_length == 0)
+				return (B_FALSE);
+		}
+		break;
+	case ISCSI_OP_SCSI_DATA:
+		/*
+		 * See RFC3720, section 10.7
+		 *
+		 * Additional headers aren't allowed, and the data-size must
+		 * be an even number of 4-byte words (unless the final bit
+		 * is set.)
+		 */
+		if (bhs->hlength)
+			return (B_FALSE);
+		if ((bhs->flags & ISCSI_FLAG_FINAL) == 0 &&
+		    ((pdu->isp_datalen & 0x3) != 0))
+			return (B_FALSE);
+		break;
+	default:
+		break;
+	}
+	return (B_TRUE);
+}
 
 static idm_status_t
 idm_sorecvhdr(idm_conn_t *ic, idm_pdu_t *pdu)
@@ -764,10 +862,10 @@ idm_sorecvhdr(idm_conn_t *ic, idm_pdu_t *pdu)
 	pdu->isp_hdrlen = sizeof (iscsi_hdr_t) +
 	    (bhs->hlength * sizeof (uint32_t));
 	pdu->isp_datalen = n2h24(bhs->dlength);
-	if (ic->ic_conn_type == CONN_TYPE_TGT &&
-	    pdu->isp_datalen > ic->ic_conn_params.max_recv_dataseglen) {
+
+	if (!idm_dataseglenokay(ic, pdu)) {
 		IDM_CONN_LOG(CE_WARN,
-		    "idm_sorecvhdr: exceeded the max data segment length");
+		    "idm_sorecvhdr: invalid data segment length");
 		return (IDM_STATUS_FAIL);
 	}
 	if (bhs->hlength > IDM_SORX_CACHE_AHSLEN) {
@@ -1550,13 +1648,12 @@ idm_so_rx_datain(idm_conn_t *ic, idm_pdu_t *pdu)
 
 	ASSERT(ic != NULL);
 	ASSERT(pdu != NULL);
+	ASSERT(IDM_PDU_OPCODE(pdu) == ISCSI_OP_SCSI_DATA_RSP);
 
 	bhs	= (iscsi_data_hdr_t *)pdu->isp_hdr;
 	datasn	= ntohl(bhs->datasn);
 	offset	= ntohl(bhs->offset);
 
-	ASSERT(bhs->opcode == ISCSI_OP_SCSI_DATA_RSP);
-
 	/*
 	 * Look up the task corresponding to the initiator task tag
 	 * to get the buffers affiliated with the task.
@@ -1611,7 +1708,7 @@ idm_so_rx_datain(idm_conn_t *ic, idm_pdu_t *pdu)
 	 * Revisit, need to provide an explicit client entry point for
 	 * phase collapse completions.
 	 */
-	if (((ihp->opcode & ISCSI_OPCODE_MASK) == ISCSI_OP_SCSI_DATA_RSP) &&
+	if ((IDM_PDU_OPCODE(pdu) == ISCSI_OP_SCSI_DATA_RSP) &&
 	    (idrhp->flags & ISCSI_FLAG_DATA_STATUS)) {
 		(*ic->ic_conn_ops.icb_rx_scsi_rsp)(ic, pdu);
 	}
@@ -1638,10 +1735,10 @@ idm_so_rx_dataout(idm_conn_t *ic, idm_pdu_t *pdu)
 
 	ASSERT(ic != NULL);
 	ASSERT(pdu != NULL);
+	ASSERT(IDM_PDU_OPCODE(pdu) == ISCSI_OP_SCSI_DATA);
 
 	bhs = (iscsi_data_hdr_t *)pdu->isp_hdr;
 	offset = ntohl(bhs->offset);
-	ASSERT(bhs->opcode == ISCSI_OP_SCSI_DATA);
 
 	/*
 	 * Look up the task corresponding to the initiator task tag
@@ -1687,6 +1784,32 @@ idm_so_rx_dataout(idm_conn_t *ic, idm_pdu_t *pdu)
 	 */
 	if (bhs->flags & ISCSI_FLAG_FINAL) {
 		/*
+		 * We have gotten the last data-message for the current
+		 * transfer.  idb_xfer_len represents the data that the
+		 * command intended to transfer, it does not represent the
+		 * actual number of bytes transferred. If we have not
+		 * transferred the expected number of bytes something is
+		 * wrong.
+		 *
+		 * We have two options, when there is a mismatch, we can
+		 * regard the transfer as invalid -- or we can modify our
+		 * notion of "xfer_len." In order to be as stringent as
+		 * possible, here we regard this transfer as in error; and
+		 * bail out.
+		 */
+		if (idb->idb_buflen == idb->idb_xfer_len &&
+		    idb->idb_buflen !=
+		    (idb->idb_exp_offset - idb->idb_bufoffset)) {
+			printf("idm_so_rx_dataout: incomplete transfer, "
+			    "protocol err");
+			IDM_CONN_LOG(CE_NOTE,
+			    "idm_so_rx_dataout: incomplete transfer: %ld, %d",
+			    offset, (int)(idb->idb_exp_offset - offset));
+			idm_task_rele(idt);
+			idm_pdu_rx_protocol_error(ic, pdu);
+			return;
+		}
+		/*
 		 * We only want to call idm_buf_rx_from_ini_done once
 		 * per transfer.  It's possible that this task has
 		 * already been aborted in which case
@@ -1884,7 +2007,7 @@ idm_sorecv_scsidata(idm_conn_t *ic, idm_pdu_t *pdu)
 	bhs	= (iscsi_data_hdr_t *)pdu->isp_hdr;
 
 	offset	= ntohl(bhs->offset);
-	opcode	= bhs->opcode;
+	opcode	= IDM_PDU_OPCODE(pdu);
 	dlength = n2h24(bhs->dlength);
 
 	ASSERT((opcode == ISCSI_OP_SCSI_DATA_RSP) ||
@@ -2196,8 +2319,8 @@ idm_i_so_tx(idm_pdu_t *pdu)
 		ihp = (iscsi_data_hdr_t *)pdu->isp_hdr;
 		/* Write of immediate data */
 		if (ic->ic_ffp &&
-		    (ihp->opcode == ISCSI_OP_SCSI_CMD ||
-		    ihp->opcode == ISCSI_OP_SCSI_DATA)) {
+		    (IDM_PDU_OPCODE(pdu) == ISCSI_OP_SCSI_CMD ||
+		    IDM_PDU_OPCODE(pdu) == ISCSI_OP_SCSI_DATA)) {
 			idt = idm_task_find(ic, ihp->itt, ihp->ttt);
 			if (idt) {
 				mutex_enter(&idt->idt_mutex);