aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/scsi/iscsi_tcp.h
diff options
context:
space:
mode:
authorTony Battersby <tonyb@cybernetics.com>2007-11-14 15:38:42 -0500
committerJames Bottomley <James.Bottomley@HansenPartnership.com>2007-11-14 15:51:58 -0500
commit505f76b3061f6e74a50f378e45ac931abc1fe784 (patch)
tree55074ebf00aa0f7fc336d83392a01b20267978c6 /drivers/scsi/iscsi_tcp.h
parent5f78e89b5f7041895c4820be5c000792243b634f (diff)
[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>
Diffstat (limited to 'drivers/scsi/iscsi_tcp.h')
-rw-r--r--drivers/scsi/iscsi_tcp.h34
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 @@
32#define IN_PROGRESS_PAD_RECV 0x4 32#define IN_PROGRESS_PAD_RECV 0x4
33 33
34/* xmit state machine */ 34/* xmit state machine */
35#define XMSTATE_IDLE 0x0 35#define XMSTATE_VALUE_IDLE 0
36#define XMSTATE_CMD_HDR_INIT 0x1 36#define XMSTATE_BIT_CMD_HDR_INIT 0
37#define XMSTATE_CMD_HDR_XMIT 0x2 37#define XMSTATE_BIT_CMD_HDR_XMIT 1
38#define XMSTATE_IMM_HDR 0x4 38#define XMSTATE_BIT_IMM_HDR 2
39#define XMSTATE_IMM_DATA 0x8 39#define XMSTATE_BIT_IMM_DATA 3
40#define XMSTATE_UNS_INIT 0x10 40#define XMSTATE_BIT_UNS_INIT 4
41#define XMSTATE_UNS_HDR 0x20 41#define XMSTATE_BIT_UNS_HDR 5
42#define XMSTATE_UNS_DATA 0x40 42#define XMSTATE_BIT_UNS_DATA 6
43#define XMSTATE_SOL_HDR 0x80 43#define XMSTATE_BIT_SOL_HDR 7
44#define XMSTATE_SOL_DATA 0x100 44#define XMSTATE_BIT_SOL_DATA 8
45#define XMSTATE_W_PAD 0x200 45#define XMSTATE_BIT_W_PAD 9
46#define XMSTATE_W_RESEND_PAD 0x400 46#define XMSTATE_BIT_W_RESEND_PAD 10
47#define XMSTATE_W_RESEND_DATA_DIGEST 0x800 47#define XMSTATE_BIT_W_RESEND_DATA_DIGEST 11
48#define XMSTATE_IMM_HDR_INIT 0x1000 48#define XMSTATE_BIT_IMM_HDR_INIT 12
49#define XMSTATE_SOL_HDR_INIT 0x2000 49#define XMSTATE_BIT_SOL_HDR_INIT 13
50 50
51#define ISCSI_PAD_LEN 4 51#define ISCSI_PAD_LEN 4
52#define ISCSI_SG_TABLESIZE SG_ALL 52#define ISCSI_SG_TABLESIZE SG_ALL
@@ -122,7 +122,7 @@ struct iscsi_data_task {
122struct iscsi_tcp_mgmt_task { 122struct iscsi_tcp_mgmt_task {
123 struct iscsi_hdr hdr; 123 struct iscsi_hdr hdr;
124 char hdrext[sizeof(__u32)]; /* Header-Digest */ 124 char hdrext[sizeof(__u32)]; /* Header-Digest */
125 int xmstate; /* mgmt xmit progress */ 125 unsigned long xmstate; /* mgmt xmit progress */
126 struct iscsi_buf headbuf; /* header buffer */ 126 struct iscsi_buf headbuf; /* header buffer */
127 struct iscsi_buf sendbuf; /* in progress buffer */ 127 struct iscsi_buf sendbuf; /* in progress buffer */
128 int sent; 128 int sent;
@@ -150,7 +150,7 @@ struct iscsi_tcp_cmd_task {
150 int pad_count; /* padded bytes */ 150 int pad_count; /* padded bytes */
151 struct iscsi_buf headbuf; /* header buf (xmit) */ 151 struct iscsi_buf headbuf; /* header buf (xmit) */
152 struct iscsi_buf sendbuf; /* in progress buffer*/ 152 struct iscsi_buf sendbuf; /* in progress buffer*/
153 int xmstate; /* xmit xtate machine */ 153 unsigned long xmstate; /* xmit xtate machine */
154 int sent; 154 int sent;
155 struct scatterlist *sg; /* per-cmd SG list */ 155 struct scatterlist *sg; /* per-cmd SG list */
156 struct scatterlist *bad_sg; /* assert statement */ 156 struct scatterlist *bad_sg; /* assert statement */