[illumos-Developer] review for 653: https://www.illumos.org/issues/653

[Garrett D'Amore]

There's a potential leak, or even worse, fatal assertion, in iSCSI.

I've described this in https://www.illumos.org/issues/653 but here's a
diff for the fix, which I think makes it easier to see.

I would greatly appreciate feedback on this.

	- Garrett

-r b749961aba64 usr/src/uts/common/io/comstar/port/iscsit/iscsit.c
--- a/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c	Wed Jan 19
08:12:06 2011 -0800
+++ b/usr/src/uts/common/io/comstar/port/iscsit/iscsit.c	Thu Jan 20
10:04:19 2011 -0800
@@ -21,6 +21,9 @@
 /*
  * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights
reserved.
  */
+/*
+ * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
+ */
 
 #include <sys/cpuvar.h>
 #include <sys/types.h>
@@ -3167,9 +3170,22 @@
 	mutex_exit(&ict->ict_mutex);
 
 	index = ntohl(cmdsn) % ISCSIT_RXPDU_QUEUE_LEN;
-	ASSERT(cbuf->cb_buffer[index] == NULL);
-	cbuf->cb_buffer[index] = rx_pdu;
-	cbuf->cb_num_elems++;
+
+	/*
+	 * In the normal case, assuming that the Initiator is not
+	 * buggy and that we don't have packet duplication occuring,
+	 * the entry in the array will be NULL.  However, we may have
+	 * received a duplicate PDU with cmdsn > expsn , and in that
+	 * case we just ignore this PDU -- the previously received one
+	 * remains queued for processing.  We need to be careful not
+	 * to leak this one however.
+	 */
+	if (cbuf->cb_buffer[index] != NULL) {
+		idm_pdu_free(rx_pdu);
+	} else {
+		cbuf->cb_buffer[index] = rx_pdu;
+		cbuf->cb_num_elems++;
+	}
 }
 
 static idm_pdu_t *