[SCSI] iscsi_tcp: fix potential lockup with write commands

There is a race condition in iscsi_tcp.c that may cause it to forget
that it received a R2T from the target.  This race may cause a data-out
command (such as a write) to lock up.  The race occurs here:

static int
iscsi_send_unsol_pdu(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
{
	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
	int rc;

	if (tcp_ctask->xmstate & XMSTATE_UNS_HDR) {
		BUG_ON(!ctask->unsol_count);
		tcp_ctask->xmstate &= ~XMSTATE_UNS_HDR; <---- RACE
		...

static int
iscsi_r2t_rsp(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
{
	...
	tcp_ctask->xmstate |= XMSTATE_SOL_HDR_INIT; <---- RACE
	...

While iscsi_xmitworker() (called from scsi_queue_work()) is preparing to
send unsolicited data, iscsi_tcp_data_recv() (called from
tcp_read_sock()) interrupts it upon receipt of a R2T from the target.
Both contexts do read-modify-write of tcp_ctask->xmstate.  Usually, gcc
on x86 will make &= and |= atomic on UP (not guaranteed of course), but
in this case iscsi_send_unsol_pdu() reads the value of xmstate before
clearing the bit, which causes gcc to read xmstate into a CPU register,
test it, clear the bit, and then store it back to memory.  If the recv
interrupt happens during this sequence, then the XMSTATE_SOL_HDR_INIT
bit set by the recv interrupt will be lost, and the R2T will be
forgotten.

The patch below (against 2.6.24-rc1) converts accesses of xmstate to use
set_bit, clear_bit, and test_bit instead of |= and &=.  I have tested
this patch and verified that it fixes the problem.  Another possible
approach would be to hold a lock during most of the rx/tx setup and
post-processing, and drop the lock only for the actual rx/tx.

Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

1 files changed, 17 insertions, 17 deletions

diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
index 7eba44df0a7f..68c36cc8997e 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -32,21 +32,21 @@
 #define IN_PROGRESS_PAD_RECV            0x4
 
 /* xmit state machine */
-#define XMSTATE_IDLE                    0x0
-#define XMSTATE_CMD_HDR_INIT            0x1
-#define XMSTATE_CMD_HDR_XMIT            0x2
-#define XMSTATE_IMM_HDR                 0x4
-#define XMSTATE_IMM_DATA                0x8
-#define XMSTATE_UNS_INIT                0x10
-#define XMSTATE_UNS_HDR                 0x20
-#define XMSTATE_UNS_DATA                0x40
-#define XMSTATE_SOL_HDR                 0x80
-#define XMSTATE_SOL_DATA                0x100
-#define XMSTATE_W_PAD                   0x200
-#define XMSTATE_W_RESEND_PAD            0x400
-#define XMSTATE_W_RESEND_DATA_DIGEST    0x800
-#define XMSTATE_IMM_HDR_INIT            0x1000
-#define XMSTATE_SOL_HDR_INIT            0x2000
+#define XMSTATE_VALUE_IDLE                      0
+#define XMSTATE_BIT_CMD_HDR_INIT                0
+#define XMSTATE_BIT_CMD_HDR_XMIT                1
+#define XMSTATE_BIT_IMM_HDR                     2
+#define XMSTATE_BIT_IMM_DATA                    3
+#define XMSTATE_BIT_UNS_INIT                    4
+#define XMSTATE_BIT_UNS_HDR                     5
+#define XMSTATE_BIT_UNS_DATA                    6
+#define XMSTATE_BIT_SOL_HDR                     7
+#define XMSTATE_BIT_SOL_DATA                    8
+#define XMSTATE_BIT_W_PAD                       9
+#define XMSTATE_BIT_W_RESEND_PAD                10
+#define XMSTATE_BIT_W_RESEND_DATA_DIGEST        11
+#define XMSTATE_BIT_IMM_HDR_INIT                12
+#define XMSTATE_BIT_SOL_HDR_INIT                13
 
 #define ISCSI_PAD_LEN                   4
 #define ISCSI_SG_TABLESIZE              SG_ALL
@@ -122,7 +122,7 @@ struct iscsi_data_task {
 struct iscsi_tcp_mgmt_task {
         struct iscsi_hdr        hdr;
         char                    hdrext[sizeof(__u32)]; /* Header-Digest */
-        int                     xmstate;        /* mgmt xmit progress */
+        unsigned long           xmstate;        /* mgmt xmit progress */
         struct iscsi_buf        headbuf;        /* header buffer */
         struct iscsi_buf        sendbuf;        /* in progress buffer */
         int                     sent;
@@ -150,7 +150,7 @@ struct iscsi_tcp_cmd_task {
         int                     pad_count;              /* padded bytes */
         struct iscsi_buf        headbuf;                /* header buf (xmit) */
         struct iscsi_buf        sendbuf;                /* in progress buffer*/
-        int                     xmstate;                /* xmit xtate machine */
+        unsigned long           xmstate;                /* xmit xtate machine */
         int                     sent;
         struct scatterlist      *sg;                    /* per-cmd SG list  */
         struct scatterlist      *bad_sg;                /* assert statement */