From: mchristi@redhat.com <mchristi@redhat.com>
Date: Mon, 15 Sep 2008 18:54:14 -0500
Subject: [scsi] libiscsi: data corruption when resending packets
Message-id: 1221522854-2234-1-git-send-email-mchristi@redhat.com
O-Subject: [PATCH] RHEL 5.3: libiscsi: fix data corruption when target has to resend packets
Bugzilla: 460158

From: Mike Christie <mchristi@redhat.com>

This is for BZ 460158.

iscsi_tcp was updating the exp_statsn (exp_statsn acknowledges
status and tells the target it is ok to let the resources for
a iscsi pdu (iscsi pdu is basically a iscsi level packet)
to be reused) before it got all the data for iscsi
pdu read into OS buffers. This lead to data corruption if something happens
to a network packet and the network layer requests a retransmit, because
when the targets gets the updated exp_statsn then
it might reallocate buffers and for the retransmit request
it may be sending data from a buffer it has reused
for a new iscsi pdu. This fixes the problem by having the LLD,
iscsi_tcp in this case, transfer data then update the status bits.

I have tested with a couple targets, iet, stgt, and istor by doing
reads to the target so it hits the node paths below to check
for regrssions. I haven't been able to cause the curruption bug to
occur, so I have not veriried that. The bz reporter did test a
similar patch for RHEL4's iscsi initiator which works like
the RHEL5 one and had the same issue. They did not have RHEL5
to test though.

I am just sending this to the lists now, so it is not yet upstream.

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 86a7aea..e7a7978 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -205,18 +205,17 @@ iscsi_tcp_cleanup_ctask(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 }
 
 /**
- * iscsi_data_rsp - SCSI Data-In Response processing
+ * iscsi_data_in - SCSI Data-In Response processing
  * @conn: iscsi connection
  * @ctask: scsi command task
  **/
 static int
-iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
+iscsi_data_in(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 {
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
 	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 	struct iscsi_data_rsp *rhdr = (struct iscsi_data_rsp *)tcp_conn->in.hdr;
 	struct iscsi_session *session = conn->session;
-	struct scsi_cmnd *sc = ctask->sc;
 	int datasn = be32_to_cpu(rhdr->datasn);
 
 	iscsi_update_cmdsn(session, (struct iscsi_nopin*)rhdr);
@@ -240,25 +239,6 @@ iscsi_data_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 	if (tcp_ctask->data_offset + tcp_conn->in.datalen > ctask->total_length)
 		return ISCSI_ERR_DATA_OFFSET;
 
-	if (rhdr->flags & ISCSI_FLAG_DATA_STATUS) {
-		conn->exp_statsn = be32_to_cpu(rhdr->statsn) + 1;
-		if (rhdr->flags & ISCSI_FLAG_DATA_UNDERFLOW) {
-			int res_count = be32_to_cpu(rhdr->residual_count);
-
-			if (res_count > 0 &&
-			    res_count <= sc->request_bufflen) {
-				sc->resid = res_count;
-				sc->result = (DID_OK << 16) | rhdr->cmd_status;
-			} else
-				sc->result = (DID_BAD_TARGET << 16) |
-					rhdr->cmd_status;
-		} else if (rhdr->flags & ISCSI_FLAG_DATA_OVERFLOW) {
-			sc->resid = be32_to_cpu(rhdr->residual_count);
-			sc->result = (DID_OK << 16) | rhdr->cmd_status;
-		} else
-			sc->result = (DID_OK << 16) | rhdr->cmd_status;
-	}
-
 	conn->datain_pdus_cnt++;
 	return 0;
 }
@@ -493,7 +473,7 @@ iscsi_tcp_hdr_recv(struct iscsi_conn *conn)
 	switch(opcode) {
 	case ISCSI_OP_SCSI_DATA_IN:
 		tcp_conn->in.ctask = session->cmds[itt];
-		rc = iscsi_data_rsp(conn, tcp_conn->in.ctask);
+		rc = iscsi_data_in(conn, tcp_conn->in.ctask);
 		if (rc)
 			return rc;
 		/* fall through */
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 603fc50..9e7b9fe 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -407,6 +407,41 @@ out:
 	return 0;
 }
 
+/**
+ * iscsi_data_in_rsp - SCSI Data-In Response processing
+ * @conn: iscsi connection
+ * @hdr:  iscsi pdu
+ * @ctask: scsi command task
+ */
+static void
+iscsi_data_in_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
+		  struct iscsi_cmd_task *ctask)
+{
+	struct iscsi_data_rsp *rhdr = (struct iscsi_data_rsp *)hdr;
+	struct scsi_cmnd *sc = ctask->sc;
+
+	if (!(rhdr->flags & ISCSI_FLAG_DATA_STATUS))
+		return;
+
+	sc->result = (DID_OK << 16) | rhdr->cmd_status;
+	conn->exp_statsn = be32_to_cpu(rhdr->statsn) + 1;
+	if (rhdr->flags & (ISCSI_FLAG_DATA_UNDERFLOW |
+	                   ISCSI_FLAG_DATA_OVERFLOW)) {
+		int res_count = be32_to_cpu(rhdr->residual_count);
+
+		if (res_count > 0 &&
+		    (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
+		     res_count <= sc->request_bufflen))
+			sc->resid = res_count;
+		else
+			sc->result = (DID_BAD_TARGET << 16) |
+				rhdr->cmd_status;
+	}
+
+	conn->scsirsp_pdus_cnt++;
+	__iscsi_put_ctask(ctask);
+}
+
 static void iscsi_tmf_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 {
 	struct iscsi_tm_rsp *tmf = (struct iscsi_tm_rsp *)hdr;
@@ -523,10 +558,7 @@ int __iscsi_complete_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
 			break;
 		case ISCSI_OP_SCSI_DATA_IN:
 			BUG_ON((void*)ctask != ctask->sc->SCp.ptr);
-			if (hdr->flags & ISCSI_FLAG_DATA_STATUS) {
-				conn->scsirsp_pdus_cnt++;
-				__iscsi_put_ctask(ctask);
-			}
+			iscsi_data_in_rsp(conn, hdr, ctask);
 			break;
 		case ISCSI_OP_R2T:
 			/* LLD handles this for now */